[PATCH] D51956: 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 added inline comments.


================
Comment at: lld/COFF/PDB.cpp:133
 
-  /// Write the PDB to disk.
-  void commit();
+  /// Write the PDB to disk and return the Guid generated for it.
+  void commit(codeview::GUID *Guid);
----------------
ruiu wrote:
> It's return type is void. I'd change the comment or change the return type.
Will do.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/PDBFileBuilder.cpp:28
 #include "llvm/Support/Path.h"
+#include "llvm/Support/xxhash.h"
 
----------------
ruiu wrote:
> As long as you are using a non-crypto hash function, there is a risk of generating the same build id, and the probability is not negligible if you have a lot of executables due to the birthday problem. Is this okay?
The 8 byte hash still gives decent hash collision resistance for up to 2**32 different pdb files, and since pdbs are keyed by executable name on the symbol server that's per binary. Projects tend to have far fewer revisions than 4 billion. Does that make sense?


https://reviews.llvm.org/D51956