[Lldb-commits] [lldb] [llvm] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
Kevin Frei via lldb-commits
lldb-commits at lists.llvm.org
Thu Nov 2 10:30:40 PDT 2023
https://github.com/kevinfrei updated https://github.com/llvm/llvm-project/pull/70996
>From 6454d4fb652f61a20850c75f0e69759dffe28511 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Wed, 18 Oct 2023 14:37:34 -0700
Subject: [PATCH 1/2] DEBUGINFOD based DWP acquisition for LLDB
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 Greg Clayton's request, that will
override the env variable, or can be used instead of the env var. This setting is the
reason for the additional API added to the llvm's 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 my implementation.
---
lldb/include/lldb/Target/Target.h | 3 +++
lldb/source/Core/CoreProperties.td | 2 +-
lldb/source/Core/Debugger.cpp | 5 ++++
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 1 +
lldb/source/Symbol/CMakeLists.txt | 1 +
lldb/source/Symbol/LocateSymbolFile.cpp | 20 ++++++++++++--
lldb/source/Target/Target.cpp | 19 +++++++++++++-
lldb/source/Target/TargetProperties.td | 4 +++
llvm/include/llvm/Debuginfod/Debuginfod.h | 4 +++
llvm/lib/Debuginfod/Debuginfod.cpp | 26 ++++++++++++++-----
10 files changed, 75 insertions(+), 10 deletions(-)
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() {
>From 7409896894f05361df2ba1159a9fd2b295d4a4c3 Mon Sep 17 00:00:00 2001
From: Kevin Frei <kevinfrei at users.noreply.github.com>
Date: Thu, 2 Nov 2023 10:30:33 -0700
Subject: [PATCH 2/2] Update lldb/source/Symbol/LocateSymbolFile.cpp
Minor clean up suggested by @bulbazord
Co-authored-by: Alex Langford <nirvashtzero at gmail.com>
---
lldb/source/Symbol/LocateSymbolFile.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/lldb/source/Symbol/LocateSymbolFile.cpp b/lldb/source/Symbol/LocateSymbolFile.cpp
index 907287f4b4100b8..1da6d37136b4d13 100644
--- a/lldb/source/Symbol/LocateSymbolFile.cpp
+++ b/lldb/source/Symbol/LocateSymbolFile.cpp
@@ -398,8 +398,7 @@ Symbols::LocateExecutableSymbolFile(const ModuleSpec &module_spec,
}
}
}
- FileSpec dsym_bundle = LocateExecutableSymbolFileDsym(module_spec);
- if (dsym_bundle)
+ if (FileSpec dsym_bundle = LocateExecutableSymbolFileDsym(module_spec))
return dsym_bundle;
// If we didn't find anything by looking locally, let's try Debuginfod.
More information about the lldb-commits
mailing list