[mlir] [clang] [libcxx] [flang] [libc] [llvm] [clang-tools-extra] [lldb] [compiler-rt] Added settings for DEBUGINFOD cache location and timeout (PR #78605)
Kevin Frei via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 19 04:52:20 PST 2024
https://github.com/kevinfrei updated https://github.com/llvm/llvm-project/pull/78605
>From 48c6e5edc1dc5f832f8f5c922c61af9070ad341d Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 18 Jan 2024 09:09:50 -0800
Subject: [PATCH 1/8] Added settings for cache location and timeout
---
.../Debuginfod/SymbolLocatorDebuginfod.cpp | 82 +++++++++++++++----
.../SymbolLocatorDebuginfodProperties.td | 8 +-
llvm/include/llvm/Debuginfod/Debuginfod.h | 13 +++
llvm/lib/Debuginfod/Debuginfod.cpp | 31 +++++--
4 files changed, 108 insertions(+), 26 deletions(-)
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index 111be6be365240..a20437c256eb43 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
@@ -112,31 +142,49 @@ 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 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 1c668b001a1655..7f17faa8241aa7 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 251fd7005305e7..719bbf9cd0ccca 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 9df30ab55cbad4..5cabb40e3551ed 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) {
>From 4ac3020d5dbdef9aa0271ba882d7dd9784c63a83 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 18 Jan 2024 10:32:03 -0800
Subject: [PATCH 2/8] Typo
---
.../SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp | 7 +++----
llvm/lib/Debuginfod/Debuginfod.cpp | 1 +
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index a20437c256eb43..afbc7d8b12febf 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -142,10 +142,9 @@ SymbolLocator *SymbolLocatorDebuginfod::CreateInstance() {
return new SymbolLocatorDebuginfod();
}
-+ static std::optional<FileSpec> +
- GetFileForModule(
- const ModuleSpec &module_spec,
- std::function<std::string(llvm::object::BuildID)> UrlBuilder) {
+static std::optional<FileSpec>
+GetFileForModule(const ModuleSpec &module_spec,
+ std::function<std::string(llvm::object::BuildID)> UrlBuilder) {
const UUID &module_uuid = module_spec.GetUUID();
// 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
diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp
index 5cabb40e3551ed..bdba384815ebf6 100644
--- a/llvm/lib/Debuginfod/Debuginfod.cpp
+++ b/llvm/lib/Debuginfod/Debuginfod.cpp
@@ -158,6 +158,7 @@ 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) {
>From 20488a62410ee2474381fb324d9162ea0c3aef83 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 18 Jan 2024 12:46:02 -0800
Subject: [PATCH 3/8] Comment cleanup
---
.../Debuginfod/SymbolLocatorDebuginfod.cpp | 11 ++++++-----
llvm/lib/Debuginfod/Debuginfod.cpp | 4 ++--
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index afbc7d8b12febf..45e2f5acb11e96 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -147,16 +147,17 @@ GetFileForModule(const ModuleSpec &module_spec,
std::function<std::string(llvm::object::BuildID)> UrlBuilder) {
const UUID &module_uuid = module_spec.GetUUID();
// 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
+ // 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
+ // Grab LLDB's Debuginfod overrides from the
+ // plugin.symbol-locator.debuginfod.* settings.
PluginProperties &plugin_props = GetGlobalPluginProperties();
llvm::Expected<llvm::StringRef> CacheDirectoryPathOrErr =
plugin_props.GetCachePath();
- // A cache location is *required*
+ // A cache location is *required*.
if (!CacheDirectoryPathOrErr)
return {};
llvm::StringRef CacheDirectoryPath = *CacheDirectoryPathOrErr;
@@ -164,7 +165,7 @@ GetFileForModule(const ModuleSpec &module_spec,
llvm::getDefaultDebuginfodUrls();
std::chrono::milliseconds Timeout = plugin_props.GetTimeout();
- // We're ready to ask the Debuginfod library to find our file
+ // 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);
@@ -173,7 +174,7 @@ GetFileForModule(const ModuleSpec &module_spec,
if (result)
return FileSpec(*result);
// An error here should be logged as a failure in the Debuginfod library,
- // just consume it here
+ // just consume it here.
consumeError(result.takeError());
return {};
}
diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp
index bdba384815ebf6..4928fcb3b3f879 100644
--- a/llvm/lib/Debuginfod/Debuginfod.cpp
+++ b/llvm/lib/Debuginfod/Debuginfod.cpp
@@ -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;
>From 6d9028cd7f8709a2570dbe80d3c8a94a1129cbe0 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 18 Jan 2024 14:09:47 -0800
Subject: [PATCH 4/8] Switch locals from PascalCase to snake_case
---
.../Debuginfod/SymbolLocatorDebuginfod.cpp | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index 45e2f5acb11e96..680167ebecd951 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -155,22 +155,22 @@ GetFileForModule(const ModuleSpec &module_spec,
// Grab LLDB's Debuginfod overrides from the
// plugin.symbol-locator.debuginfod.* settings.
PluginProperties &plugin_props = GetGlobalPluginProperties();
- llvm::Expected<llvm::StringRef> CacheDirectoryPathOrErr =
+ llvm::Expected<llvm::StringRef> cache_path_or_err =
plugin_props.GetCachePath();
// A cache location is *required*.
- if (!CacheDirectoryPathOrErr)
+ if (!cache_path_or_err)
return {};
- llvm::StringRef CacheDirectoryPath = *CacheDirectoryPathOrErr;
- llvm::SmallVector<llvm::StringRef> DebuginfodUrls =
+ llvm::StringRef cache_path = *cache_path_or_err;
+ llvm::SmallVector<llvm::StringRef> debuginfod_urls =
llvm::getDefaultDebuginfodUrls();
- std::chrono::milliseconds Timeout = plugin_props.GetTimeout();
+ 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);
+ std::string url_path = UrlBuilder(build_id);
+ std::string cache_key = llvm::getDebuginfodCacheKey(url_path);
llvm::Expected<std::string> result = llvm::getCachedOrDownloadArtifact(
- CacheKey, UrlPath, CacheDirectoryPath, DebuginfodUrls, Timeout);
+ cache_key, url_path, cache_path, debuginfod_urls, timeout);
if (result)
return FileSpec(*result);
// An error here should be logged as a failure in the Debuginfod library,
>From 50ce5fe2b595fbfea178cfa66dbfd089e64cf682 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 18 Jan 2024 15:02:25 -0800
Subject: [PATCH 5/8] Log Debuginfod server failures in verbose mode
---
.../Debuginfod/SymbolLocatorDebuginfod.cpp | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index 680167ebecd951..71503790fbb087 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -11,6 +11,8 @@
#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"
@@ -173,9 +175,13 @@ GetFileForModule(const ModuleSpec &module_spec,
cache_key, url_path, cache_path, debuginfod_urls, 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());
+
+ 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 {};
}
>From 9b248d4db999b1befaf56f3401fdd4ce2fafc677 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 18 Jan 2024 16:19:07 -0800
Subject: [PATCH 6/8] Switch from StringRef to std::string in
PluginProps::GetCachePath
---
.../Debuginfod/SymbolLocatorDebuginfod.cpp | 16 ++++++----------
llvm/include/llvm/Debuginfod/Debuginfod.h | 9 +++++----
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index 71503790fbb087..65bf3c531378f8 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -57,7 +57,7 @@ class PluginProperties : public Properties {
return urls;
}
- llvm::Expected<llvm::StringRef> GetCachePath() {
+ llvm::Expected<std::string> GetCachePath() {
OptionValueString *s =
m_collection_sp->GetPropertyAtIndexAsOptionValueString(
ePropertySymbolCachePath);
@@ -65,13 +65,11 @@ class PluginProperties : public Properties {
if (!s || !s->GetCurrentValueAsRef().size()) {
llvm::Expected<std::string> maybeCachePath =
llvm::getDefaultDebuginfodCacheDirectory();
- if (!maybeCachePath) {
+ if (!maybeCachePath)
return maybeCachePath;
- }
- m_cache_path = *maybeCachePath;
- return llvm::StringRef(m_cache_path);
+ return *maybeCachePath;
}
- return s->GetCurrentValueAsRef();
+ return s->GetCurrentValue();
}
std::chrono::milliseconds GetTimeout() const {
@@ -96,7 +94,6 @@ class PluginProperties : public Properties {
}
// Storage for the StringRef's used within the Debuginfod library.
Args m_server_urls;
- std::string m_cache_path;
};
} // namespace
@@ -157,12 +154,11 @@ GetFileForModule(const ModuleSpec &module_spec,
// Grab LLDB's Debuginfod overrides from the
// plugin.symbol-locator.debuginfod.* settings.
PluginProperties &plugin_props = GetGlobalPluginProperties();
- llvm::Expected<llvm::StringRef> cache_path_or_err =
- plugin_props.GetCachePath();
+ llvm::Expected<std::string> cache_path_or_err = plugin_props.GetCachePath();
// A cache location is *required*.
if (!cache_path_or_err)
return {};
- llvm::StringRef cache_path = *cache_path_or_err;
+ std::string cache_path = *cache_path_or_err;
llvm::SmallVector<llvm::StringRef> debuginfod_urls =
llvm::getDefaultDebuginfodUrls();
std::chrono::milliseconds timeout = plugin_props.GetTimeout();
diff --git a/llvm/include/llvm/Debuginfod/Debuginfod.h b/llvm/include/llvm/Debuginfod/Debuginfod.h
index 719bbf9cd0ccca..ef03948a706c06 100644
--- a/llvm/include/llvm/Debuginfod/Debuginfod.h
+++ b/llvm/include/llvm/Debuginfod/Debuginfod.h
@@ -46,7 +46,7 @@ bool canUseDebuginfod();
/// environment variable.
SmallVector<StringRef> getDefaultDebuginfodUrls();
-/// Creates the cache-key for a given Debuginfod UrlPath
+/// 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
@@ -61,7 +61,8 @@ 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.
+/// Get the full URL path for a source request of a given BuildID and file
+/// path.
std::string getDebuginfodSourceUrlPath(object::BuildIDRef ID,
StringRef SourceFilePath);
@@ -70,14 +71,14 @@ std::string getDebuginfodSourceUrlPath(object::BuildIDRef ID,
Expected<std::string> getCachedOrDownloadSource(object::BuildIDRef ID,
StringRef SourceFilePath);
-/// Get the full UrlPath for an Executable request of a given BuildID.
+/// 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 UrlPath for a debug binary request of a given BuildID.
+/// 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
>From 6a29abdfea269e0daf1ad521c029c500630d9bd5 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 18 Jan 2024 16:50:09 -0800
Subject: [PATCH 7/8] Fixed log message
---
.../SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index 65bf3c531378f8..2cd7bbbb244902 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -175,8 +175,7 @@ GetFileForModule(const ModuleSpec &module_spec,
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}",
+ "Debuginfod failed to download symbol artifact {0} with error {1}",
url_path, err_message);
return {};
}
>From 8decbe096a725a05b2613706020e114ca8fe40ab Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 18 Jan 2024 16:56:48 -0800
Subject: [PATCH 8/8] Took JDevlieghere's wording for the timeout setting
---
.../Debuginfod/SymbolLocatorDebuginfodProperties.td | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td
index 7f17faa8241aa7..0ff02674b8ea31 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfodProperties.td
@@ -9,5 +9,5 @@ let Definition = "symbollocatordebuginfod" in {
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).">;
+ 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.">;
}
More information about the cfe-commits
mailing list