[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:
> 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.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/InfoStreamBuilder.cpp:30
+      NamedStreams(NamedStreams) {
+  ::memset(Guid.Guid, 0, sizeof(Guid));
+}
----------------
ruiu wrote:
> This is suspicious -- is sizeof(Guid) == sizeof(Guid.Guid)?
Yes, but I could add a `static_assert` to that effect.


https://reviews.llvm.org/D43913