[PATCH] D43978: Write a hash of the binary as the PE Debug Directory Timestamp

[Zachary Turner via Phabricator via llvm-commits]

zturner updated this revision to Diff 136659.
zturner added a comment.

It turns out I wasn't actually doing enough.  The value that symbol servers *really* care about is the time stamp in the top-level coff file header.  However, since link.exe sets this time stamp and the time stamp of al debug directories to matching values, we do the same here.

With my first version of the patch, symbol servers were still not working correctly, but after this version that writes the time stamp to the COFF file header, it is now working correctly.


https://reviews.llvm.org/D43978

Files:
  lld/COFF/Writer.cpp


Index: lld/COFF/Writer.cpp
===================================================================
--- lld/COFF/Writer.cpp
+++ lld/COFF/Writer.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Support/Parallel.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/RandomNumberGenerator.h"
+#include "llvm/Support/xxhash.h"
 #include <algorithm>
 #include <cstdio>
 #include <map>
@@ -71,11 +72,18 @@
       uint64_t Offs = OS->getFileOff() + (Record->getRVA() - OS->getRVA());
       D->PointerToRawData = Offs;
 
+      TimeDateStamps.push_back(&D->TimeDateStamp);
       ++D;
     }
   }
 
+  void setTimeDateStamp(uint32_t TimeDateStamp) {
+    for (support::ulittle32_t *TDS : TimeDateStamps)
+      *TDS = TimeDateStamp;
+  }
+
 private:
+  mutable std::vector<support::ulittle32_t *> TimeDateStamps;
   const std::vector<Chunk *> &Records;
 };
 
@@ -157,7 +165,7 @@
   RVATableChunk *GuardFidsTable = nullptr;
   RVATableChunk *SEHTable = nullptr;
 
-  Chunk *DebugDirectory = nullptr;
+  DebugDirectoryChunk *DebugDirectory = nullptr;
   std::vector<Chunk *> DebugRecords;
   CVDebugRecordChunk *BuildId = nullptr;
   Optional<codeview::DebugInfo> PreviousBuildId;
@@ -1032,16 +1040,38 @@
     return;
 
   assert(BuildId && "BuildId is not set!");
+  assert(DebugDirectory && "DebugDirectory is not set!");
+
+  // Set the time stamp to a hash of the executable file, but do this before
+  // actually setting the guid and age, since those introduce some
+  // non-determinism.  This way at least the hash will be deterministic.
+  // Note that there are several time stamps that need to be set, and they
+  // should all match.
+  // * The first is in the COFF file header which occurs immediately after
+  //   the DOS stub at the beginning of the file.
+  // * The second occur in the debug directories, one in each directory.  The
+  //   PE spec says this should be the time that the debug information was
+  //   created, but in practice I cannot find a link.exe generated binary where
+  //   the values aren't identical to the one in the COFF file header.
+  StringRef OutputFileData(
+      reinterpret_cast<const char *>(Buffer->getBufferStart()),
+      Buffer->getBufferSize());
+  uint32_t Hash = static_cast<uint32_t>(xxHash64(OutputFileData));
+  DebugDirectory->setTimeDateStamp(Hash);
+  uint8_t *Buf = Buffer->getBufferStart();
+  Buf += DOSStubSize + sizeof(PEMagic);
+  object::coff_file_header *CoffHeader =
+      reinterpret_cast<coff_file_header *>(Buf);
+  CoffHeader->TimeDateStamp = Hash;
 
   if (PreviousBuildId.hasValue()) {
     *BuildId->BuildId = *PreviousBuildId;
     BuildId->BuildId->PDB70.Age = BuildId->BuildId->PDB70.Age + 1;
-    return;
+  } else {
+    BuildId->BuildId->Signature.CVSignature = OMF::Signature::PDB70;
+    BuildId->BuildId->PDB70.Age = 1;
+    llvm::getRandomBytes(BuildId->BuildId->PDB70.Signature, 16);
   }
-
-  BuildId->BuildId->Signature.CVSignature = OMF::Signature::PDB70;
-  BuildId->BuildId->PDB70.Age = 1;
-  llvm::getRandomBytes(BuildId->BuildId->PDB70.Signature, 16);
 }
 
 // Sort .pdata section contents according to PE/COFF spec 5.5.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43978.136659.patch
Type: text/x-patch
Size: 3106 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180302/7e85386a/attachment.bin>