[Lldb-commits] [lldb] [llvm] Added settings for DEBUGINFOD cache location and timeout (PR #78605)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Jan 18 09:28:32 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
@llvm/pr-subscribers-debuginfo
Author: Kevin Frei (kevinfrei)
<details>
<summary>Changes</summary>
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).
---
Full diff: https://github.com/llvm/llvm-project/pull/78605.diff
4 Files Affected:
- (modified) lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp (+62-15)
- (modified) lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td (+7-1)
- (modified) llvm/include/llvm/Debuginfod/Debuginfod.h (+13)
- (modified) llvm/lib/Debuginfod/Debuginfod.cpp (+23-8)
``````````diff
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index 111be6be365240e..2a618271624a1b6 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -9,6 +9,7 @@
#include "SymbolLocatorDebuginfod.h"
#include "lldb/Core/PluginManager.h"
+#include "lldb/Interpreter/OptionValueString.h"
#include "lldb/Utility/Args.h"
#include "llvm/Debuginfod/Debuginfod.h"
@@ -54,6 +55,34 @@ class PluginProperties : public Properties {
return urls;
}
+ llvm::Expected<llvm::StringRef> 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;
+ }
+ m_cache_path = *maybeCachePath;
+ return llvm::StringRef(m_cache_path);
+ }
+ return s->GetCurrentValueAsRef();
+ }
+
+ 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();
@@ -65,6 +94,7 @@ class PluginProperties : public Properties {
}
// Storage for the StringRef's used within the Debuginfod library.
Args m_server_urls;
+ std::string m_cache_path;
};
} // namespace
@@ -114,29 +144,46 @@ SymbolLocator *SymbolLocatorDebuginfod::CreateInstance() {
static std::optional<FileSpec> GetFileForModule(
const ModuleSpec &module_spec,
- std::function<llvm::Expected<std::string>(llvm::object::BuildIDRef)>
- PullFromServer) {
- if (!ModuleList::GetGlobalModuleListProperties().GetEnableExternalLookup())
- return {};
+ 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 the settings values we need
+ PluginProperties &plugin_props = GetGlobalPluginProperties();
+ llvm::Expected<llvm::StringRef> CacheDirectoryPathOrErr =
+ plugin_props.GetCachePath();
+ // A cache location is *required*
+ if (!CacheDirectoryPathOrErr)
+ return {};
+ llvm::StringRef CacheDirectoryPath = *CacheDirectoryPathOrErr;
+ llvm::SmallVector<llvm::StringRef> DebuginfodUrls =
+ 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 UrlPath = UrlBuilder(build_id);
+ std::string CacheKey = llvm::getDebuginfodCacheKey(UrlPath);
+ llvm::Expected<std::string> result = llvm::getCachedOrDownloadArtifact(
+ CacheKey, UrlPath, CacheDirectoryPath, DebuginfodUrls, Timeout);
+ if (result)
+ return FileSpec(*result);
+ // An error here should be logged as a failure in the Debuginfod library,
+ // just consume it here
+ consumeError(result.takeError());
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..7f17faa8241aa78 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 defaults to DEBUGINFOD_TIMEOUT environment variable (or 90 seconds).">;
}
diff --git a/llvm/include/llvm/Debuginfod/Debuginfod.h b/llvm/include/llvm/Debuginfod/Debuginfod.h
index 251fd7005305e74..719bbf9cd0ccca2 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();
+/// Creates the cache-key for a given Debuginfod UrlPath
+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,25 @@ Expected<std::string> getDefaultDebuginfodCacheDirectory();
/// DEBUGINFOD_TIMEOUT environment variable, default is 90 seconds (90000 ms).
std::chrono::milliseconds getDefaultDebuginfodTimeout();
+/// Get the full UrlPath 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 UrlPath 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 UrlPath 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..5cabb40e3551ed5 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));
}
@@ -120,29 +120,44 @@ 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.
Expected<std::string> getCachedOrDownloadArtifact(StringRef UniqueKey,
StringRef UrlPath) {
``````````
</details>
https://github.com/llvm/llvm-project/pull/78605
More information about the lldb-commits
mailing list