[llvm] 6d5f8d3 - Added settings for DEBUGINFOD cache location and timeout (#78605)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 22 11:09:53 PST 2024
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 <freik at 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.
More information about the llvm-commits
mailing list