[PATCH] D43913: Delay writing the PDB build id until just before file commit.

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: lld/COFF/PDB.cpp:1128-1136
+  // Set the build id in the pdb file.
+  // FIXME: Set Signature here to the timestamp of the executable.  For
+  //        reproducible builds, we will probably want to go further and set
+  //        this field to a hash of the PDB file, then set the timestamp of
+  //        the executable to be the same hash.
+  Result.IdPtr->Age = BuildId.PDB70.Age;
+  ::memcpy(&Result.IdPtr->Guid, &BuildId.PDB70.Signature,
----------------
ruiu wrote:
> zturner wrote:
> > ruiu wrote:
> > > Does this unconditionally set `time(nullptr)` to the build id? If so, can't you pass it as an argument to Builder ctor or something?
> > Yes, but for now I'm trying to keep this no functional change.  This is exactly what the code did before, so I wanted to leave it the same.  In followup patches when we actually use a hash, the call to `time` will just go away.
> I wonder if you could pass a lambda to compute a hash value instead. Lambda is a convenient way to pass a piece of code to other module. I'm wondering if it's applicable here.
In practice this value is going to be a hash of the PDB.  So instead of calling `time(nullptr)` I just call a function to compute the hash.  Then somehow I pass this value back to `lld::coff::Writer` so it can write the same value to the Debug Directory in the PE.  So there won't be a need to pass a lambda, because the PDB Linker will still be the one to compute it.  So in a followup patch, I'll delete this, replace it with something like like `Result.IdPtr->Signature = hashPdb(*Result.File);` and then return the value of `Result.IdPtr->Signature` back to the caller so it can write the same value.  So I don't think lambdas or anything fancy are needed.


https://reviews.llvm.org/D43913