[PATCH] D118174: [Symbolize][DebugInfoD] Try looking up failed paths as Build IDs.

Wed Jan 26 12:31:37 PST 2022

mysterymath added inline comments.

================
Comment at: llvm/lib/DebugInfo/Symbolize/Symbolize.cpp:522
+          reinterpret_cast<const uint8_t *>(Bytes.data()), Bytes.size());
+      if (BuildID.size() != 20)
+        return BinOrErr.takeError();
----------------
mysterymath wrote:
> phosek wrote:
> > Build ID doesn't have any prescribed length, and linkers typically support a number of hashing algorithms (see https://github.com/llvm/llvm-project/blob/3704abaa166bec865f56f48337a1732eab77dc68/lld/ELF/Driver.cpp#L764) so this isn't going to work.
> From the link, it looks like the smallest presently supported would be MD5 and UUID, both at 128 bits.
> Given that Build IDs are intended to be globally unique (right?), that seems like a reasonable minimum for this check.
> 
> It should be fairly unlikely to have a filename of >= 16 characters that is a valid hex identifier unless it's some kind of ID; we'd be into
> aBadBeefCafeBabe territory.
Ah, but the --build_id flag can be used to stamp binaries with an arbitrary hex string as part of the build process.

So, the possibilities that immediately occur to me:
1. Keep this as a heuristic check that captures "the usual" build IDs, but then you simply couldn't use this feature in llvm-symbolizer with your own custom build ID schema.
2. Remove this check entirely, and do lookups on anything that could be a build ID. This could issue spurious lookups on paths like "bad", but only in the case where "bad" isn't a valid binary.
3. Provide an in-band mechanism in the syntax of llvm-symbolizer for specifying whether the "path" component of a symoblization request is intended to be a path or a build ID. This would need some further design.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118174/new/

https://reviews.llvm.org/D118174