[lldb] [llvm] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)

via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 1 15:23:15 PDT 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-debuginfo

Author: Kevin Frei (kevinfrei)

<details>
<summary>Changes</summary>

I've plumbed the LLVM DebugInfoD client into LLDB, and added automatic downloading of DWP files to the SymbolFileDWARF.cpp plugin. If you have DEBUGINFOD_URLS set to a space delimited set of web servers, LLDB will try to use them as a last resort when searching for DWP files. If you do *not* have that environment variable set, nothing should be changed. There's also a setting, per @<!-- -->clayborg 's suggestion, that will override the environment variable, or can be used instead of the environment variable. The setting is why I also needed to add an API to the llvm-debuginfod library

### Test Plan:

Suggestions are welcome here. I should probably have some positive and negative tests, but I wanted to get the diff up for people who have a clue what they're doing to rip it to pieces before spending too much time validating the initial implementation.

---
Full diff: https://github.com/llvm/llvm-project/pull/70996.diff


10 Files Affected:

- (modified) lldb/include/lldb/Target/Target.h (+3) 
- (modified) lldb/source/Core/CoreProperties.td (+1-1) 
- (modified) lldb/source/Core/Debugger.cpp (+5) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+1) 
- (modified) lldb/source/Symbol/CMakeLists.txt (+1) 
- (modified) lldb/source/Symbol/LocateSymbolFile.cpp (+18-2) 
- (modified) lldb/source/Target/Target.cpp (+18-1) 
- (modified) lldb/source/Target/TargetProperties.td (+4) 
- (modified) llvm/include/llvm/Debuginfod/Debuginfod.h (+4) 
- (modified) llvm/lib/Debuginfod/Debuginfod.cpp (+20-6) 


``````````diff
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 82045988018b606..cd5c88767c900d1 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -258,6 +258,8 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+  Args GetDebugInfoDURLs() const;
+
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();
@@ -270,6 +272,7 @@ class TargetProperties : public Properties {
   void DisableASLRValueChangedCallback();
   void InheritTCCValueChangedCallback();
   void DisableSTDIOValueChangedCallback();
+  void DebugInfoDURLsChangedCallback();
 
   // Settings checker for target.jit-save-objects-dir:
   void CheckJITObjectsDir();
diff --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td
index 92884258347e9be..865030b0133bbb2 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -4,7 +4,7 @@ let Definition = "modulelist" in {
   def EnableExternalLookup: Property<"enable-external-lookup", "Boolean">,
     Global,
     DefaultTrue,
-    Desc<"Control the use of external tools and repositories to locate symbol files. Directories listed in target.debug-file-search-paths and directory of the executable are always checked first for separate debug info files. Then depending on this setting: On macOS, Spotlight would be also used to locate a matching .dSYM bundle based on the UUID of the executable. On NetBSD, directory /usr/libdata/debug would be also searched. On platforms other than NetBSD directory /usr/lib/debug would be also searched.">;
+    Desc<"Control the use of external tools and repositories to locate symbol files. Directories listed in target.debug-file-search-paths and directory of the executable are always checked first for separate debug info files. Then depending on this setting: On macOS, Spotlight would be also used to locate a matching .dSYM bundle based on the UUID of the executable. On NetBSD, directory /usr/libdata/debug would be also searched. On platforms other than NetBSD directory /usr/lib/debug would be also searched. If all other methods fail, and the DEBUGINFOD_URLS environment variable is specified, the Debuginfod protocol is used to acquire symbols from a compatible Debuginfod service.">;
   def EnableBackgroundLookup: Property<"enable-background-lookup", "Boolean">,
     Global,
     DefaultFalse,
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 21f71e449ca5ed0..9a3e82f3e6a2adf 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -61,6 +61,8 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator.h"
+#include "llvm/Debuginfod/Debuginfod.h"
+#include "llvm/Debuginfod/HTTPClient.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Process.h"
@@ -594,6 +596,9 @@ lldb::DWIMPrintVerbosity Debugger::GetDWIMPrintVerbosity() const {
 void Debugger::Initialize(LoadPluginCallbackType load_plugin_callback) {
   assert(g_debugger_list_ptr == nullptr &&
          "Debugger::Initialize called more than once!");
+  // We might be using the Debuginfod service, so we have to initialize the
+  // HTTPClient *before* any new threads start.
+  llvm::HTTPClient::initialize();
   g_debugger_list_mutex_ptr = new std::recursive_mutex();
   g_debugger_list_ptr = new DebuggerList();
   g_thread_pool = new llvm::ThreadPool(llvm::optimal_concurrency());
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index ee7164d2f050ed1..c036963a1ec6e87 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4325,6 +4325,7 @@ const std::shared_ptr<SymbolFileDWARFDwo> &SymbolFileDWARF::GetDwpSymbolFile() {
     module_spec.GetSymbolFileSpec() =
         FileSpec(m_objfile_sp->GetModule()->GetFileSpec().GetPath() + ".dwp");
 
+    module_spec.GetUUID() = m_objfile_sp->GetUUID();
     FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
     FileSpec dwp_filespec =
         Symbols::LocateExecutableSymbolFile(module_spec, search_paths);
diff --git a/lldb/source/Symbol/CMakeLists.txt b/lldb/source/Symbol/CMakeLists.txt
index cec49b8b2cb4b63..91569f103cf86c8 100644
--- a/lldb/source/Symbol/CMakeLists.txt
+++ b/lldb/source/Symbol/CMakeLists.txt
@@ -48,6 +48,7 @@ add_lldb_library(lldbSymbol NO_PLUGIN_DEPENDENCIES
     lldbHost
     lldbTarget
     lldbUtility
+    LLVMDebuginfod
 
   LINK_COMPONENTS
     Support
diff --git a/lldb/source/Symbol/LocateSymbolFile.cpp b/lldb/source/Symbol/LocateSymbolFile.cpp
index 66ee7589ac60499..907287f4b4100b8 100644
--- a/lldb/source/Symbol/LocateSymbolFile.cpp
+++ b/lldb/source/Symbol/LocateSymbolFile.cpp
@@ -25,6 +25,8 @@
 #include "lldb/Utility/UUID.h"
 
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/Debuginfod/Debuginfod.h"
+#include "llvm/Object/BuildID.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ThreadPool.h"
 
@@ -396,8 +398,22 @@ Symbols::LocateExecutableSymbolFile(const ModuleSpec &module_spec,
       }
     }
   }
-
-  return LocateExecutableSymbolFileDsym(module_spec);
+  FileSpec dsym_bundle = LocateExecutableSymbolFileDsym(module_spec);
+  if (dsym_bundle)
+    return dsym_bundle;
+
+  // If we didn't find anything by looking locally, let's try Debuginfod.
+  if (module_uuid.IsValid() && llvm::canUseDebuginfod()) {
+    llvm::object::BuildID build_id(module_uuid.GetBytes());
+    llvm::Expected<std::string> result =
+        llvm::getCachedOrDownloadDebuginfo(build_id);
+    if (result)
+      return FileSpec(*result);
+    // An error is just fine, here...
+    consumeError(result.takeError());
+  }
+  // Just return the empty FileSpec if nothing was found.
+  return dsym_bundle;
 }
 
 void Symbols::DownloadSymbolFileAsync(const UUID &uuid) {
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 5f8756c57675c95..1c0ead3677ea386 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -65,6 +65,7 @@
 
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/Debuginfod/Debuginfod.h"
 
 #include <memory>
 #include <mutex>
@@ -4180,7 +4181,8 @@ TargetProperties::TargetProperties(Target *target)
         ePropertyInheritTCC, [this] { InheritTCCValueChangedCallback(); });
     m_collection_sp->SetValueChangedCallback(
         ePropertyDisableSTDIO, [this] { DisableSTDIOValueChangedCallback(); });
-
+    m_collection_sp->SetValueChangedCallback(
+        ePropertyDebugInfoDURLs, [this] { DebugInfoDURLsChangedCallback(); });
     m_collection_sp->SetValueChangedCallback(
         ePropertySaveObjectsDir, [this] { CheckJITObjectsDir(); });
     m_experimental_properties_up =
@@ -4892,6 +4894,21 @@ void TargetProperties::SetDebugUtilityExpression(bool debug) {
   SetPropertyAtIndex(idx, debug);
 }
 
+Args TargetProperties::GetDebugInfoDURLs() const {
+  Args urls;
+  m_collection_sp->GetPropertyAtIndexAsArgs(ePropertyDebugInfoDURLs, urls);
+  return urls;
+}
+
+void TargetProperties::DebugInfoDURLsChangedCallback() {
+  Args urls = GetDebugInfoDURLs();
+  llvm::SmallVector<llvm::StringRef> dbginfod_urls;
+  std::transform(urls.begin(), urls.end(), dbginfod_urls.end(),
+                 [](const auto &obj) { return obj.ref(); });
+  llvm::setDefaultDebuginfodUrls(dbginfod_urls);
+}
+
+
 // Target::TargetEventData
 
 Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp)
diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td
index 154a6e5919ab0cd..c21c9d86c416c34 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -195,6 +195,10 @@ let Definition = "target" in {
   def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">,
     DefaultFalse,
     Desc<"Enable debugging of LLDB-internal utility expressions.">;
+  def DebugInfoDURLs: Property<"debuginfod-urls", "Array">,
+    Global,
+    ElementType<"String">,
+    Desc<"A list valid debuginfod server URLs that can be used to locate symbol files.">;
 }
 
 let Definition = "process_experimental" in {
diff --git a/llvm/include/llvm/Debuginfod/Debuginfod.h b/llvm/include/llvm/Debuginfod/Debuginfod.h
index ec7f5691dda4fbf..9351af27cc5fe2c 100644
--- a/llvm/include/llvm/Debuginfod/Debuginfod.h
+++ b/llvm/include/llvm/Debuginfod/Debuginfod.h
@@ -46,6 +46,10 @@ bool canUseDebuginfod();
 /// environment variable.
 SmallVector<StringRef> getDefaultDebuginfodUrls();
 
+/// Sets the list of debuginfod server URLs to query. This overrides the
+/// environment variable DEBUGINFOD_URLS.
+void setDefaultDebuginfodUrls(SmallVector<StringRef> URLs);
+
 /// Finds a default local file caching directory for the debuginfod client,
 /// first checking DEBUGINFOD_CACHE_PATH.
 Expected<std::string> getDefaultDebuginfodCacheDirectory();
diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp
index fa4c1a0499f059e..a74dfc5900cdaf5 100644
--- a/llvm/lib/Debuginfod/Debuginfod.cpp
+++ b/llvm/lib/Debuginfod/Debuginfod.cpp
@@ -47,6 +47,10 @@ namespace llvm {
 
 using llvm::object::BuildIDRef;
 
+SmallVector<StringRef> DebuginfodUrls;
+
+bool DebuginfodUrlsSet = false;
+
 static std::string uniqueKey(llvm::StringRef S) {
   return utostr(xxh3_64bits(S));
 }
@@ -62,15 +66,25 @@ bool canUseDebuginfod() {
 }
 
 SmallVector<StringRef> getDefaultDebuginfodUrls() {
-  const char *DebuginfodUrlsEnv = std::getenv("DEBUGINFOD_URLS");
-  if (DebuginfodUrlsEnv == nullptr)
-    return SmallVector<StringRef>();
-
-  SmallVector<StringRef> DebuginfodUrls;
-  StringRef(DebuginfodUrlsEnv).split(DebuginfodUrls, " ");
+  if (!DebuginfodUrlsSet) {
+    // Only read from the environment variable if the user hasn't already
+    // set the value
+    const char *DebuginfodUrlsEnv = std::getenv("DEBUGINFOD_URLS");
+    if (DebuginfodUrlsEnv != nullptr) {
+      StringRef(DebuginfodUrlsEnv).split(DebuginfodUrls, " ", -1, false);
+    }
+    DebuginfodUrlsSet = true;
+  }
   return DebuginfodUrls;
 }
 
+// Override the default debuginfod URL list.
+void setDefaultDebuginfodUrls(SmallVector<StringRef> URLs) {
+  DebuginfodUrls.clear();
+  DebuginfodUrls.insert(DebuginfodUrls.begin(), URLs.begin(), URLs.end());
+  DebuginfodUrlsSet = true;
+}
+
 /// Finds a default local file caching directory for the debuginfod client,
 /// first checking DEBUGINFOD_CACHE_PATH.
 Expected<std::string> getDefaultDebuginfodCacheDirectory() {

``````````

</details>


https://github.com/llvm/llvm-project/pull/70996


More information about the llvm-commits mailing list