Author: Kevin Frei Date: 2024-01-22T11:09:48-08:00 New Revision: 6d5f8d3e6e473a6890f232dba8b14b7b46140558
URL: https://github.com/llvm/llvm-project/commit/6d5f8d3e6e473a6890f232dba8b14b7b46140558 DIFF: https://github.com/llvm/llvm-project/commit/6d5f8d3e6e473a6890f232dba8b14b7b46140558.diff LOG: Added settings for DEBUGINFOD cache location and timeout (#78605) I've been working on more/better configuration for improving DEBUGINFOD support. This is the first (and easiest) slice of the work. I've added `timeout` and `cache-path` settings that can override the DEBUGINFOD library defaults (and environment variables.) I also renamed the `plugin.symbol-locator.debuginfod.server_urls` setting to `server-urls` to be more consistent with the rest of LLDB's settings (the underscore switch is switched to a hyphen) I've got a few tests that validate the cache-path setting (as a side-effect), but they've exposed a few bugs that I'll be putting up a separate PR for (which will include the tests). --------- Co-authored-by: Kevin Frei <fr...@meta.com> Added: Modified: lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td llvm/include/llvm/Debuginfod/Debuginfod.h llvm/lib/Debuginfod/Debuginfod.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp index 111be6be365240e..2cd7bbbb2449028 100644 --- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp +++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp @@ -9,7 +9,10 @@ #include "SymbolLocatorDebuginfod.h" #include "lldb/Core/PluginManager.h" +#include "lldb/Interpreter/OptionValueString.h" #include "lldb/Utility/Args.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/Utility/Log.h" #include "llvm/Debuginfod/Debuginfod.h" #include "llvm/Debuginfod/HTTPClient.h" @@ -54,6 +57,32 @@ class PluginProperties : public Properties { return urls; } + llvm::Expected<std::string> GetCachePath() { + OptionValueString *s = + m_collection_sp->GetPropertyAtIndexAsOptionValueString( + ePropertySymbolCachePath); + // If we don't have a valid cache location, use the default one. + if (!s || !s->GetCurrentValueAsRef().size()) { + llvm::Expected<std::string> maybeCachePath = + llvm::getDefaultDebuginfodCacheDirectory(); + if (!maybeCachePath) + return maybeCachePath; + return *maybeCachePath; + } + return s->GetCurrentValue(); + } + + std::chrono::milliseconds GetTimeout() const { + std::optional<uint64_t> seconds = + m_collection_sp->GetPropertyAtIndexAs<uint64_t>(ePropertyTimeout); + if (seconds && *seconds != 0) { + return std::chrono::duration_cast<std::chrono::milliseconds>( + std::chrono::seconds(*seconds)); + } else { + return llvm::getDefaultDebuginfodTimeout(); + } + } + private: void ServerURLsChangedCallback() { m_server_urls = GetDebugInfoDURLs(); @@ -112,31 +141,51 @@ 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) { - if (!ModuleList::GetGlobalModuleListProperties().GetEnableExternalLookup()) - return {}; +static std::optional<FileSpec> +GetFileForModule(const ModuleSpec &module_spec, + std::function<std::string(llvm::object::BuildID)> UrlBuilder) { 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 = PullFromServer(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()); - } + // Don't bother if we don't have a valid UUID, Debuginfod isn't available, + // or if the 'symbols.enable-external-lookup' setting is false. + if (!module_uuid.IsValid() || !llvm::canUseDebuginfod() || + !ModuleList::GetGlobalModuleListProperties().GetEnableExternalLookup()) + return {}; + + // Grab LLDB's Debuginfod overrides from the + // plugin.symbol-locator.debuginfod.* settings. + PluginProperties &plugin_props = GetGlobalPluginProperties(); + llvm::Expected<std::string> cache_path_or_err = plugin_props.GetCachePath(); + // A cache location is *required*. + if (!cache_path_or_err) + return {}; + std::string cache_path = *cache_path_or_err; + llvm::SmallVector<llvm::StringRef> debuginfod_urls = + llvm::getDefaultDebuginfodUrls(); + std::chrono::milliseconds timeout = plugin_props.GetTimeout(); + + // We're ready to ask the Debuginfod library to find our file. + llvm::object::BuildID build_id(module_uuid.GetBytes()); + std::string url_path = UrlBuilder(build_id); + std::string cache_key = llvm::getDebuginfodCacheKey(url_path); + llvm::Expected<std::string> result = llvm::getCachedOrDownloadArtifact( + cache_key, url_path, cache_path, debuginfod_urls, timeout); + if (result) + return FileSpec(*result); + + Log *log = GetLog(LLDBLog::Symbols); + auto err_message = llvm::toString(result.takeError()); + LLDB_LOGV(log, + "Debuginfod failed to download symbol artifact {0} with error {1}", + url_path, err_message); return {}; } std::optional<ModuleSpec> SymbolLocatorDebuginfod::LocateExecutableObjectFile( const ModuleSpec &module_spec) { - return GetFileForModule(module_spec, llvm::getCachedOrDownloadExecutable); + return GetFileForModule(module_spec, llvm::getDebuginfodExecutableUrlPath); } std::optional<FileSpec> SymbolLocatorDebuginfod::LocateExecutableSymbolFile( const ModuleSpec &module_spec, const FileSpecList &default_search_paths) { - return GetFileForModule(module_spec, llvm::getCachedOrDownloadDebuginfo); + return GetFileForModule(module_spec, llvm::getDebuginfodDebuginfoUrlPath); } diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td index 1c668b001a16557..0ff02674b8ea319 100644 --- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td +++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td @@ -1,7 +1,13 @@ include "../../../../include/lldb/Core/PropertiesBase.td" let Definition = "symbollocatordebuginfod" in { - def ServerURLs : Property<"server_urls", "Array">, + 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.">; + def SymbolCachePath: Property<"cache-path", "String">, + DefaultStringValue<"">, + Desc<"The path where symbol files should be cached. This defaults to LLDB's system cache location.">; + def Timeout : Property<"timeout", "UInt64">, + DefaultUnsignedValue<0>, + Desc<"Timeout (in seconds) for requests made to a DEBUGINFOD server. A value of zero means we use the debuginfod default timeout: DEBUGINFOD_TIMEOUT if the environment variable is set and 90 seconds otherwise.">; } diff --git a/llvm/include/llvm/Debuginfod/Debuginfod.h b/llvm/include/llvm/Debuginfod/Debuginfod.h index 251fd7005305e74..ef03948a706c06d 100644 --- a/llvm/include/llvm/Debuginfod/Debuginfod.h +++ b/llvm/include/llvm/Debuginfod/Debuginfod.h @@ -46,6 +46,9 @@ bool canUseDebuginfod(); /// environment variable. SmallVector<StringRef> getDefaultDebuginfodUrls(); +/// Returns the cache key for a given debuginfod URL path. +std::string getDebuginfodCacheKey(StringRef UrlPath); + /// Sets the list of debuginfod server URLs to query. This overrides the /// environment variable DEBUGINFOD_URLS. void setDefaultDebuginfodUrls(const SmallVector<StringRef> &URLs); @@ -58,15 +61,26 @@ Expected<std::string> getDefaultDebuginfodCacheDirectory(); /// DEBUGINFOD_TIMEOUT environment variable, default is 90 seconds (90000 ms). std::chrono::milliseconds getDefaultDebuginfodTimeout(); +/// Get the full URL path for a source request of a given BuildID and file +/// path. +std::string getDebuginfodSourceUrlPath(object::BuildIDRef ID, + StringRef SourceFilePath); + /// Fetches a specified source file by searching the default local cache /// directory and server URLs. Expected<std::string> getCachedOrDownloadSource(object::BuildIDRef ID, StringRef SourceFilePath); +/// Get the full URL path for an executable request of a given BuildID. +std::string getDebuginfodExecutableUrlPath(object::BuildIDRef ID); + /// Fetches an executable by searching the default local cache directory and /// server URLs. Expected<std::string> getCachedOrDownloadExecutable(object::BuildIDRef ID); +/// Get the full URL path for a debug binary request of a given BuildID. +std::string getDebuginfodDebuginfoUrlPath(object::BuildIDRef ID); + /// Fetches a debug binary by searching the default local cache directory and /// server URLs. Expected<std::string> getCachedOrDownloadDebuginfo(object::BuildIDRef ID); diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp index 9df30ab55cbad48..4928fcb3b3f8791 100644 --- a/llvm/lib/Debuginfod/Debuginfod.cpp +++ b/llvm/lib/Debuginfod/Debuginfod.cpp @@ -54,7 +54,7 @@ std::optional<SmallVector<StringRef>> DebuginfodUrls; llvm::sys::RWMutex UrlsMutex; } // namespace -static std::string uniqueKey(llvm::StringRef S) { +std::string getDebuginfodCacheKey(llvm::StringRef S) { return utostr(xxh3_64bits(S)); } @@ -72,7 +72,7 @@ SmallVector<StringRef> getDefaultDebuginfodUrls() { std::shared_lock<llvm::sys::RWMutex> ReadGuard(UrlsMutex); if (!DebuginfodUrls) { // Only read from the environment variable if the user hasn't already - // set the value + // set the value. ReadGuard.unlock(); std::unique_lock<llvm::sys::RWMutex> WriteGuard(UrlsMutex); DebuginfodUrls = SmallVector<StringRef>(); @@ -86,7 +86,7 @@ SmallVector<StringRef> getDefaultDebuginfodUrls() { return DebuginfodUrls.value(); } -// Set the default debuginfod URL list, override the environment variable +// Set the default debuginfod URL list, override the environment variable. void setDefaultDebuginfodUrls(const SmallVector<StringRef> &URLs) { std::unique_lock<llvm::sys::RWMutex> WriteGuard(UrlsMutex); DebuginfodUrls = URLs; @@ -120,27 +120,43 @@ std::chrono::milliseconds getDefaultDebuginfodTimeout() { /// cache and return the cached file path. They first search the local cache, /// followed by the debuginfod servers. -Expected<std::string> getCachedOrDownloadSource(BuildIDRef ID, - StringRef SourceFilePath) { +std::string getDebuginfodSourceUrlPath(BuildIDRef ID, + StringRef SourceFilePath) { SmallString<64> UrlPath; sys::path::append(UrlPath, sys::path::Style::posix, "buildid", buildIDToString(ID), "source", sys::path::convert_to_slash(SourceFilePath)); - return getCachedOrDownloadArtifact(uniqueKey(UrlPath), UrlPath); + return std::string(UrlPath); } -Expected<std::string> getCachedOrDownloadExecutable(BuildIDRef ID) { +Expected<std::string> getCachedOrDownloadSource(BuildIDRef ID, + StringRef SourceFilePath) { + std::string UrlPath = getDebuginfodSourceUrlPath(ID, SourceFilePath); + return getCachedOrDownloadArtifact(getDebuginfodCacheKey(UrlPath), UrlPath); +} + +std::string getDebuginfodExecutableUrlPath(BuildIDRef ID) { SmallString<64> UrlPath; sys::path::append(UrlPath, sys::path::Style::posix, "buildid", buildIDToString(ID), "executable"); - return getCachedOrDownloadArtifact(uniqueKey(UrlPath), UrlPath); + return std::string(UrlPath); } -Expected<std::string> getCachedOrDownloadDebuginfo(BuildIDRef ID) { +Expected<std::string> getCachedOrDownloadExecutable(BuildIDRef ID) { + std::string UrlPath = getDebuginfodExecutableUrlPath(ID); + return getCachedOrDownloadArtifact(getDebuginfodCacheKey(UrlPath), UrlPath); +} + +std::string getDebuginfodDebuginfoUrlPath(BuildIDRef ID) { SmallString<64> UrlPath; sys::path::append(UrlPath, sys::path::Style::posix, "buildid", buildIDToString(ID), "debuginfo"); - return getCachedOrDownloadArtifact(uniqueKey(UrlPath), UrlPath); + return std::string(UrlPath); +} + +Expected<std::string> getCachedOrDownloadDebuginfo(BuildIDRef ID) { + std::string UrlPath = getDebuginfodDebuginfoUrlPath(ID); + return getCachedOrDownloadArtifact(getDebuginfodCacheKey(UrlPath), UrlPath); } // General fetching function. _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
