[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

asmith created this revision. asmith added reviewers: zturner, llvm-commits. Herald added a subscriber: lldb-commits. Provide an implementation of GetUUID() for remote debugging scenarios based on the md5 of the object's path. Include a simple lit test that checks the first 8 bytes of the UUID i

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

zturner added a comment. Should we use the GUID from the COFF Debug Directory instead? It certainly seems more appropriate, if it's there. The UUID's purpose is to match symbol to the executable, so if you use a hash of the path it might solve this one problem, but won't solve the general cas

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

labath added a comment. I would definitely encourage using something better than the file checksum as UUID, if at all possible. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:139 +uuid = +UUID::fromOptionalData(llvm::ArrayRef(Resu

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

Hui added a comment. Not quite sure but correct me if i am wrong. (1) I think the Debug Directory is optional for COFF if it does have debug information and pdb to match with. (2) The Debug Directory does not contain COFF timestamp. Using md5 seems very tentative. Please elaborate how to lever

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

zturner added a comment. In D56229#1346869 , @Hui wrote: > Not quite sure but correct me if i am wrong. > > (1) I think the Debug Directory is optional for COFF if it does have debug > information and pdb to match with. > > (2) The Debug Directory does no

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

clayborg added a comment. The UUID that is used in ELF and Mach-o is designed to be something that is stable in a binary after it has been linked and should be the same before and after any kind of post production (stripping symbols, stripping section content to make a stand alone symbol file,

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

labath added a comment. Herald added a project: LLDB. In D56229#1346941 , @zturner wrote: > Well, I guess I would ask what you want to do with the GUID? If you want to > match it to a debug information file, then the Debug Directory is the correct > way

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

asmith updated this revision to Diff 195282. asmith edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56229/new/ https://reviews.llvm.org/D56229 Files: lit/Modules/PECOFF/export-dllfunc.yaml source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp sour

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

labath edited reviewers, added: amccarth, labath; removed: zturner, llvm-commits. labath added a subscriber: amccarth. labath added a comment. s/@zturner/@amccarth, as Zach probably won't have time to review this Comment at: lit/Modules/PECOFF/export-dllfunc.yaml:11-12 + +# ti

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

labath added inline comments. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:61 + if (!COFFObj->getDebugPDBInfo(pdb_info, pdb_file) && pdb_info) +return UUID::fromOptionalData(pdb_info->PDB70.Signature); + Is there a specific reason you us

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

Hui added inline comments. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:60 + llvm::StringRef pdb_file; + if (!COFFObj->getDebugPDBInfo(pdb_info, pdb_file) && pdb_info) +return UUID::fromOptionalData(pdb_info->PDB70.Signature); labath wr

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

labath added inline comments. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:60 + llvm::StringRef pdb_file; + if (!COFFObj->getDebugPDBInfo(pdb_info, pdb_file) && pdb_info) +return UUID::fromOptionalData(pdb_info->PDB70.Signature); Hui wr

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

asmith updated this revision to Diff 195504. asmith edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56229/new/ https://reviews.llvm.org/D56229 Files: lit/Modules/PECOFF/export-dllfunc.yaml lit/Modules/PECOFF/uuid.yaml source/Plugins/ObjectFile/PEC

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

labath added a comment. Thanks. I have a couple of small comments, but I think this is basically done. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:43 +namespace { +using namespace llvm; llvm style is to only use the anonymous namespaces

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

Hui added inline comments. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:891-892 + + if (!CreateBinary()) +return UUID(); + auto COFFObj = labath wrote: > I don't think this is necessary as `CreateInstance` will refuse to return the > O

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

labath added inline comments. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:891-892 + + if (!CreateBinary()) +return UUID(); + auto COFFObj = Hui wrote: > labath wrote: > > I don't think this is necessary as `CreateInstance` will refuse

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

asmith updated this revision to Diff 196874. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56229/new/ https://reviews.llvm.org/D56229 Files: lit/Modules/PECOFF/export-dllfunc.yaml lit/Modules/PECOFF/uuid.yaml source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp source/Plugins/Ob

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:81 +info.Age = read32be(&pdb_info->PDB70.Age); +return UUID::fromOptionalData((uint8_t *

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

asmith updated this revision to Diff 197239. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56229/new/ https://reviews.llvm.org/D56229 Files: lit/Modules/PECOFF/export-dllfunc.yaml lit/Modules/PECOFF/uuid.yaml source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp source/Plugins/Ob