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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56229/new/
https://reviews.llvm.org/D56229
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
