[PATCH] D35086: Fix some differences between LLD and MSVC generated PDBs

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

(BTW, ignore in case you already noticed them, but I also have https://reviews.llvm.org/D35092 and https://reviews.llvm.org/D35039 if you don't mind taking a look at those as well)



================
Comment at: lld/COFF/PDB.cpp:344
   InfoBuilder.setAge(DI ? DI->PDB70.Age : 0);
+  InfoBuilder.setSignature(time(nullptr));
 
----------------
rnk wrote:
> rnk wrote:
> > Generally speaking we want our tool output to be deterministic. Clang has a /Brepro flag that controls the emission of timestamps in file headers. Can you add a FIXME here to add a flag for this?
> Actually, ignore this. We already need to absolute-ize paths, so our output isn't really deterministic.
There are other places where we parts of LLD where we will need to by default produce non-reproducible builds as well (for example, the guid in a PDB needs to be unique).  Probably one or two other examples that escape me at the moment.  But yea, we should add a flag for this and use the flag in tests.


================
Comment at: lld/test/COFF/pdb-diff.test:10
+
+CHECK:        ----------------------
+CHECK-NEXT:   |  MSF Super Block   |
----------------
rnk wrote:
> Are you worried about the fragility of these CHECKs? FileCheck ignores whitespace differences, so it might not be so bad, but I'm a little concerned.
Doesn't it only ignore leading and trailing whitespace differences?  We could add a "no table" mode that just outputs `<Field>: <Result>` but I think that can come later, right now it's just good to have minimal test coverage documenting the places we differ.


https://reviews.llvm.org/D35086