Author: Jonas Devlieghere
Date: 2024-02-08T11:24:07-08:00
New Revision: 74fc16aaaa227b84e22706d2c5e376287f560b9e

URL: 
https://github.com/llvm/llvm-project/commit/74fc16aaaa227b84e22706d2c5e376287f560b9e
DIFF: 
https://github.com/llvm/llvm-project/commit/74fc16aaaa227b84e22706d2c5e376287f560b9e.diff

LOG: [lldb] Expand background symbol download (#80890)

LLDB has a setting (symbols.enable-background-lookup) that calls
dsymForUUID on a background thread for images as they appear in the
current backtrace. Originally, the laziness of only looking up symbols
for images in the backtrace only existed to bring the number of
dsymForUUID calls down to a manageable number.

Users have requesting the same functionality but blocking. This gives
them the same user experience as enabling dsymForUUID globally, but
without the massive upfront cost of having to download all the images,
the majority of which they'll likely not need.

This patch renames the setting to have a more generic name
(symbols.auto-download) and changes its values from a boolean to an
enum. Users can now specify "off", "background" and "foreground". The
default remains "off" although I'll probably change that in the near
future.

Added: 
    

Modified: 
    lldb/include/lldb/Core/ModuleList.h
    lldb/include/lldb/lldb-enumerations.h
    lldb/source/Core/CoreProperties.td
    lldb/source/Core/ModuleList.cpp
    lldb/source/Host/common/Host.cpp
    lldb/source/Symbol/SymbolLocator.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/ModuleList.h 
b/lldb/include/lldb/Core/ModuleList.h
index d78f7c5ef3f706..43d931a8447406 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -47,6 +47,26 @@ class UUID;
 class VariableList;
 struct ModuleFunctionSearchOptions;
 
+static constexpr OptionEnumValueElement g_auto_download_enum_values[] = {
+    {
+        lldb::eSymbolDownloadOff,
+        "off",
+        "Disable automatically downloading symbols.",
+    },
+    {
+        lldb::eSymbolDownloadBackground,
+        "background",
+        "Download symbols in the background for images as they appear in the "
+        "backtrace.",
+    },
+    {
+        lldb::eSymbolDownloadForeground,
+        "foreground",
+        "Download symbols in the foreground for images as they appear in the "
+        "backtrace.",
+    },
+};
+
 class ModuleListProperties : public Properties {
   mutable llvm::sys::RWMutex m_symlink_paths_mutex;
   PathMappingList m_symlink_paths;
@@ -60,7 +80,6 @@ class ModuleListProperties : public Properties {
   bool SetClangModulesCachePath(const FileSpec &path);
   bool GetEnableExternalLookup() const;
   bool SetEnableExternalLookup(bool new_value);
-  bool GetEnableBackgroundLookup() const;
   bool GetEnableLLDBIndexCache() const;
   bool SetEnableLLDBIndexCache(bool new_value);
   uint64_t GetLLDBIndexCacheMaxByteSize();
@@ -71,6 +90,8 @@ class ModuleListProperties : public Properties {
 
   bool GetLoadSymbolOnDemand();
 
+  lldb::SymbolDownload GetSymbolAutoDownload() const;
+
   PathMappingList GetSymlinkMappings() const;
 };
 

diff  --git a/lldb/include/lldb/lldb-enumerations.h 
b/lldb/include/lldb/lldb-enumerations.h
index 7e9b538aa8372b..4640533047833b 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -1314,6 +1314,12 @@ enum class ChildCacheState {
               ///< re-use what we computed the last time we called Update.
 };
 
+enum SymbolDownload {
+  eSymbolDownloadOff = 0,
+  eSymbolDownloadBackground = 1,
+  eSymbolDownloadForeground = 2,
+};
+
 } // namespace lldb
 
 #endif // LLDB_LLDB_ENUMERATIONS_H

diff  --git a/lldb/source/Core/CoreProperties.td 
b/lldb/source/Core/CoreProperties.td
index 8d81967bdb50a4..9c4aa2de3d7b73 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -8,7 +8,12 @@ let Definition = "modulelist" in {
   def EnableBackgroundLookup: Property<"enable-background-lookup", "Boolean">,
     Global,
     DefaultFalse,
-    Desc<"On macOS, enable calling dsymForUUID (or an equivalent 
script/binary) in the background to locate symbol files that weren't found.">;
+    Desc<"Alias for backward compatibility: when enabled this is the 
equivalent to 'symbols.download background'.">;
+  def AutoDownload: Property<"auto-download", "Enum">,
+    Global,
+    DefaultEnumValue<"eSymbolDownloadOff">,
+    EnumValues<"OptionEnumValues(g_auto_download_enum_values)">,
+    Desc<"On macOS, automatically download symbols with dsymForUUID (or an 
equivalent script/binary) for relevant images in the debug session.">;
   def ClangModulesCachePath: Property<"clang-modules-cache-path", "FileSpec">,
     Global,
     DefaultStringValue<"">,

diff  --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b7f393636ba28d..b03490bacf9593 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -104,10 +104,15 @@ bool ModuleListProperties::SetEnableExternalLookup(bool 
new_value) {
   return SetPropertyAtIndex(ePropertyEnableExternalLookup, new_value);
 }
 
-bool ModuleListProperties::GetEnableBackgroundLookup() const {
-  const uint32_t idx = ePropertyEnableBackgroundLookup;
-  return GetPropertyAtIndexAs<bool>(
-      idx, g_modulelist_properties[idx].default_uint_value != 0);
+SymbolDownload ModuleListProperties::GetSymbolAutoDownload() const {
+  // Backward compatibility alias.
+  if (GetPropertyAtIndexAs<bool>(ePropertyEnableBackgroundLookup, false))
+    return eSymbolDownloadBackground;
+
+  const uint32_t idx = ePropertyAutoDownload;
+  return GetPropertyAtIndexAs<lldb::SymbolDownload>(
+      idx, static_cast<lldb::SymbolDownload>(
+               g_modulelist_properties[idx].default_uint_value));
 }
 
 FileSpec ModuleListProperties::GetClangModulesCachePath() const {

diff  --git a/lldb/source/Host/common/Host.cpp 
b/lldb/source/Host/common/Host.cpp
index f4cec97f5af633..b72ba7e8d20128 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -550,6 +550,8 @@ llvm::Error Host::OpenFileInExternalEditor(llvm::StringRef 
editor,
 }
 
 bool Host::IsInteractiveGraphicSession() { return false; }
+
+bool Host::IsNetworkLimited() { return false; }
 #endif
 
 std::unique_ptr<Connection> Host::CreateDefaultConnection(llvm::StringRef url) 
{

diff  --git a/lldb/source/Symbol/SymbolLocator.cpp 
b/lldb/source/Symbol/SymbolLocator.cpp
index 918f13ed9c193f..93a5bc428b6140 100644
--- a/lldb/source/Symbol/SymbolLocator.cpp
+++ b/lldb/source/Symbol/SymbolLocator.cpp
@@ -10,6 +10,7 @@
 
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Host/Host.h"
 
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/ThreadPool.h"
@@ -18,12 +19,10 @@ using namespace lldb;
 using namespace lldb_private;
 
 void SymbolLocator::DownloadSymbolFileAsync(const UUID &uuid) {
-  if (!ModuleList::GetGlobalModuleListProperties().GetEnableBackgroundLookup())
-    return;
-
   static llvm::SmallSet<UUID, 8> g_seen_uuids;
   static std::mutex g_mutex;
-  Debugger::GetThreadPool().async([=]() {
+
+  auto lookup = [=]() {
     {
       std::lock_guard<std::mutex> guard(g_mutex);
       if (g_seen_uuids.count(uuid))
@@ -36,12 +35,23 @@ void SymbolLocator::DownloadSymbolFileAsync(const UUID 
&uuid) {
     module_spec.GetUUID() = uuid;
     if (!PluginManager::DownloadObjectAndSymbolFile(module_spec, error,
                                                     /*force_lookup=*/true,
-                                                    /*copy_executable=*/false))
+                                                    /*copy_executable=*/true))
       return;
 
     if (error.Fail())
       return;
 
     Debugger::ReportSymbolChange(module_spec);
-  });
+  };
+
+  switch (ModuleList::GetGlobalModuleListProperties().GetSymbolAutoDownload()) 
{
+  case eSymbolDownloadOff:
+    break;
+  case eSymbolDownloadBackground:
+    Debugger::GetThreadPool().async(lookup);
+    break;
+  case eSymbolDownloadForeground:
+    lookup();
+    break;
+  };
 }


        
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to