JDevlieghere created this revision. JDevlieghere added reviewers: jasonmolenda, jingham, mib, clayborg. Herald added a project: All. JDevlieghere requested review of this revision.
On macOS, LLDB uses the DebugSymbols.framework to locate symbol rich dSYM bundles. [1] The framework uses a variety of methods, one of them calling into a binary or shell script to locate (and download) dSYMs. Internally at Apple, that tool is called `dsymForUUID` and for simplicity I'm just going to refer to it that way here too, even though it can be be an arbitrary executable. The most common use case for dsymForUUID is to fetch symbols from the network. This can take a long time, and because the calls to the DebugSymbols.framework are blocking, lldb does not launch the process immediately. This is the expected behavior and many people therefore often don't use this functionality, but instead use `add-dsym` when they want symbols for a given frame, backtrace or module. This is a little faster because you're only fetching symbols for the module you care about, but it's still a slow, blocking operation. This patch introduces a hybrid approach between the two. When `symbols.enable-background-lookup` is enabled, lldb will do the equivalent of `add-dsym` for every module it doesn't have symbols for in the background. From the user's perspective there is no slowdown, because the process launches immediately, with whatever symbols are available and more symbol information is added over time as the background fetching completes. [1] https://lldb.llvm.org/use/symbols.html https://reviews.llvm.org/D131328 Files: lldb/include/lldb/Core/Debugger.h lldb/include/lldb/Core/ModuleList.h lldb/include/lldb/Symbol/LocateSymbolFile.h lldb/include/lldb/Target/TargetList.h lldb/source/Core/CoreProperties.td lldb/source/Core/Debugger.cpp lldb/source/Core/ModuleList.cpp lldb/source/Symbol/LocateSymbolFile.cpp lldb/source/Symbol/LocateSymbolFileMacOSX.cpp lldb/source/Target/TargetList.cpp
Index: lldb/source/Target/TargetList.cpp =================================================================== --- lldb/source/Target/TargetList.cpp +++ lldb/source/Target/TargetList.cpp @@ -540,3 +540,11 @@ m_selected_target_idx = 0; return GetTargetAtIndex(m_selected_target_idx); } + +std::vector<TargetWP> TargetList::GetCurrentTargets() { + std::vector<TargetWP> targets; + std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex); + for (TargetSP target_sp : m_target_list) + targets.push_back(target_sp); + return targets; +} Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp =================================================================== --- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp +++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp @@ -492,7 +492,8 @@ } bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec, - Status &error, bool force_lookup) { + Status &error, bool force_lookup, + bool copy_executable) { bool success = false; const UUID *uuid_ptr = module_spec.GetUUIDPtr(); const FileSpec *file_spec_ptr = module_spec.GetFileSpecPtr(); @@ -592,12 +593,16 @@ file_spec_ptr->GetPath(file_path, sizeof(file_path)); StreamString command; + const char *copy_executable_str = + copy_executable ? " --copyExecutable" : ""; if (!uuid_str.empty()) - command.Printf("%s --ignoreNegativeCache --copyExecutable %s", - g_dsym_for_uuid_exe_path, uuid_str.c_str()); + command.Printf("%s --ignoreNegativeCache %s%s", + g_dsym_for_uuid_exe_path, copy_executable_str, + uuid_str.c_str()); else if (file_path[0] != '\0') - command.Printf("%s --ignoreNegativeCache --copyExecutable %s", - g_dsym_for_uuid_exe_path, file_path); + command.Printf("%s --ignoreNegativeCache %s%s", + g_dsym_for_uuid_exe_path, copy_executable_str, + file_path); if (!command.GetString().empty()) { Log *log = GetLog(LLDBLog::Host); Index: lldb/source/Symbol/LocateSymbolFile.cpp =================================================================== --- lldb/source/Symbol/LocateSymbolFile.cpp +++ lldb/source/Symbol/LocateSymbolFile.cpp @@ -8,6 +8,8 @@ #include "lldb/Symbol/LocateSymbolFile.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Core/Module.h" #include "lldb/Core/ModuleList.h" #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/Progress.h" @@ -24,6 +26,7 @@ #include "lldb/Utility/UUID.h" #include "llvm/Support/FileSystem.h" +#include "llvm/Support/ThreadPool.h" // From MacOSX system header "mach/machine.h" typedef int cpu_type_t; @@ -223,7 +226,8 @@ // an appropriate parent directory if (!LocateDSYMInVincinityOfExecutable(module_spec, symbol_fspec)) { // We failed to easily find the dSYM above, so use DebugSymbols - LocateMacOSXFilesUsingDebugSymbols(module_spec, dsym_module_spec); + if (!LocateMacOSXFilesUsingDebugSymbols(module_spec, dsym_module_spec)) + Symbols::DownloadSymbolFileAsync(module_spec); } else { dsym_module_spec.GetSymbolFileSpec() = symbol_fspec; } @@ -248,7 +252,8 @@ module_specs.FindMatchingModuleSpec(module_spec, matched_module_spec)) { result.GetFileSpec() = exec_fspec; } else { - LocateMacOSXFilesUsingDebugSymbols(module_spec, result); + if (!LocateMacOSXFilesUsingDebugSymbols(module_spec, result)) + DownloadSymbolFileAsync(module_spec); } return result; @@ -397,6 +402,67 @@ return LocateExecutableSymbolFileDsym(module_spec); } +void Symbols::DownloadSymbolFileAsync(const ModuleSpec &module_spec) { + if (!ModuleList::GetGlobalModuleListProperties().GetEnableBackgroundLookup()) + return; + + Debugger::GetThreadPool().async([=]() { + const UUID *uuid = module_spec.GetUUIDPtr(); + if (!uuid) + return; + + Status error; + ModuleSpec out_module_spec; + out_module_spec.GetUUID() = *uuid; + if (!Symbols::DownloadObjectAndSymbolFile(out_module_spec, error, + /*force_lookup=*/true, + /*copy_executable=*/false)) + return; + + if (error.Fail()) + return; + + const FileSpec &symbol_fspec = out_module_spec.GetSymbolFileSpec(); + if (!symbol_fspec) + return; + + // Find the corresponding module. + ModuleSP module_sp = ModuleList::FindSharedModule(*uuid); + + // Make sure a symbol file wasn't added in the meantime. + if (module_sp->GetSymbolFileFileSpec()) + return; + + // Set the symbol file in the module. + module_sp->SetSymbolFileFileSpec(symbol_fspec); + + // Iterate over all the targets and notify the ones that contain our + // module. We need to be careful about the underlying debugger and target + // list getting modified. Luckily we don't need a lock for that. For + // correctness it doesn't matter if a debugger or target got deleted in the + // meantime. If a new target was created, we will have already set the + // symbol file in the shared module list. + for (DebuggerWP debugger_wp : Debugger::GetCurrentDebuggers()) { + DebuggerSP debugger_sp = debugger_wp.lock(); + if (!debugger_sp) + continue; + + for (TargetWP target_wp : + debugger_sp->GetTargetList().GetCurrentTargets()) { + TargetSP target_sp = target_wp.lock(); + if (!target_sp) + continue; + + if (target_sp->GetImages().FindModule(module_sp.get())) { + ModuleList module_list; + module_list.Append(module_sp); + target_sp->SymbolsDidLoad(module_list); + } + } + } + }); +} + #if !defined(__APPLE__) FileSpec Symbols::FindSymbolFileInBundle(const FileSpec &symfile_bundle, @@ -407,7 +473,8 @@ } bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec, - Status &error, bool force_lookup) { + Status &error, bool force_lookup, + bool copy_executable) { // Fill in the module_spec.GetFileSpec() for the object file and/or the // module_spec.GetSymbolFileSpec() for the debug symbols file. return false; Index: lldb/source/Core/ModuleList.cpp =================================================================== --- lldb/source/Core/ModuleList.cpp +++ lldb/source/Core/ModuleList.cpp @@ -106,6 +106,12 @@ nullptr, ePropertyEnableExternalLookup, new_value); } +bool ModuleListProperties::GetEnableBackgroundLookup() const { + const uint32_t idx = ePropertyEnableBackgroundLookup; + return m_collection_sp->GetPropertyAtIndexAsBoolean( + nullptr, idx, g_modulelist_properties[idx].default_uint_value != 0); +} + FileSpec ModuleListProperties::GetClangModulesCachePath() const { return m_collection_sp ->GetPropertyAtIndexAsOptionValueFileSpec(nullptr, false, @@ -768,6 +774,10 @@ GetSharedModuleList().FindModules(module_spec, matching_module_list); } +lldb::ModuleSP ModuleList::FindSharedModule(const UUID &uuid) { + return GetSharedModuleList().FindModule(uuid); +} + size_t ModuleList::RemoveOrphanSharedModules(bool mandatory) { return GetSharedModuleList().RemoveOrphans(mandatory); } Index: lldb/source/Core/Debugger.cpp =================================================================== --- lldb/source/Core/Debugger.cpp +++ lldb/source/Core/Debugger.cpp @@ -659,6 +659,16 @@ return debugger_sp; } +std::vector<DebuggerWP> Debugger::GetCurrentDebuggers() { + std::vector<DebuggerWP> debuggers; + if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) { + std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr); + for (DebuggerSP debugger_sp : *g_debugger_list_ptr) + debuggers.push_back(debugger_sp); + } + return debuggers; +} + void Debugger::Destroy(DebuggerSP &debugger_sp) { if (!debugger_sp) return; Index: lldb/source/Core/CoreProperties.td =================================================================== --- lldb/source/Core/CoreProperties.td +++ lldb/source/Core/CoreProperties.td @@ -5,6 +5,10 @@ Global, DefaultTrue, Desc<"Control the use of external tools and repositories to locate symbol files. Directories listed in target.debug-file-search-paths and directory of the executable are always checked first for separate debug info files. Then depending on this setting: On macOS, Spotlight would be also used to locate a matching .dSYM bundle based on the UUID of the executable. On NetBSD, directory /usr/libdata/debug would be also searched. On platforms other than NetBSD directory /usr/lib/debug would be also searched.">; + 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.">; def ClangModulesCachePath: Property<"clang-modules-cache-path", "FileSpec">, Global, DefaultStringValue<"">, Index: lldb/include/lldb/Target/TargetList.h =================================================================== --- lldb/include/lldb/Target/TargetList.h +++ lldb/include/lldb/Target/TargetList.h @@ -189,6 +189,8 @@ return TargetIterable(m_target_list, m_target_list_mutex); } + std::vector<lldb::TargetWP> GetCurrentTargets(); + private: collection m_target_list; mutable std::recursive_mutex m_target_list_mutex; Index: lldb/include/lldb/Symbol/LocateSymbolFile.h =================================================================== --- lldb/include/lldb/Symbol/LocateSymbolFile.h +++ lldb/include/lldb/Symbol/LocateSymbolFile.h @@ -14,6 +14,7 @@ #include "lldb/Core/FileSpecList.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/Status.h" +#include "lldb/lldb-forward.h" namespace lldb_private { @@ -52,7 +53,10 @@ // static bool DownloadObjectAndSymbolFile(ModuleSpec &module_spec, Status &error, - bool force_lookup = true); + bool force_lookup = true, + bool copy_executable = true); + + static void DownloadSymbolFileAsync(const ModuleSpec &module_spec); }; } // namespace lldb_private Index: lldb/include/lldb/Core/ModuleList.h =================================================================== --- lldb/include/lldb/Core/ModuleList.h +++ lldb/include/lldb/Core/ModuleList.h @@ -60,6 +60,7 @@ 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(); @@ -457,6 +458,8 @@ static void FindSharedModules(const ModuleSpec &module_spec, ModuleList &matching_module_list); + static lldb::ModuleSP FindSharedModule(const UUID &uuid); + static size_t RemoveOrphanSharedModules(bool mandatory); static bool RemoveSharedModuleIfOrphaned(const Module *module_ptr); Index: lldb/include/lldb/Core/Debugger.h =================================================================== --- lldb/include/lldb/Core/Debugger.h +++ lldb/include/lldb/Core/Debugger.h @@ -96,6 +96,8 @@ CreateInstance(lldb::LogOutputCallback log_callback = nullptr, void *baton = nullptr); + static std::vector<lldb::DebuggerWP> GetCurrentDebuggers(); + static lldb::TargetSP FindTargetWithProcessID(lldb::pid_t pid); static lldb::TargetSP FindTargetWithProcess(Process *process);
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits