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

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 16 00:20:39 PDT 2019

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
+# timestamp and build id of debug info in the coff header varies. So does its UUID.
+# BASIC-CHECK-DAG: UUID: {{[0-9A-F]{7,}[0-9A-F]}}-{{.*}}
Since the U(G)UID needs to be stable and match the value computed from other sources, it would be good to have a test where we check that a file has some exact UUID.

Is there any way to use yaml2obj to generate such a file? For instance, if we drop the `lld-link` step and yamlify the resulting `dll` file instead. Would that work?

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);
Can `getDebugPDBInfo` succeed and still return a null pdb_info? If not, can we delete the second part?

Instead I believe you should check the CVSignature field of the returned struct to see that it indeed contains a PDB70 record.

Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:866
+  m_uuid = GetCoffUUID(GetFileSpec());
+  return m_uuid;
ObjectFilePECOFF already has a `llvm::object::Binary` created for the underlying file. I think it's super-wasteful (and potentially racy, etc.) to create a second one just to read out it's GUID. If you make a second version of this function, taking a `Binary` (and have the FileSpec version delegate to that), then you can avoid this.



More information about the lldb-commits mailing list