[PATCH] D51887: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence.

[Nico Weber via Phabricator via llvm-commits]

thakis created this revision.
thakis added a reviewer: zturner.
Herald added a subscriber: hiraditya.

This is not quite ready for real review. I still need to collect performance data of this. Also, the PDB hash changes on every run at the moment for reasons I don't understand yet, which defeats the 'deterministic builds!' purpose of this patch. It's because of 2 bytes changing; according to `pdbutil explain` the difference is here:

  bin/llvm-pdbutil explain -offset=0x5010 test1.pdb 
  
  Explaining file offset 20496 of file 'test1.pdb'.
  Block:Offset = 5:0010.
  Address is in block 5 (allocated).
    Address is at offset 16/576 of Stream 7 (Public Symbol Hash).

I still need to debug that. So feel free to ignore for now, but I figured I'd upload what I have. It's probably done enough that I can use it for benchmarking.

Actual patch description:

Previously, lld-link would use a random byte sequence as the PDB GUID. Instead, use a hash of the PDB file contents.

To compute it, introduce HashingFileBufferByteStream that computes a running hash of the bytes it writes.

Since we already use xxhash, add its streaming parts to llvm//Support/xxhash. xxhash gives only 8 bytes of content hash, so put a fixed string in the other 8 bytes available in the PDB GUID.

To not disturb llvm-pdbutil pdb2yaml, make the hash generation an opt-in feature on InfoStreamBuilder and let ldb/COFF/PDB.cpp always set it.

Since writing the PDB computes this ID which also goes in the exe, the PDB writing code now must be called before writeBuildId(). writeBuildId() for that reason is no longer included in the "Code Layout" timer.

Since the PDB GUID is now a function of the PDB contents, the PDB Age is always set to 1. There was a long comment above loadExistingBuildId (now gone) about how not changing the GUID and only incrementing the age was important, but according to the discussion in PR35914 that comment was incorrect.


https://reviews.llvm.org/D51887

Files:
  lld/COFF/PDB.cpp
  lld/COFF/PDB.h
  lld/COFF/Writer.cpp
  lld/test/COFF/rsds.test
  llvm/include/llvm/DebugInfo/MSF/MSFBuilder.h
  llvm/include/llvm/DebugInfo/PDB/Native/InfoStreamBuilder.h
  llvm/include/llvm/DebugInfo/PDB/Native/PDBFileBuilder.h
  llvm/include/llvm/Support/xxhash.h
  llvm/lib/DebugInfo/MSF/MSFBuilder.cpp
  llvm/lib/DebugInfo/PDB/Native/InfoStreamBuilder.cpp
  llvm/lib/DebugInfo/PDB/Native/PDBFileBuilder.cpp
  llvm/lib/Support/xxhash.cpp
  llvm/tools/llvm-pdbutil/llvm-pdbutil.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D51887.164731.patch
Type: text/x-patch
Size: 23569 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180910/e015bcf5/attachment.bin>