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 <fr...@meta.com> Date: Wed, 18 Oct 2023 14:37:34 -0700 Subject: [PATCH 1/9] 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 <kevinf...@hotmail.com> Date: Thu, 2 Nov 2023 16:22:10 -0700 Subject: [PATCH 2/9] 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 <kevinf...@hotmail.com> Date: Wed, 8 Nov 2023 13:57:35 -0800 Subject: [PATCH 3/9] 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 <fr...@meta.com> Date: Thu, 9 Nov 2023 16:43:40 -0800 Subject: [PATCH 4/9] 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 <fr...@meta.com> Date: Fri, 10 Nov 2023 16:26:28 -0800 Subject: [PATCH 5/9] 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 <kevinf...@users.noreply.github.com> Date: Wed, 15 Nov 2023 16:05:09 -0800 Subject: [PATCH 6/9] Update llvm/lib/Debuginfod/Debuginfod.cpp Co-authored-by: Alex Langford <nirvashtz...@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 <fr...@meta.com> Date: Wed, 29 Nov 2023 17:02:45 -0800 Subject: [PATCH 7/9] 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, >From d6627637e62418ddb90c78e796eb3635fc7340a5 Mon Sep 17 00:00:00 2001 From: Kevin Frei <fr...@meta.com> Date: Thu, 30 Nov 2023 08:58:47 -0800 Subject: [PATCH 8/9] Fixed the autoformatting stuff --- .../Debuginfod/SymbolLocatorDebuginfod.cpp | 12 +++++++----- llvm/lib/Debuginfod/Debuginfod.cpp | 5 +++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp index 14d6bcf38710bc8..1b7716d71dec095 100644 --- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp +++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp @@ -58,8 +58,9 @@ class PluginProperties : public Properties { void ServerURLsChangedCallback() { m_server_urls = GetDebugInfoDURLs(); llvm::SmallVector<llvm::StringRef> dbginfod_urls; - llvm::for_each( - m_server_urls, [&](const auto &obj) { dbginfod_urls.push_back(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. @@ -111,9 +112,10 @@ SymbolLocator *SymbolLocatorDebuginfod::CreateInstance() { return new SymbolLocatorDebuginfod(); } -static std::optional<FileSpec> -GetFileForModule(const ModuleSpec &module_spec, - std::function<llvm::Expected<std::string>(llvm::object::BuildIDRef)> PullFromServer) { +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()); diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp index 856c3e6cb73ddc9..c1cab8d79cabd7f 100644 --- a/llvm/lib/Debuginfod/Debuginfod.cpp +++ b/llvm/lib/Debuginfod/Debuginfod.cpp @@ -52,7 +52,7 @@ namespace { std::optional<SmallVector<StringRef>> DebuginfodUrls; // Many Readers/Single Writer lock protecting the global debuginfod URL list. std::shared_mutex UrlsMutex; -} +} // namespace static std::string uniqueKey(llvm::StringRef S) { return utostr(xxh3_64bits(S)); @@ -77,7 +77,8 @@ SmallVector<StringRef> getDefaultDebuginfodUrls() { 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); + StringRef(DebuginfodUrlsEnv) + .split(DebuginfodUrls.value(), " ", -1, false); } WriteGuard.unlock(); ReadGuard.lock(); >From 8e93c1009adb4d4f09257923b7b3f342675415f8 Mon Sep 17 00:00:00 2001 From: Kevin Frei <fr...@meta.com> Date: Thu, 30 Nov 2023 14:32:07 -0800 Subject: [PATCH 9/9] Added a release note item for DEDBUGINFOD --- llvm/docs/ReleaseNotes.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst index f221025c06d7907..db3117acfd87b83 100644 --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -240,6 +240,13 @@ Changes to LLDB (SME) and Scalable Matrix Extension 2 (SME2) for both live processes and core files. For details refer to the `AArch64 Linux documentation <https://lldb.llvm.org/use/aarch64-linux.html>`_. +* LLDB now supports symbol and binary acquisition automatically using the + DEBUFINFOD protocol. The standard mechanism of specifying DEBUFINOD servers in + the ``DEBUGINFOD_URLS`` environment variable is used by default. In addition, + users can specify servers to request symbols from using the LLDB setting + ``plugin.symbol-locator.debuginfod.server_urls``, override or adding to the + environment variable. + Changes to Sanitizers --------------------- _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits