[lldb] [llvm] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
Kevin Frei via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 29 17:05:13 PST 2023
https://github.com/kevinfrei updated https://github.com/llvm/llvm-project/pull/70996
>From b04c85dbed0b369e747aa2a3823789203156736b 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/7] 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/Target/Target.cpp | 19 +++++++++++++-
lldb/source/Target/TargetProperties.td | 4 +++
llvm/include/llvm/Debuginfod/Debuginfod.h | 4 +++
llvm/lib/Debuginfod/Debuginfod.cpp | 26 ++++++++++++++-----
9 files changed, 57 insertions(+), 8 deletions(-)
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index c37682e2a03859f..7f10f0409fb1315 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 aabd83a194932ff..cfe18c8528dce9b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4326,6 +4326,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 =
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
diff --git a/lldb/source/Symbol/CMakeLists.txt b/lldb/source/Symbol/CMakeLists.txt
index ad3488dcdfceced..702e6be4c7c8c8d 100644
--- a/lldb/source/Symbol/CMakeLists.txt
+++ b/lldb/source/Symbol/CMakeLists.txt
@@ -38,6 +38,7 @@ add_lldb_library(lldbSymbol NO_PLUGIN_DEPENDENCIES
lldbHost
lldbTarget
lldbUtility
+ LLVMDebuginfod
LINK_COMPONENTS
Support
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index a6d7148c84e20be..901ee411f584110 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 2e60ae5f30a0da802d1752d2e013aa1ccfaff20b Mon Sep 17 00:00:00 2001
From: Kevin Frei <kevinfrei at hotmail.com>
Date: Thu, 2 Nov 2023 16:22:10 -0700
Subject: [PATCH 2/7] All the non-contentious, obvious changes from PR feedback
are in
---
lldb/include/lldb/Target/Target.h | 4 ++--
lldb/source/Target/Target.cpp | 12 ++++++------
lldb/source/Target/TargetProperties.td | 2 +-
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 7f10f0409fb1315..183aaa19599153f 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -258,7 +258,7 @@ class TargetProperties : public Properties {
bool GetDebugUtilityExpression() const;
- Args GetDebugInfoDURLs() const;
+ Args GetDebuginfodURLs() const;
private:
// Callbacks for m_launch_info.
@@ -272,7 +272,7 @@ class TargetProperties : public Properties {
void DisableASLRValueChangedCallback();
void InheritTCCValueChangedCallback();
void DisableSTDIOValueChangedCallback();
- void DebugInfoDURLsChangedCallback();
+ void DebuginfodURLsChangedCallback();
// Settings checker for target.jit-save-objects-dir:
void CheckJITObjectsDir();
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 901ee411f584110..a28fcc699a7dca2 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -4182,7 +4182,7 @@ TargetProperties::TargetProperties(Target *target)
m_collection_sp->SetValueChangedCallback(
ePropertyDisableSTDIO, [this] { DisableSTDIOValueChangedCallback(); });
m_collection_sp->SetValueChangedCallback(
- ePropertyDebugInfoDURLs, [this] { DebugInfoDURLsChangedCallback(); });
+ ePropertyDebuginfodURLs, [this] { DebuginfodURLsChangedCallback(); });
m_collection_sp->SetValueChangedCallback(
ePropertySaveObjectsDir, [this] { CheckJITObjectsDir(); });
m_experimental_properties_up =
@@ -4894,16 +4894,16 @@ void TargetProperties::SetDebugUtilityExpression(bool debug) {
SetPropertyAtIndex(idx, debug);
}
-Args TargetProperties::GetDebugInfoDURLs() const {
+Args TargetProperties::GetDebuginfodURLs() const {
Args urls;
- m_collection_sp->GetPropertyAtIndexAsArgs(ePropertyDebugInfoDURLs, urls);
+ m_collection_sp->GetPropertyAtIndexAsArgs(ePropertyDebuginfodURLs, urls);
return urls;
}
-void TargetProperties::DebugInfoDURLsChangedCallback() {
- Args urls = GetDebugInfoDURLs();
+void TargetProperties::DebuginfodURLsChangedCallback() {
+ Args urls = GetDebuginfodURLs();
llvm::SmallVector<llvm::StringRef> dbginfod_urls;
- std::transform(urls.begin(), urls.end(), dbginfod_urls.end(),
+ llvm::transform(urls, dbginfod_urls.end(),
[](const auto &obj) { return obj.ref(); });
llvm::setDefaultDebuginfodUrls(dbginfod_urls);
}
diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td
index c21c9d86c416c34..bfbd27bffb3c898 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -195,7 +195,7 @@ 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">,
+ def DebuginfodURLs: Property<"debuginfod-urls", "Array">,
Global,
ElementType<"String">,
Desc<"A list valid debuginfod server URLs that can be used to locate symbol files.">;
>From cddb3839115a7451cd157ea44d8910baee48931a Mon Sep 17 00:00:00 2001
From: Kevin Frei <kevinfrei at hotmail.com>
Date: Wed, 8 Nov 2023 13:57:35 -0800
Subject: [PATCH 3/7] Moved to a plugin model
---
.../Plugins/SymbolLocator/CMakeLists.txt | 1 +
.../SymbolLocator/Debuginfod/CMakeLists.txt | 21 +++
.../Debuginfod/SymbolLocatorDebuginfod.cpp | 154 ++++++++++++++++++
.../Debuginfod/SymbolLocatorDebuginfod.h | 64 ++++++++
.../SymbolLocatorDebuginfodProperties.td | 8 +
5 files changed, 248 insertions(+)
create mode 100644 lldb/source/Plugins/SymbolLocator/Debuginfod/CMakeLists.txt
create mode 100644 lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
create mode 100644 lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.h
create mode 100644 lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td
diff --git a/lldb/source/Plugins/SymbolLocator/CMakeLists.txt b/lldb/source/Plugins/SymbolLocator/CMakeLists.txt
index 74abecd79694902..ca969626f4ffc4a 100644
--- a/lldb/source/Plugins/SymbolLocator/CMakeLists.txt
+++ b/lldb/source/Plugins/SymbolLocator/CMakeLists.txt
@@ -2,3 +2,4 @@ add_subdirectory(Default)
if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
add_subdirectory(DebugSymbols)
endif()
+add_subdirectory(Debuginfod)
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/CMakeLists.txt b/lldb/source/Plugins/SymbolLocator/Debuginfod/CMakeLists.txt
new file mode 100644
index 000000000000000..f07e93e131376a2
--- /dev/null
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/CMakeLists.txt
@@ -0,0 +1,21 @@
+lldb_tablegen(SymbolLocatorDebuginfodProperties.inc -gen-lldb-property-defs
+ SOURCE SymbolLocatorDebuginfodProperties.td
+ TARGET LLDBPluginSymbolLocatorDebuginfodPropertiesGen)
+
+lldb_tablegen(SymbolLocatorDebuginfodPropertiesEnum.inc -gen-lldb-property-enum-defs
+ SOURCE SymbolLocatorDebuginfodProperties.td
+ TARGET LLDBPluginSymbolLocatorDebuginfodPropertiesEnumGen)
+
+add_lldb_library(lldbPluginSymbolLocatorDebuginfod PLUGIN
+ SymbolLocatorDebuginfod.cpp
+
+ LINK_LIBS
+ lldbCore
+ lldbHost
+ lldbSymbol
+ LLVMDebuginfod
+ )
+
+add_dependencies(lldbPluginSymbolLocatorDebuginfod
+ LLDBPluginSymbolLocatorDebuginfodPropertiesGen
+ LLDBPluginSymbolLocatorDebuginfodPropertiesEnumGen)
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
new file mode 100644
index 000000000000000..b8245cc122e61bd
--- /dev/null
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -0,0 +1,154 @@
+//===-- SymbolLocatorDebuginfod.cpp ------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "SymbolLocatorDebuginfod.h"
+
+#include <cstring>
+#include <optional>
+
+#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/Host.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/StreamString.h"
+#include "lldb/Utility/Timer.h"
+#include "lldb/Utility/UUID.h"
+
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/Debuginfod/HTTPClient.h"
+#include "llvm/Debuginfod/Debuginfod.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/ThreadPool.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+LLDB_PLUGIN_DEFINE(SymbolLocatorDebuginfod)
+
+namespace {
+
+#define LLDB_PROPERTIES_symbollocatordebuginfod
+#include "SymbolLocatorDebuginfodProperties.inc"
+
+enum {
+#define LLDB_PROPERTIES_symbollocatordebuginfod
+#include "SymbolLocatorDebuginfodPropertiesEnum.inc"
+};
+
+class PluginProperties : public Properties {
+public:
+ static llvm::StringRef GetSettingName() {
+ return SymbolLocatorDebuginfod::GetPluginNameStatic();
+ }
+
+ PluginProperties() {
+ m_collection_sp = std::make_shared<OptionValueProperties>(GetSettingName());
+ m_collection_sp->Initialize(g_symbollocatordebuginfod_properties);
+ m_collection_sp->SetValueChangedCallback(
+ ePropertyURLs, [this] { URLsChangedCallback(); }
+ );
+ }
+
+ Args GetDebugInfoDURLs() const {
+ Args urls;
+ m_collection_sp->GetPropertyAtIndexAsArgs(ePropertyURLs, urls);
+ return urls;
+ }
+
+private:
+ void URLsChangedCallback() {
+ Args urls = GetDebugInfoDURLs();
+ llvm::SmallVector<llvm::StringRef> dbginfod_urls;
+ llvm::transform(urls, dbginfod_urls.end(),
+ [](const auto &obj) { return obj.ref(); });
+ llvm::setDefaultDebuginfodUrls(dbginfod_urls);
+ }
+};
+
+} // namespace
+
+
+SymbolLocatorDebuginfod::SymbolLocatorDebuginfod() : SymbolLocator() {}
+
+void SymbolLocatorDebuginfod::Initialize() {
+ PluginManager::RegisterPlugin(
+ GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance,
+ LocateExecutableObjectFile, LocateExecutableSymbolFile,
+ DownloadObjectAndSymbolFile);
+ // There's a "safety" concern on this:
+ // Does plugin initialization occur while things are still single threaded?
+ llvm::HTTPClient::initialize();
+}
+
+void SymbolLocatorDebuginfod::Terminate() {
+ PluginManager::UnregisterPlugin(CreateInstance);
+ // There's a "safety" concern on this:
+ // Does plugin termination occur while things are still single threaded?
+ llvm::HTTPClient::cleanup();
+}
+
+llvm::StringRef SymbolLocatorDebuginfod::GetPluginDescriptionStatic() {
+ return "Debuginfod symbol locator.";
+}
+
+SymbolLocator *SymbolLocatorDebuginfod::CreateInstance() {
+ return new SymbolLocatorDebuginfod();
+}
+
+std::optional<ModuleSpec> SymbolLocatorDebuginfod::LocateExecutableObjectFile(
+ const ModuleSpec &module_spec) {
+ const UUID &module_uuid = module_spec.GetUUID();
+ if (module_uuid.IsValid() && llvm::canUseDebuginfod()) {
+ llvm::object::BuildID build_id(module_uuid.GetBytes());
+ llvm::Expected<std::string> result =
+ llvm::getCachedOrDownloadExecutable(build_id);
+ if (result)
+ return FileSpec(*result);
+ // An error here should be logged as a failure in the Debuginfod library,
+ // so just consume it here
+ consumeError(result.takeError());
+ }
+ return {};
+}
+
+std::optional<FileSpec> SymbolLocatorDebuginfod::LocateExecutableSymbolFile(
+ const ModuleSpec &module_spec, const FileSpecList &default_search_paths) {
+ const UUID &module_uuid = module_spec.GetUUID();
+ 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 here should be logged as a failure in the Debuginfod library,
+ // so just consume it here
+ consumeError(result.takeError());
+ }
+ return {};
+}
+
+bool SymbolLocatorDebuginfod::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
+ Status &error,
+ bool force_lookup,
+ bool copy_executable) {
+ // TODO: Fill this with the appropriate Debuginfod stuff
+ return false;
+}
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.h b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.h
new file mode 100644
index 000000000000000..d6c6c1d01555f94
--- /dev/null
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.h
@@ -0,0 +1,64 @@
+//===-- SymbolLocatorDebuginfod.h ----------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_SOURCE_PLUGINS_SYMBOLLOCATOR_DEBUGINFOD_SYMBOLLOCATORDEBUGINFOD_H
+#define LLDB_SOURCE_PLUGINS_SYMBOLLOCATOR_DEBUGINFOD_SYMBOLLOCATORDEBUGINFOD_H
+
+#include "lldb/Symbol/SymbolLocator.h"
+#include "lldb/lldb-private.h"
+
+namespace lldb_private {
+
+class SymbolLocatorDebuginfod : public SymbolLocator {
+public:
+ SymbolLocatorDebuginfod();
+
+ static void Initialize();
+ static void Terminate();
+
+ static llvm::StringRef GetPluginNameStatic() { return "Debuginfod"; }
+ static llvm::StringRef GetPluginDescriptionStatic();
+
+ static lldb_private::SymbolLocator *CreateInstance();
+
+ /// PluginInterface protocol.
+ /// \{
+ llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
+ /// \}
+
+ // Locate the executable file given a module specification.
+ //
+ // Locating the file should happen only on the local computer or using the
+ // current computers global settings.
+ static std::optional<ModuleSpec>
+ LocateExecutableObjectFile(const ModuleSpec &module_spec);
+
+ // Locate the symbol file given a module specification.
+ //
+ // Locating the file should happen only on the local computer or using the
+ // current computers global settings.
+ static std::optional<FileSpec>
+ LocateExecutableSymbolFile(const ModuleSpec &module_spec,
+ const FileSpecList &default_search_paths);
+
+ // Locate the object and symbol file given a module specification.
+ //
+ // Locating the file can try to download the file from a corporate build
+ // repository, or using any other means necessary to locate both the
+ // unstripped object file and the debug symbols. The force_lookup argument
+ // controls whether the external program is called unconditionally to find
+ // the symbol file, or if the user's settings are checked to see if they've
+ // enabled the external program before calling.
+ static bool DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
+ Status &error, bool force_lookup,
+ bool copy_executable);
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_SYMBOLLOCATOR_DEBUGINFOD_SYMBOLLOCATORDEBUGINFOD_H
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td
new file mode 100644
index 000000000000000..c16d765ff761342
--- /dev/null
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td
@@ -0,0 +1,8 @@
+include "../../../../include/lldb/Core/PropertiesBase.td"
+
+let Definition = "symbollocatordebuginfod" in {
+ def URLs: Property<"urls", "String">,
+ Global,
+ DefaultStringValue<"">,
+ Desc<"A space-separated, ordered list of Debuginfod server URLs to query for symbols">;
+}
>From 27f526a9c198eeb2496dbd27e266dc94a379b977 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 9 Nov 2023 16:43:40 -0800
Subject: [PATCH 4/7] Cleaned up leftovers from pre-plugin work
---
lldb/include/lldb/Target/Target.h | 3 ---
lldb/source/Core/Debugger.cpp | 5 -----
.../Debuginfod/SymbolLocatorDebuginfod.cpp | 2 +-
lldb/source/Target/Target.cpp | 18 ------------------
lldb/source/Target/TargetProperties.td | 4 ----
5 files changed, 1 insertion(+), 31 deletions(-)
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 183aaa19599153f..c37682e2a03859f 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -258,8 +258,6 @@ class TargetProperties : public Properties {
bool GetDebugUtilityExpression() const;
- Args GetDebuginfodURLs() const;
-
private:
// Callbacks for m_launch_info.
void Arg0ValueChangedCallback();
@@ -272,7 +270,6 @@ 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/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 9a3e82f3e6a2adf..21f71e449ca5ed0 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -61,8 +61,6 @@
#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"
@@ -596,9 +594,6 @@ 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/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index b8245cc122e61bd..c66549f65050cbc 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -149,6 +149,6 @@ bool SymbolLocatorDebuginfod::DownloadObjectAndSymbolFile(ModuleSpec &module_spe
Status &error,
bool force_lookup,
bool copy_executable) {
- // TODO: Fill this with the appropriate Debuginfod stuff
+ // TODO: Continue to add more Debuginfod capabilities
return false;
}
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index a28fcc699a7dca2..afcf4c02c3599b6 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -65,7 +65,6 @@
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SetVector.h"
-#include "llvm/Debuginfod/Debuginfod.h"
#include <memory>
#include <mutex>
@@ -4181,8 +4180,6 @@ 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 =
@@ -4894,21 +4891,6 @@ 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;
- llvm::transform(urls, 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 bfbd27bffb3c898..154a6e5919ab0cd 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -195,10 +195,6 @@ 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 {
>From e9b624096ef34c4be150fbb0631172bb37324ee1 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Fri, 10 Nov 2023 16:26:28 -0800
Subject: [PATCH 5/7] The server-urls property appears to be wired up correctly
---
lldb/include/lldb/Core/PluginManager.h | 11 ++-
lldb/source/Core/CoreProperties.td | 2 +-
lldb/source/Core/PluginManager.cpp | 28 ++++--
.../Debuginfod/SymbolLocatorDebuginfod.cpp | 86 ++++++++-----------
.../Debuginfod/SymbolLocatorDebuginfod.h | 18 +---
.../SymbolLocatorDebuginfodProperties.td | 7 +-
lldb/source/Symbol/CMakeLists.txt | 1 -
lldb/source/Target/Target.cpp | 1 +
llvm/lib/Debuginfod/Debuginfod.cpp | 2 +-
9 files changed, 80 insertions(+), 76 deletions(-)
diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h
index 318f8b63c251a19..f2296e29202384c 100644
--- a/lldb/include/lldb/Core/PluginManager.h
+++ b/lldb/include/lldb/Core/PluginManager.h
@@ -355,7 +355,8 @@ class PluginManager {
nullptr,
SymbolLocatorDownloadObjectAndSymbolFile download_object_symbol_file =
nullptr,
- SymbolLocatorFindSymbolFileInBundle find_symbol_file_in_bundle = nullptr);
+ SymbolLocatorFindSymbolFileInBundle find_symbol_file_in_bundle = nullptr,
+ DebuggerInitializeCallback debugger_init_callback = nullptr);
static bool UnregisterPlugin(SymbolLocatorCreateInstance create_callback);
@@ -528,6 +529,14 @@ class PluginManager {
Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp,
llvm::StringRef description, bool is_global_property);
+ static lldb::OptionValuePropertiesSP
+ GetSettingForSymbolLocatorPlugin(Debugger &debugger,
+ llvm::StringRef setting_name);
+
+ static bool CreateSettingForSymbolLocatorPlugin(
+ Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp,
+ llvm::StringRef description, bool is_global_property);
+
static bool CreateSettingForTracePlugin(
Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp,
llvm::StringRef description, bool is_global_property);
diff --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td
index 865030b0133bbb2..0e0f468d3ecd764 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. 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.">;
+ 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 there may be symbol-locator plugins that, if configured properly, will also attempt to acquire symbols. The debuginfod plugin defaults to the DEGUFINFOD_URLS environment variable which is configurable through the 'plugin.symbol-locator.debuginfod.server_urls' setting.">;
def EnableBackgroundLookup: Property<"enable-background-lookup", "Boolean">,
Global,
DefaultFalse,
diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp
index 23c06357e2f95f3..dea380e47f4eed6 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -1091,9 +1091,10 @@ struct SymbolLocatorInstance
SymbolLocatorLocateExecutableObjectFile locate_executable_object_file,
SymbolLocatorLocateExecutableSymbolFile locate_executable_symbol_file,
SymbolLocatorDownloadObjectAndSymbolFile download_object_symbol_file,
- SymbolLocatorFindSymbolFileInBundle find_symbol_file_in_bundle)
- : PluginInstance<SymbolLocatorCreateInstance>(name, description,
- create_callback),
+ SymbolLocatorFindSymbolFileInBundle find_symbol_file_in_bundle,
+ DebuggerInitializeCallback debugger_init_callback)
+ : PluginInstance<SymbolLocatorCreateInstance>(
+ name, description, create_callback, debugger_init_callback),
locate_executable_object_file(locate_executable_object_file),
locate_executable_symbol_file(locate_executable_symbol_file),
download_object_symbol_file(download_object_symbol_file),
@@ -1117,11 +1118,12 @@ bool PluginManager::RegisterPlugin(
SymbolLocatorLocateExecutableObjectFile locate_executable_object_file,
SymbolLocatorLocateExecutableSymbolFile locate_executable_symbol_file,
SymbolLocatorDownloadObjectAndSymbolFile download_object_symbol_file,
- SymbolLocatorFindSymbolFileInBundle find_symbol_file_in_bundle) {
+ SymbolLocatorFindSymbolFileInBundle find_symbol_file_in_bundle,
+ DebuggerInitializeCallback debugger_init_callback) {
return GetSymbolLocatorInstances().RegisterPlugin(
name, description, create_callback, locate_executable_object_file,
locate_executable_symbol_file, download_object_symbol_file,
- find_symbol_file_in_bundle);
+ find_symbol_file_in_bundle, debugger_init_callback);
}
bool PluginManager::UnregisterPlugin(
@@ -1533,6 +1535,7 @@ void PluginManager::DebuggerInitialize(Debugger &debugger) {
GetPlatformInstances().PerformDebuggerCallback(debugger);
GetProcessInstances().PerformDebuggerCallback(debugger);
GetSymbolFileInstances().PerformDebuggerCallback(debugger);
+ GetSymbolLocatorInstances().PerformDebuggerCallback(debugger);
GetOperatingSystemInstances().PerformDebuggerCallback(debugger);
GetStructuredDataPluginInstances().PerformDebuggerCallback(debugger);
GetTracePluginInstances().PerformDebuggerCallback(debugger);
@@ -1660,6 +1663,7 @@ static constexpr llvm::StringLiteral kProcessPluginName("process");
static constexpr llvm::StringLiteral kTracePluginName("trace");
static constexpr llvm::StringLiteral kObjectFilePluginName("object-file");
static constexpr llvm::StringLiteral kSymbolFilePluginName("symbol-file");
+static constexpr llvm::StringLiteral kSymbolLocatorPluginName("symbol-locator");
static constexpr llvm::StringLiteral kJITLoaderPluginName("jit-loader");
static constexpr llvm::StringLiteral
kStructuredDataPluginName("structured-data");
@@ -1708,6 +1712,20 @@ bool PluginManager::CreateSettingForProcessPlugin(
description, is_global_property);
}
+lldb::OptionValuePropertiesSP
+PluginManager::GetSettingForSymbolLocatorPlugin(Debugger &debugger,
+ llvm::StringRef setting_name) {
+ return GetSettingForPlugin(debugger, setting_name, kSymbolLocatorPluginName);
+}
+
+bool PluginManager::CreateSettingForSymbolLocatorPlugin(
+ Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp,
+ llvm::StringRef description, bool is_global_property) {
+ return CreateSettingForPlugin(debugger, kSymbolLocatorPluginName,
+ "Settings for symbol locator plug-ins",
+ properties_sp, description, is_global_property);
+}
+
bool PluginManager::CreateSettingForTracePlugin(
Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp,
llvm::StringRef description, bool is_global_property) {
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index c66549f65050cbc..bc8584dbda8cee6 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -1,4 +1,4 @@
-//===-- SymbolLocatorDebuginfod.cpp ------------------------------------------===//
+//===-- SymbolLocatorDebuginfod.cpp ---------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -8,35 +8,11 @@
#include "SymbolLocatorDebuginfod.h"
-#include <cstring>
-#include <optional>
-
-#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
-#include "lldb/Core/Debugger.h"
-#include "lldb/Core/Module.h"
-#include "lldb/Core/ModuleList.h"
-#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/PluginManager.h"
-#include "lldb/Core/Progress.h"
-#include "lldb/Core/Section.h"
-#include "lldb/Host/FileSystem.h"
-#include "lldb/Host/Host.h"
-#include "lldb/Symbol/ObjectFile.h"
-#include "lldb/Target/Target.h"
-#include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/DataBuffer.h"
-#include "lldb/Utility/DataExtractor.h"
-#include "lldb/Utility/LLDBLog.h"
-#include "lldb/Utility/Log.h"
-#include "lldb/Utility/StreamString.h"
-#include "lldb/Utility/Timer.h"
-#include "lldb/Utility/UUID.h"
-
-#include "llvm/ADT/SmallSet.h"
-#include "llvm/Debuginfod/HTTPClient.h"
+#include "lldb/Utility/Args.h"
+
#include "llvm/Debuginfod/Debuginfod.h"
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/ThreadPool.h"
+#include "llvm/Debuginfod/HTTPClient.h"
using namespace lldb;
using namespace lldb_private;
@@ -62,19 +38,24 @@ class PluginProperties : public Properties {
PluginProperties() {
m_collection_sp = std::make_shared<OptionValueProperties>(GetSettingName());
m_collection_sp->Initialize(g_symbollocatordebuginfod_properties);
+
+ // We need to read the default value first to read the environment variable.
+ llvm::SmallVector<llvm::StringRef> urls = llvm::getDefaultDebuginfodUrls();
+ Args arg_urls{urls};
+ m_collection_sp->SetPropertyAtIndexFromArgs(ePropertyServerURLs, arg_urls);
+
m_collection_sp->SetValueChangedCallback(
- ePropertyURLs, [this] { URLsChangedCallback(); }
- );
+ ePropertyServerURLs, [this] { ServerURLsChangedCallback(); });
}
Args GetDebugInfoDURLs() const {
Args urls;
- m_collection_sp->GetPropertyAtIndexAsArgs(ePropertyURLs, urls);
+ m_collection_sp->GetPropertyAtIndexAsArgs(ePropertyServerURLs, urls);
return urls;
}
private:
- void URLsChangedCallback() {
+ void ServerURLsChangedCallback() {
Args urls = GetDebugInfoDURLs();
llvm::SmallVector<llvm::StringRef> dbginfod_urls;
llvm::transform(urls, dbginfod_urls.end(),
@@ -85,23 +66,38 @@ class PluginProperties : public Properties {
} // namespace
+static PluginProperties &GetGlobalPluginProperties() {
+ static PluginProperties g_settings;
+ return g_settings;
+}
SymbolLocatorDebuginfod::SymbolLocatorDebuginfod() : SymbolLocator() {}
void SymbolLocatorDebuginfod::Initialize() {
- PluginManager::RegisterPlugin(
- GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance,
- LocateExecutableObjectFile, LocateExecutableSymbolFile,
- DownloadObjectAndSymbolFile);
- // There's a "safety" concern on this:
- // Does plugin initialization occur while things are still single threaded?
- llvm::HTTPClient::initialize();
+ static llvm::once_flag g_once_flag;
+
+ llvm::call_once(g_once_flag, []() {
+ PluginManager::RegisterPlugin(
+ GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance,
+ LocateExecutableObjectFile, LocateExecutableSymbolFile, nullptr,
+ nullptr, SymbolLocatorDebuginfod::DebuggerInitialize);
+ llvm::HTTPClient::initialize();
+ });
+}
+
+void SymbolLocatorDebuginfod::DebuggerInitialize(Debugger &debugger) {
+ if (!PluginManager::GetSettingForSymbolLocatorPlugin(
+ debugger, PluginProperties::GetSettingName())) {
+ const bool is_global_setting = true;
+ PluginManager::CreateSettingForSymbolLocatorPlugin(
+ debugger, GetGlobalPluginProperties().GetValueProperties(),
+ "Properties for the Debuginfod Symbol Locator plug-in.",
+ is_global_setting);
+ }
}
void SymbolLocatorDebuginfod::Terminate() {
PluginManager::UnregisterPlugin(CreateInstance);
- // There's a "safety" concern on this:
- // Does plugin termination occur while things are still single threaded?
llvm::HTTPClient::cleanup();
}
@@ -144,11 +140,3 @@ std::optional<FileSpec> SymbolLocatorDebuginfod::LocateExecutableSymbolFile(
}
return {};
}
-
-bool SymbolLocatorDebuginfod::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
- Status &error,
- bool force_lookup,
- bool copy_executable) {
- // TODO: Continue to add more Debuginfod capabilities
- return false;
-}
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.h b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.h
index d6c6c1d01555f94..0ea79fa1df2a5f7 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.h
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.h
@@ -1,4 +1,4 @@
-//===-- SymbolLocatorDebuginfod.h ----------------------------------*- C++ -*-===//
+//===-- SymbolLocatorDebuginfod.h -------------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -9,6 +9,7 @@
#ifndef LLDB_SOURCE_PLUGINS_SYMBOLLOCATOR_DEBUGINFOD_SYMBOLLOCATORDEBUGINFOD_H
#define LLDB_SOURCE_PLUGINS_SYMBOLLOCATOR_DEBUGINFOD_SYMBOLLOCATORDEBUGINFOD_H
+#include "lldb/Core/Debugger.h"
#include "lldb/Symbol/SymbolLocator.h"
#include "lldb/lldb-private.h"
@@ -20,8 +21,9 @@ class SymbolLocatorDebuginfod : public SymbolLocator {
static void Initialize();
static void Terminate();
+ static void DebuggerInitialize(Debugger &debugger);
- static llvm::StringRef GetPluginNameStatic() { return "Debuginfod"; }
+ static llvm::StringRef GetPluginNameStatic() { return "debuginfod"; }
static llvm::StringRef GetPluginDescriptionStatic();
static lldb_private::SymbolLocator *CreateInstance();
@@ -45,18 +47,6 @@ class SymbolLocatorDebuginfod : public SymbolLocator {
static std::optional<FileSpec>
LocateExecutableSymbolFile(const ModuleSpec &module_spec,
const FileSpecList &default_search_paths);
-
- // Locate the object and symbol file given a module specification.
- //
- // Locating the file can try to download the file from a corporate build
- // repository, or using any other means necessary to locate both the
- // unstripped object file and the debug symbols. The force_lookup argument
- // controls whether the external program is called unconditionally to find
- // the symbol file, or if the user's settings are checked to see if they've
- // enabled the external program before calling.
- static bool DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
- Status &error, bool force_lookup,
- bool copy_executable);
};
} // namespace lldb_private
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td
index c16d765ff761342..1c668b001a16557 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td
@@ -1,8 +1,7 @@
include "../../../../include/lldb/Core/PropertiesBase.td"
let Definition = "symbollocatordebuginfod" in {
- def URLs: Property<"urls", "String">,
- Global,
- DefaultStringValue<"">,
- Desc<"A space-separated, ordered list of Debuginfod server URLs to query for symbols">;
+ def ServerURLs : Property<"server_urls", "Array">,
+ ElementType<"String">,
+ Desc<"An ordered list of Debuginfod server URLs to query for symbols. This defaults to the contents of the DEBUGINFOD_URLS environment variable.">;
}
diff --git a/lldb/source/Symbol/CMakeLists.txt b/lldb/source/Symbol/CMakeLists.txt
index 702e6be4c7c8c8d..ad3488dcdfceced 100644
--- a/lldb/source/Symbol/CMakeLists.txt
+++ b/lldb/source/Symbol/CMakeLists.txt
@@ -38,7 +38,6 @@ add_lldb_library(lldbSymbol NO_PLUGIN_DEPENDENCIES
lldbHost
lldbTarget
lldbUtility
- LLVMDebuginfod
LINK_COMPONENTS
Support
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index afcf4c02c3599b6..a6d7148c84e20be 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -4180,6 +4180,7 @@ TargetProperties::TargetProperties(Target *target)
ePropertyInheritTCC, [this] { InheritTCCValueChangedCallback(); });
m_collection_sp->SetValueChangedCallback(
ePropertyDisableSTDIO, [this] { DisableSTDIOValueChangedCallback(); });
+
m_collection_sp->SetValueChangedCallback(
ePropertySaveObjectsDir, [this] { CheckJITObjectsDir(); });
m_experimental_properties_up =
diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp
index a74dfc5900cdaf5..0cccb896765ffa4 100644
--- a/llvm/lib/Debuginfod/Debuginfod.cpp
+++ b/llvm/lib/Debuginfod/Debuginfod.cpp
@@ -78,7 +78,7 @@ SmallVector<StringRef> getDefaultDebuginfodUrls() {
return DebuginfodUrls;
}
-// Override the default debuginfod URL list.
+// Set the default debuginfod URL list, override the environment variable
void setDefaultDebuginfodUrls(SmallVector<StringRef> URLs) {
DebuginfodUrls.clear();
DebuginfodUrls.insert(DebuginfodUrls.begin(), URLs.begin(), URLs.end());
>From 05a5c91d4141d44f5002c1f357334138b676c679 Mon Sep 17 00:00:00 2001
From: Kevin Frei <kevinfrei at users.noreply.github.com>
Date: Wed, 15 Nov 2023 16:05:09 -0800
Subject: [PATCH 6/7] Update llvm/lib/Debuginfod/Debuginfod.cpp
Co-authored-by: Alex Langford <nirvashtzero at gmail.com>
---
llvm/lib/Debuginfod/Debuginfod.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp
index 0cccb896765ffa4..41e8ff8dddf6c8b 100644
--- a/llvm/lib/Debuginfod/Debuginfod.cpp
+++ b/llvm/lib/Debuginfod/Debuginfod.cpp
@@ -69,10 +69,8 @@ SmallVector<StringRef> getDefaultDebuginfodUrls() {
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) {
+ if (const char *DebuginfodUrlsEnv = std::getenv("DEBUGINFOD_URLS"))
StringRef(DebuginfodUrlsEnv).split(DebuginfodUrls, " ", -1, false);
- }
DebuginfodUrlsSet = true;
}
return DebuginfodUrls;
>From 27f2900c3cee47f67a347186be6031ca2c14c18b Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Wed, 29 Nov 2023 17:02:45 -0800
Subject: [PATCH 7/7] Fixed memory ownership issues, and @clayborg's feedback
---
.../Debuginfod/SymbolLocatorDebuginfod.cpp | 34 ++++++++-----------
llvm/include/llvm/Debuginfod/Debuginfod.h | 2 +-
llvm/lib/Debuginfod/Debuginfod.cpp | 32 ++++++++++-------
3 files changed, 36 insertions(+), 32 deletions(-)
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index bc8584dbda8cee6..14d6bcf38710bc8 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -56,12 +56,14 @@ class PluginProperties : public Properties {
private:
void ServerURLsChangedCallback() {
- Args urls = GetDebugInfoDURLs();
+ m_server_urls = GetDebugInfoDURLs();
llvm::SmallVector<llvm::StringRef> dbginfod_urls;
- llvm::transform(urls, dbginfod_urls.end(),
- [](const auto &obj) { return obj.ref(); });
+ llvm::for_each(
+ m_server_urls, [&](const auto &obj) { dbginfod_urls.push_back(obj.ref()); });
llvm::setDefaultDebuginfodUrls(dbginfod_urls);
}
+ // Storage for the StringRef's used within the Debuginfod library.
+ Args m_server_urls;
};
} // namespace
@@ -109,13 +111,13 @@ SymbolLocator *SymbolLocatorDebuginfod::CreateInstance() {
return new SymbolLocatorDebuginfod();
}
-std::optional<ModuleSpec> SymbolLocatorDebuginfod::LocateExecutableObjectFile(
- const ModuleSpec &module_spec) {
+static std::optional<FileSpec>
+GetFileForModule(const ModuleSpec &module_spec,
+ std::function<llvm::Expected<std::string>(llvm::object::BuildIDRef)> PullFromServer) {
const UUID &module_uuid = module_spec.GetUUID();
if (module_uuid.IsValid() && llvm::canUseDebuginfod()) {
llvm::object::BuildID build_id(module_uuid.GetBytes());
- llvm::Expected<std::string> result =
- llvm::getCachedOrDownloadExecutable(build_id);
+ llvm::Expected<std::string> result = PullFromServer(build_id);
if (result)
return FileSpec(*result);
// An error here should be logged as a failure in the Debuginfod library,
@@ -125,18 +127,12 @@ std::optional<ModuleSpec> SymbolLocatorDebuginfod::LocateExecutableObjectFile(
return {};
}
+std::optional<ModuleSpec> SymbolLocatorDebuginfod::LocateExecutableObjectFile(
+ const ModuleSpec &module_spec) {
+ return GetFileForModule(module_spec, llvm::getCachedOrDownloadExecutable);
+}
+
std::optional<FileSpec> SymbolLocatorDebuginfod::LocateExecutableSymbolFile(
const ModuleSpec &module_spec, const FileSpecList &default_search_paths) {
- const UUID &module_uuid = module_spec.GetUUID();
- 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 here should be logged as a failure in the Debuginfod library,
- // so just consume it here
- consumeError(result.takeError());
- }
- return {};
+ return GetFileForModule(module_spec, llvm::getCachedOrDownloadDebuginfo);
}
diff --git a/llvm/include/llvm/Debuginfod/Debuginfod.h b/llvm/include/llvm/Debuginfod/Debuginfod.h
index 9351af27cc5fe2c..251fd7005305e74 100644
--- a/llvm/include/llvm/Debuginfod/Debuginfod.h
+++ b/llvm/include/llvm/Debuginfod/Debuginfod.h
@@ -48,7 +48,7 @@ SmallVector<StringRef> getDefaultDebuginfodUrls();
/// Sets the list of debuginfod server URLs to query. This overrides the
/// environment variable DEBUGINFOD_URLS.
-void setDefaultDebuginfodUrls(SmallVector<StringRef> URLs);
+void setDefaultDebuginfodUrls(const SmallVector<StringRef> &URLs);
/// Finds a default local file caching directory for the debuginfod client,
/// first checking DEBUGINFOD_CACHE_PATH.
diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp
index 41e8ff8dddf6c8b..856c3e6cb73ddc9 100644
--- a/llvm/lib/Debuginfod/Debuginfod.cpp
+++ b/llvm/lib/Debuginfod/Debuginfod.cpp
@@ -41,15 +41,18 @@
#include "llvm/Support/xxhash.h"
#include <atomic>
+#include <optional>
#include <thread>
namespace llvm {
using llvm::object::BuildIDRef;
-SmallVector<StringRef> DebuginfodUrls;
-
-bool DebuginfodUrlsSet = false;
+namespace {
+std::optional<SmallVector<StringRef>> DebuginfodUrls;
+// Many Readers/Single Writer lock protecting the global debuginfod URL list.
+std::shared_mutex UrlsMutex;
+}
static std::string uniqueKey(llvm::StringRef S) {
return utostr(xxh3_64bits(S));
@@ -66,21 +69,26 @@ bool canUseDebuginfod() {
}
SmallVector<StringRef> getDefaultDebuginfodUrls() {
- if (!DebuginfodUrlsSet) {
+ std::shared_lock<std::shared_mutex> ReadGuard(UrlsMutex);
+ if (!DebuginfodUrls) {
// Only read from the environment variable if the user hasn't already
// set the value
- if (const char *DebuginfodUrlsEnv = std::getenv("DEBUGINFOD_URLS"))
- StringRef(DebuginfodUrlsEnv).split(DebuginfodUrls, " ", -1, false);
- DebuginfodUrlsSet = true;
+ ReadGuard.unlock();
+ std::unique_lock<std::shared_mutex> WriteGuard(UrlsMutex);
+ DebuginfodUrls = SmallVector<StringRef>();
+ if (const char *DebuginfodUrlsEnv = std::getenv("DEBUGINFOD_URLS")) {
+ StringRef(DebuginfodUrlsEnv).split(DebuginfodUrls.value(), " ", -1, false);
+ }
+ WriteGuard.unlock();
+ ReadGuard.lock();
}
- return DebuginfodUrls;
+ return DebuginfodUrls.value();
}
// Set the default debuginfod URL list, override the environment variable
-void setDefaultDebuginfodUrls(SmallVector<StringRef> URLs) {
- DebuginfodUrls.clear();
- DebuginfodUrls.insert(DebuginfodUrls.begin(), URLs.begin(), URLs.end());
- DebuginfodUrlsSet = true;
+void setDefaultDebuginfodUrls(const SmallVector<StringRef> &URLs) {
+ std::unique_lock<std::shared_mutex> WriteGuard(UrlsMutex);
+ DebuginfodUrls = URLs;
}
/// Finds a default local file caching directory for the debuginfod client,
More information about the llvm-commits
mailing list