[PATCH] D118665: [Debuginfod] Flag-determine debuginfod lookups in llvm-symbolizer.

[Daniel Thornburgh via Phabricator via llvm-commits]

mysterymath created this revision.
Herald added a subscriber: rupprecht.
Herald added a reviewer: jhenderson.
mysterymath updated this revision to Diff 405054.
mysterymath added a comment.
mysterymath added reviewers: phosek, mcgrathr.
mysterymath updated this revision to Diff 405058.
mysterymath published this revision for review.
Herald added subscribers: llvm-commits, MaskRay.
Herald added a project: LLVM.

Update from parent.


mysterymath added a comment.

Add test.


This change adds a pair of flags controlling whether llvm-symbolizer
attempts debuginfod lookups. Lookups are attempted if --debuginfod is
passed and disabled if --no-debuginfod is passed.

The default behavior is made more nuanced: debuginfod lookups are now
only attempted if an HTTP client is compiled in and at least one backing
debuginfod URL was configured via environment variable. Previously,
debuginfod lookups would always be attempted, even if there were no
chance that they could succeed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118665

Files:
  llvm/docs/CommandGuide/llvm-symbolizer.rst
  llvm/test/tools/llvm-symbolizer/debuginfod.test
  llvm/tools/llvm-symbolizer/Opts.td
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp


Index: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
===================================================================
--- llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
+++ llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
@@ -20,6 +20,7 @@
 #include "llvm/DebugInfo/Symbolize/DIPrinter.h"
 #include "llvm/DebugInfo/Symbolize/Symbolize.h"
 #include "llvm/Debuginfod/DIFetcher.h"
+#include "llvm/Debuginfod/Debuginfod.h"
 #include "llvm/Debuginfod/HTTPClient.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
@@ -261,6 +262,20 @@
   return IsAddr2Line ? FunctionNameKind::None : FunctionNameKind::LinkageName;
 }
 
+ExitOnError ExitOnErr;
+
+static bool shouldUseDebuginfodByDefault() {
+  // A debuginfod lookup could succeed if a HTTP client is available and at
+  // least one backing URL is configured.
+  return HTTPClient::isAvailable() &&
+         !ExitOnErr(getDefaultDebuginfodUrls()).empty();
+
+  // A debuginfod lookup could also succeed if something were present in the
+  // cache directory, but it would be surprising to enable debuginfod on this
+  // basis alone. To use existing caches in an "offline" fashion, the debuginfod
+  // flag must be set.
+}
+
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
   sys::InitializeCOMRAII COM(sys::COMThreadingMode::MultiThreaded);
@@ -330,10 +345,13 @@
 
   LLVMSymbolizer Symbolizer(Opts);
 
-  // Look up symbols using the debuginfod client.
-  Symbolizer.addDIFetcher(DebuginfodDIFetcher());
-  // The HTTPClient must be initialized for use by the debuginfod client.
-  HTTPClient::initialize();
+  if (Args.hasFlag(OPT_debuginfod, OPT_no_debuginfod,
+                   shouldUseDebuginfodByDefault())) {
+    // Look up symbols using the debuginfod client.
+    Symbolizer.addDIFetcher(DebuginfodDIFetcher());
+    // The HTTPClient must be initialized for use by the debuginfod client.
+    HTTPClient::initialize();
+  }
 
   std::unique_ptr<DIPrinter> Printer;
   if (Style == OutputStyle::GNU)
Index: llvm/tools/llvm-symbolizer/Opts.td
===================================================================
--- llvm/tools/llvm-symbolizer/Opts.td
+++ llvm/tools/llvm-symbolizer/Opts.td
@@ -22,6 +22,7 @@
       MetaVarName<"<offset>">;
 def basenames : Flag<["--"], "basenames">, HelpText<"Strip directory names from paths">;
 defm debug_file_directory : Eq<"debug-file-directory", "Path to directory where to look for debug files">, MetaVarName<"<dir>">;
+defm debuginfod : B<"debuginfod", "Use debuginfod to find debug binaries", "Don't use debuginfod to find debug binaries">;
 defm default_arch
     : Eq<"default-arch", "Default architecture (for multi-arch objects)">,
       Group<grp_mach_o>;
Index: llvm/test/tools/llvm-symbolizer/debuginfod.test
===================================================================
--- llvm/test/tools/llvm-symbolizer/debuginfod.test
+++ llvm/test/tools/llvm-symbolizer/debuginfod.test
@@ -23,5 +23,10 @@
 # The symbolizer should call the debuginfod client library, which finds the
 # debuginfo placed in the cache, enabling symbolization of the address.
 RUN: env DEBUGINFOD_CACHE_PATH=%t llvm-symbolizer \
-RUN:   --obj=%t/addr.exe 0x40054d | FileCheck %s --check-prefix=FOUND
+RUN:   --obj=%t/addr.exe 0x40054d --debuginfod | \
+RUN:   FileCheck %s --check-prefix=FOUND
 FOUND: {{[/\]+}}tmp{{[/\]+}}x.c:14:0
+
+# The symbolizer shouldn't call the debuginfod library by default with no URLs.
+RUN: env DEBUGINFOD_CACHE_PATH=%t llvm-symbolizer --print-address \
+RUN:   --obj=%t/addr.exe 0x40054d | FileCheck %s --check-prefix=NOTFOUND
Index: llvm/docs/CommandGuide/llvm-symbolizer.rst
===================================================================
--- llvm/docs/CommandGuide/llvm-symbolizer.rst
+++ llvm/docs/CommandGuide/llvm-symbolizer.rst
@@ -183,6 +183,13 @@
   Print just the file's name without any directories, instead of the
   absolute path.
 
+.. option:: --debuginfod, --no-debuginfod
+
+  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``.
+
 .. _llvm-symbolizer-opt-C:
 
 .. option:: --demangle, -C


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D118665.405058.patch
Type: text/x-patch
Size: 4261 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220201/90c4c544/attachment.bin>