[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: llvm/lib/DebugInfo/PDB/Native/PDBFileBuilder.cpp:28
 #include "llvm/Support/Path.h"
+#include "llvm/Support/xxhash.h"
 
----------------
ruiu wrote:
> thakis wrote:
> > 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?
> Maybe it is safe. But what could happen if two executables have the same hash? Since xxhash is not cryptographically-safe, you could easily generate two executables having the same ID. Is there any security risks or something caused by that possibility? If the probability is small and the result of hash collision is not that bad, xxhash is probably okay.
The main use case for this guid is to an executable to its pdb file. The common workflow is that a build server builds an executable and its pdb, then uploads both to a symbol server (under the namespace of the exe, the exe in a subdir containing the exe's pe timestamp and size, and the pdb under the guid). If the executable crashes, it produces a minidump. From the minidump, crash infrastructure can obtain the full executable and the pdb.

Since nothing guarantees that the pdb guid is a hash of the pdb data, I can't think of anything where being able to produce a pdb with a given uuid that is an xxhash buys you anything: Since nothing forces the guid to be a hash, you can just produce a pdb and set its guid field to whatever you want anyways.


https://reviews.llvm.org/D51956