swiftlang · jasonmolenda · Feb 17, 2024 · Nov 3, 2023 · Nov 4, 2023 · Nov 4, 2023
diff --git 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;
@@ -78,7 +98,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();
@@ -89,6 +108,8 @@ class ModuleListProperties : public Properties {
 
   bool GetLoadSymbolOnDemand();
 
+  lldb::SymbolDownload GetSymbolAutoDownload() const;
+
   PathMappingList GetSymlinkMappings() const;
 };
 

diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h
@@ -345,6 +345,38 @@ class PluginManager {
   static SymbolVendorCreateInstance
   GetSymbolVendorCreateCallbackAtIndex(uint32_t idx);
 
+  // SymbolLocator
+  static bool RegisterPlugin(
+      llvm::StringRef name, llvm::StringRef description,
+      SymbolLocatorCreateInstance create_callback,
+      SymbolLocatorLocateExecutableObjectFile locate_executable_object_file =
+          nullptr,
+      SymbolLocatorLocateExecutableSymbolFile locate_executable_symbol_file =
+          nullptr,
+      SymbolLocatorDownloadObjectAndSymbolFile download_object_symbol_file =
+          nullptr,
+      SymbolLocatorFindSymbolFileInBundle find_symbol_file_in_bundle = nullptr);
+
+  static bool UnregisterPlugin(SymbolLocatorCreateInstance create_callback);
+
+  static SymbolLocatorCreateInstance
+  GetSymbolLocatorCreateCallbackAtIndex(uint32_t idx);
+
+  static ModuleSpec LocateExecutableObjectFile(const ModuleSpec &module_spec);
+
+  static FileSpec
+  LocateExecutableSymbolFile(const ModuleSpec &module_spec,
+                             const FileSpecList &default_search_paths);
+
+  static bool DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
+                                          Status &error,
+                                          bool force_lookup = true,
+                                          bool copy_executable = true);
+
+  static FileSpec FindSymbolFileInBundle(const FileSpec &dsym_bundle_fspec,
+                                         const UUID *uuid,
+                                         const ArchSpec *arch);
+
   // Trace
   static bool RegisterPlugin(
       llvm::StringRef name, llvm::StringRef description,

diff --git a/lldb/include/lldb/Symbol/LocateSymbolFile.h b/lldb/include/lldb/Symbol/LocateSymbolFile.h
@@ -24,38 +24,6 @@ class UUID;
 
 class Symbols {
 public:
-  // Locate the executable file given a module specification.
-  //
-  // Locating the file should happen only on the local computer or using the
-  // current computers global settings.
-  static ModuleSpec LocateExecutableObjectFile(const ModuleSpec &module_spec);
-
-  // Locate the symbol file given a module specification.
-  //
-  // Locating the file should happen only on the local computer or using the
-  // current computers global settings.
-  static FileSpec
-  LocateExecutableSymbolFile(const ModuleSpec &module_spec,
-                             const FileSpecList &default_search_paths);
-
-  static FileSpec FindSymbolFileInBundle(const FileSpec &dsym_bundle_fspec,
-                                         const lldb_private::UUID *uuid,
-                                         const ArchSpec *arch);
-
-  // Locate the object and symbol file given a module specification.
-  //
-  // Locating the file can try to download the file from a corporate build
-  // repository, or using any other means necessary to locate both the
-  // unstripped object file and the debug symbols. The force_lookup argument
-  // controls whether the external program is called unconditionally to find
-  // the symbol file, or if the user's settings are checked to see if they've
-  // enabled the external program before calling.
-  //
-  static bool DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
-                                          Status &error,
-                                          bool force_lookup = true,
-                                          bool copy_executable = true);
-
   /// Locate the symbol file for the given UUID on a background thread. This
   /// function returns immediately. Under the hood it uses the debugger's
   /// thread pool to call DownloadObjectAndSymbolFile. If a symbol file is

diff --git a/lldb/include/lldb/Symbol/SymbolLocator.h b/lldb/include/lldb/Symbol/SymbolLocator.h
@@ -0,0 +1,31 @@
+//===-- SymbolLocator.h -----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_SYMBOL_SYMBOLLOCATOR_H
+#define LLDB_SYMBOL_SYMBOLLOCATOR_H
+
+#include "lldb/Core/PluginInterface.h"
+#include "lldb/Utility/UUID.h"
+
+namespace lldb_private {
+
+class SymbolLocator : public PluginInterface {
+public:
+  SymbolLocator() = default;
+
+  /// Locate the symbol file for the given UUID on a background thread. This
+  /// function returns immediately. Under the hood it uses the debugger's
+  /// thread pool to call DownloadObjectAndSymbolFile. If a symbol file is
+  /// found, this will notify all target which contain the module with the
+  /// given UUID.
+  static void DownloadSymbolFileAsync(const UUID &uuid);
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_SYMBOL_SYMBOLFILELOCATOR_H
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
@@ -1332,6 +1332,12 @@ enum CompletionType {
   eCustomCompletion = (1u << 25)
 };
 
+enum SymbolDownload {
+  eSymbolDownloadOff = 0,
+  eSymbolDownloadBackground = 1,
+  eSymbolDownloadForeground = 2,
+};
+
 /// Specifies if children need to be re-computed
 /// after a call to \ref SyntheticChildrenFrontEnd::Update.
 enum class ChildCacheState {

diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h
@@ -220,6 +220,7 @@ class SymbolContextScope;
 class SymbolContextSpecifier;
 class SymbolFile;
 class SymbolFileType;
+class SymbolLocator;
 class SymbolVendor;
 class Symtab;
 class SyntheticChildren;

diff --git a/lldb/include/lldb/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h
@@ -91,6 +91,16 @@ typedef SymbolVendor *(*SymbolVendorCreateInstance)(
     const lldb::ModuleSP &module_sp,
     lldb_private::Stream
         *feedback_strm); // Module can be NULL for default system symbol vendor
+typedef SymbolLocator *(*SymbolLocatorCreateInstance)();
+typedef std::optional<ModuleSpec> (*SymbolLocatorLocateExecutableObjectFile)(
+    const ModuleSpec &module_spec);
+typedef std::optional<FileSpec> (*SymbolLocatorFindSymbolFileInBundle)(
+    const FileSpec &dsym_bundle_fspec, const UUID *uuid, const ArchSpec *arch);
+typedef std::optional<FileSpec> (*SymbolLocatorLocateExecutableSymbolFile)(
+    const ModuleSpec &module_spec, const FileSpecList &default_search_paths);
+typedef bool (*SymbolLocatorDownloadObjectAndSymbolFile)(
+    ModuleSpec &module_spec, Status &error, bool force_lookup,
+    bool copy_executable);
 typedef bool (*BreakpointHitCallback)(void *baton,
                                       StoppointCallbackContext *context,
                                       lldb::user_id_t break_id,

diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
@@ -7,7 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/API/SBTarget.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Utility/Instrumentation.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/lldb-public.h"
@@ -39,6 +38,7 @@
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
 #include "lldb/Core/SearchFilter.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Core/StructuredDataImpl.h"
@@ -1561,8 +1561,9 @@ lldb::SBModule SBTarget::AddModule(const SBModuleSpec &module_spec) {
                                                  true /* notify */));
     if (!sb_module.IsValid() && module_spec.m_opaque_up->GetUUID().IsValid()) {
       Status error;
-      if (Symbols::DownloadObjectAndSymbolFile(*module_spec.m_opaque_up, error,
-                                               /* force_lookup */ true)) {
+      if (PluginManager::DownloadObjectAndSymbolFile(*module_spec.m_opaque_up,
+                                                     error,
+                                                     /* force_lookup */ true)) {
         if (FileSystem::Instance().Exists(
                 module_spec.m_opaque_up->GetFileSpec())) {
           sb_module.SetSP(target_sp->GetOrCreateModule(*module_spec.m_opaque_up,

diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Core/IOHandler.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Core/ValueObjectVariable.h"
 #include "lldb/DataFormatters/ValueObjectPrinter.h"
@@ -35,7 +36,6 @@
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/FuncUnwinders.h"
 #include "lldb/Symbol/LineTable.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/UnwindPlan.h"
@@ -2533,7 +2533,7 @@ class CommandObjectTargetModulesAdd : public CommandObjectParsed {
           module_spec.GetSymbolFileSpec() =
               m_symbol_file.GetOptionValue().GetCurrentValue();
         Status error;
-        if (Symbols::DownloadObjectAndSymbolFile(module_spec, error)) {
+        if (PluginManager::DownloadObjectAndSymbolFile(module_spec, error)) {
           ModuleSP module_sp(
               target->GetOrCreateModule(module_spec, true /* notify */));
           if (module_sp) {
@@ -4229,7 +4229,7 @@ class CommandObjectTargetSymbolsAdd : public CommandObjectParsed {
   bool DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
                                    CommandReturnObject &result, bool &flush) {
     Status error;
-    if (Symbols::DownloadObjectAndSymbolFile(module_spec, error)) {
+    if (PluginManager::DownloadObjectAndSymbolFile(module_spec, error)) {
       if (module_spec.GetSymbolFileSpec())
         return AddModuleSymbols(m_exe_ctx.GetTargetPtr(), module_spec, flush,
                                 result);

diff --git 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/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp
@@ -14,7 +14,6 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Platform.h"
@@ -219,9 +218,9 @@ ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
     if (!module_sp) {
       FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
       module_spec.GetSymbolFileSpec() =
-          Symbols::LocateExecutableSymbolFile(module_spec, search_paths);
+          PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
       ModuleSpec objfile_module_spec =
-          Symbols::LocateExecutableObjectFile(module_spec);
+          PluginManager::LocateExecutableObjectFile(module_spec);
       module_spec.GetFileSpec() = objfile_module_spec.GetFileSpec();
       if (FileSystem::Instance().Exists(module_spec.GetFileSpec()) &&
           FileSystem::Instance().Exists(module_spec.GetSymbolFileSpec())) {
@@ -232,8 +231,8 @@ ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
     // If we haven't found a binary, or we don't have a SymbolFile, see
     // if there is an external search tool that can find it.
     if (!module_sp || !module_sp->GetSymbolFileFileSpec()) {
-      Symbols::DownloadObjectAndSymbolFile(module_spec, error,
-                                           force_symbol_search);
+      PluginManager::DownloadObjectAndSymbolFile(module_spec, error,
+                                                 force_symbol_search);
       if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
         module_sp = std::make_shared<Module>(module_spec);
       } else if (force_symbol_search && error.AsCString("") &&

diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
@@ -23,11 +23,11 @@
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Symbol/SymbolFile.h"
+#include "lldb/Symbol/SymbolLocator.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/Symtab.h"
 #include "lldb/Symbol/Type.h"
@@ -1345,7 +1345,7 @@ UnwindTable &Module::GetUnwindTable() {
   if (!m_unwind_table) {
     m_unwind_table.emplace(*this);
     if (!m_symfile_spec)
-      Symbols::DownloadSymbolFileAsync(GetUUID());
+      SymbolLocator::DownloadSymbolFileAsync(GetUUID());
   }
   return *m_unwind_table;
 }

diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
@@ -9,12 +9,12 @@
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/OptionValueFileSpec.h"
 #include "lldb/Interpreter/OptionValueFileSpecList.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Interpreter/Property.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Symbol/TypeList.h"
@@ -137,10 +137,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 {
@@ -1098,7 +1103,7 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp,
   // Fixup the incoming path in case the path points to a valid file, yet the
   // arch or UUID (if one was passed in) don't match.
   ModuleSpec located_binary_modulespec =
-      Symbols::LocateExecutableObjectFile(module_spec);
+      PluginManager::LocateExecutableObjectFile(module_spec);
 
   // Don't look for the file if it appears to be the same one we already
   // checked for above...