[PATCH] D131224: [objdump] Find debug information with Build ID/debuginfod.

[Daniel Thornburgh via Phabricator via llvm-commits]

mysterymath added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-objdump.rst:134-137
+  Whether or not to try debuginfod lookups for debug binaries. Unless specified,
+  debuginfod is only enabled if libcurl was compiled in (``LLVM_ENABLE_CURL``)
+  and at least one server URL was provided by the environment variable
+  ``DEBUGINFOD_URLS``.
----------------
jhenderson wrote:
> Just trying to understand this: is this saying that `LLVM_ENABLE_CURL` and the server URLs effectively just impact the default value of this option, but that you can still specify --debuginfod without that being used?
Yes, if you specify `--debuginfod` without curl available, it will only search the local debuginfod cached directory. This has mostly just testing utility.


================
Comment at: llvm/test/tools/llvm-objdump/debuginfod.test:9
+# Produce a stripped copy of the input binary.
+RUN: llvm-objcopy --strip-debug %p/Inputs/embedded-source %t/stripped
+
----------------
jhenderson wrote:
> Is there a particular reason to use a canned binary instead of building from assembly with debug information (i.e. I believe `llvm-mc -g`)? If so, could you please include the original source file and instructions on how to compile it to create the canned input.
The binaries produced by llvm-mc don't include build IDs; these are currently produced by lld. I don't know of a LLVM-only way to readably and reliably produce a build ID section.
I've added a link to the existing embedded-sources.test, which provides the information used to regenerate this file. I had to run the regeneration command as part of this change to ensure that a build ID was present, since the canned binary predates their generation by default.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1281
+  if (!DebugBinary) {
+    consumeError(DebugBinary.takeError());
+    return None;
----------------
jhenderson wrote:
> Why are we throwing away the error here, rather than reporting it? Please at least add a comment explaining why.
Changed this to output a warning in this case; added a corresponding test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131224