[Lldb-commits] [lldb] [llvm] Added settings for DEBUGINFOD cache location and timeout (PR #78605)

Kevin Frei via lldb-commits lldb-commits at lists.llvm.org
Thu Jan 18 10:45:27 PST 2024


https://github.com/kevinfrei updated https://github.com/llvm/llvm-project/pull/78605

>From b46553c6fe17a50dc2072544e46b7a1dde127436 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/2] 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 410349fb626230ba7fe10a68d10fb87ca755cc1f 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/2] 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) {



More information about the lldb-commits mailing list