llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) <details> <summary>Changes</summary> This builds on top of the work started in c3a302d to convert LocateSymbolFile to a SymbolLocator plugin. This commit moves DownloadObjectAndSymbolFile. --- Patch is 64.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71267.diff 17 Files Affected: - (modified) lldb/include/lldb/Core/PluginManager.h (+7) - (modified) lldb/include/lldb/Symbol/LocateSymbolFile.h (-14) - (modified) lldb/include/lldb/lldb-private-interfaces.h (+4) - (modified) lldb/source/API/SBTarget.cpp (+4-2) - (modified) lldb/source/Commands/CommandObjectTarget.cpp (+3-2) - (modified) lldb/source/Core/DynamicLoader.cpp (+2-2) - (modified) lldb/source/Core/PluginManager.cpp (+21-1) - (modified) lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp (+2-2) - (modified) lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp (+2-1) - (modified) lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (+2-2) - (modified) lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp (+362-3) - (modified) lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.h (+12) - (modified) lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp (+9-1) - (modified) lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.h (+12) - (modified) lldb/source/Symbol/CMakeLists.txt (-10) - (modified) lldb/source/Symbol/LocateSymbolFile.cpp (+4-19) - (removed) lldb/source/Symbol/LocateSymbolFileMacOSX.cpp (-649) ``````````diff diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h index 50f0662f4bb2b95..318f8b63c251a19 100644 --- a/lldb/include/lldb/Core/PluginManager.h +++ b/lldb/include/lldb/Core/PluginManager.h @@ -353,6 +353,8 @@ class PluginManager { 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); @@ -366,6 +368,11 @@ class PluginManager { 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); diff --git a/lldb/include/lldb/Symbol/LocateSymbolFile.h b/lldb/include/lldb/Symbol/LocateSymbolFile.h index 3c0f069c27f8dd6..9247814a9e2ab22 100644 --- a/lldb/include/lldb/Symbol/LocateSymbolFile.h +++ b/lldb/include/lldb/Symbol/LocateSymbolFile.h @@ -24,20 +24,6 @@ class UUID; class Symbols { public: - // 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/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h index 4c324a721029075..65181dec644e20d 100644 --- a/lldb/include/lldb/lldb-private-interfaces.h +++ b/lldb/include/lldb/lldb-private-interfaces.h @@ -96,6 +96,10 @@ 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 index a4a7ac338c20768..03134ff54fa46e9 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -38,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" @@ -1525,8 +1526,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 index ca8484cc79d4054..180210d45b7e000 100644 --- 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" @@ -2801,7 +2802,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) { @@ -4497,7 +4498,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/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp index efed29da0835ebf..f4baaf450ee97ac 100644 --- a/lldb/source/Core/DynamicLoader.cpp +++ b/lldb/source/Core/DynamicLoader.cpp @@ -232,8 +232,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/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index 810e487f723de06..23c06357e2f95f3 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -1090,15 +1090,18 @@ struct SymbolLocatorInstance CallbackType create_callback, SymbolLocatorLocateExecutableObjectFile locate_executable_object_file, SymbolLocatorLocateExecutableSymbolFile locate_executable_symbol_file, + SymbolLocatorDownloadObjectAndSymbolFile download_object_symbol_file, SymbolLocatorFindSymbolFileInBundle find_symbol_file_in_bundle) : PluginInstance<SymbolLocatorCreateInstance>(name, description, create_callback), locate_executable_object_file(locate_executable_object_file), locate_executable_symbol_file(locate_executable_symbol_file), + download_object_symbol_file(download_object_symbol_file), find_symbol_file_in_bundle(find_symbol_file_in_bundle) {} SymbolLocatorLocateExecutableObjectFile locate_executable_object_file; SymbolLocatorLocateExecutableSymbolFile locate_executable_symbol_file; + SymbolLocatorDownloadObjectAndSymbolFile download_object_symbol_file; SymbolLocatorFindSymbolFileInBundle find_symbol_file_in_bundle; }; typedef PluginInstances<SymbolLocatorInstance> SymbolLocatorInstances; @@ -1113,10 +1116,12 @@ bool PluginManager::RegisterPlugin( SymbolLocatorCreateInstance create_callback, SymbolLocatorLocateExecutableObjectFile locate_executable_object_file, SymbolLocatorLocateExecutableSymbolFile locate_executable_symbol_file, + SymbolLocatorDownloadObjectAndSymbolFile download_object_symbol_file, SymbolLocatorFindSymbolFileInBundle find_symbol_file_in_bundle) { return GetSymbolLocatorInstances().RegisterPlugin( name, description, create_callback, locate_executable_object_file, - locate_executable_symbol_file, find_symbol_file_in_bundle); + locate_executable_symbol_file, download_object_symbol_file, + find_symbol_file_in_bundle); } bool PluginManager::UnregisterPlugin( @@ -1157,6 +1162,21 @@ FileSpec PluginManager::LocateExecutableSymbolFile( return {}; } +bool PluginManager::DownloadObjectAndSymbolFile(ModuleSpec &module_spec, + Status &error, + bool force_lookup, + bool copy_executable) { + auto &instances = GetSymbolLocatorInstances().GetInstances(); + for (auto &instance : instances) { + if (instance.download_object_symbol_file) { + if (instance.download_object_symbol_file(module_spec, error, force_lookup, + copy_executable)) + return true; + } + } + return false; +} + FileSpec PluginManager::FindSymbolFileInBundle(const FileSpec &symfile_bundle, const UUID *uuid, const ArchSpec *arch) { diff --git a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp index 5aeaf3ae24d7c7b..a255a3b9e760c94 100644 --- a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp +++ b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp @@ -798,8 +798,8 @@ bool DynamicLoaderDarwinKernel::KextImageInfo::LoadImageUsingMemoryModule( Status kernel_search_error; if (IsKernel() && (!m_module_sp || !m_module_sp->GetSymbolFileFileSpec())) { - if (Symbols::DownloadObjectAndSymbolFile(module_spec, - kernel_search_error, true)) { + if (PluginManager::DownloadObjectAndSymbolFile( + module_spec, kernel_search_error, true)) { if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) { m_module_sp = std::make_shared<Module>(module_spec.GetFileSpec(), target.GetArchitecture()); diff --git a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp index a1bf8efb064b614..8235cc7f1ba6445 100644 --- a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp +++ b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp @@ -345,7 +345,8 @@ bool DynamicLoaderFreeBSDKernel::KModImageInfo::LoadImageUsingMemoryModule( ModuleSpec module_spec(FileSpec(GetPath()), target.GetArchitecture()); if (IsKernel()) { Status error; - if (Symbols::DownloadObjectAndSymbolFile(module_spec, error, true)) { + if (PluginManager::DownloadObjectAndSymbolFile(module_spec, error, + true)) { if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) m_module_sp = std::make_shared<Module>(module_spec.GetFileSpec(), target.GetArchitecture()); diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp index 9af544b31d89649..d10069f17cbbe3b 100644 --- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp +++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp @@ -292,8 +292,8 @@ Status ProcessKDP::DoConnectRemote(llvm::StringRef remote_url) { if (!module_spec.GetSymbolFileSpec() || !module_spec.GetSymbolFileSpec()) { Status symbl_error; - Symbols::DownloadObjectAndSymbolFile(module_spec, symbl_error, - true); + PluginManager::DownloadObjectAndSymbolFile(module_spec, + symbl_error, true); } if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) { diff --git a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp index 2c57ef8b6a54ea0..fb94e5cb31d49bf 100644 --- a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp +++ b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp @@ -66,7 +66,7 @@ void SymbolLocatorDebugSymbols::Initialize() { PluginManager::RegisterPlugin( GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance, LocateExecutableObjectFile, LocateExecutableSymbolFile, - FindSymbolFileInBundle); + DownloadObjectAndSymbolFile, FindSymbolFileInBundle); } void SymbolLocatorDebugSymbols::Terminate() { @@ -390,7 +390,7 @@ static bool FileAtPathContainsArchAndUUID(const FileSpec &file_fspec, // return true if there is a matching dSYM bundle next to the exec_fspec, // and return that value in dsym_fspec. // If there is a .dSYM.yaa compressed archive next to the exec_fspec, -// call through Symbols::DownloadObjectAndSymbolFile to download the +// call through PluginManager::DownloadObjectAndSymbolFile to download the // expanded/uncompressed dSYM and return that filepath in dsym_fspec. static bool LookForDsymNextToExecutablePath(const ModuleSpec &mod_spec, const FileSpec &exec_fspec, @@ -447,7 +447,8 @@ static bool LookForDsymNextToExecutablePath(const ModuleSpec &mod_spec, if (FileSystem::Instance().Exists(dsym_yaa_fspec)) { ModuleSpec mutable_mod_spec = mod_spec; Status error; - if (Symbols::DownloadObjectAndSymbolFile(mutable_mod_spec, error, true) && + if (PluginManager::DownloadObjectAndSymbolFile(mutable_mod_spec, error, + true) && FileSystem::Instance().Exists(mutable_mod_spec.GetSymbolFileSpec())) { dsym_fspec = mutable_mod_spec.GetSymbolFileSpec(); return true; @@ -789,3 +790,361 @@ std::optional<FileSpec> SymbolLocatorDebugSymbols::LocateExecutableSymbolFile( return dsym_module_spec.GetSymbolFileSpec(); } + +static bool GetModuleSpecInfoFromUUIDDictionary(CFDictionaryRef uuid_dict, + ModuleSpec &module_spec, + Status &error, + const std::string &command) { + Log *log = GetLog(LLDBLog::Host); + bool success = false; + if (uuid_dict != NULL && CFGetTypeID(uuid_dict) == CFDictionaryGetTypeID()) { + std::string str; + CFStringRef cf_str; + CFDictionaryRef cf_dict; + + cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict, + CFSTR("DBGError")); + if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) { + if (CFCString::FileSystemRepresentation(cf_str, str)) { + std::string errorstr = command; + errorstr += ":\n"; + errorstr += str; + error.SetErrorString(errorstr); + } + } + + cf_str = (CFStringRef)CFDictionaryGetValue( + (CFDictionaryRef)uuid_dict, CFSTR("DBGSymbolRichExecutable")); + if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) { + if (CFCString::FileSystemRepresentation(cf_str, str)) { + module_spec.GetFileSpec().SetFile(str.c_str(), FileSpec::Style::native); + FileSystem::Instance().Resolve(module_spec.GetFileSpec()); + LLDB_LOGF(log, + "From dsymForUUID plist: Symbol rich executable is at '%s'", + str.c_str()); + } + } + + cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict, + CFSTR("DBGDSYMPath")); + if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) { + if (CFCString::FileSystemRepresentation(cf_str, str)) { + module_spec.GetSymbolFileSpec().SetFile(str.c_str(), + FileSpec::Style::native); + FileSystem::Instance().Resolve(module_spec.GetFileSpec()); + success = true; + LLDB_LOGF(log, "From dsymForUUID plist: dSYM is at '%s'", str.c_str()); + } + } + + std::string DBGBuildSourcePath; + std::string DBGSourcePath; + + // If DBGVersion 1 or DBGVersion missing, ignore DBGSourcePathRemapping. + // If DBGVersion 2, strip last two components of path remappings from + // entries to fix an issue with a specific set of + // DBGSourcePathRemapping entries that lldb worked + // with. + // If DBGVersion 3, trust & use the source path remappings as-is. + // + cf_dict = (CFDictionaryRef)CFDictionaryGetValue( + (CFDictionaryRef)uuid_dict, CFSTR("DBGSourcePathRemapping")); + if (cf_dict && CFGetTypeID(cf_dict) == CFDictionaryGetTypeID()) { + // If we see DBGVersion with a value of 2 or higher, this is a new style + // DBGSourcePathRemapping dictionary + bool new_style_source_remapping_dictionary = false; + bool do_truncate_remapping_names = false; + std::string original_DBGSourcePath_value = DBGSourcePath; + cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict, + CFSTR("DBGVersion")); + if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) { + std::string version; + CFCString::FileSystemRepresentation(cf_str, version); + if (!version.empty() && isdigit(version[0])) { + int version_number = atoi(version.c_str()); + if (version_number > 1) { + new_style_source_remapping_dictionary = true; + } + if (version_number == 2) { + do_truncate_remapping_names = true; + } + } + } + + CFIndex kv_pair_count = CFDictionaryGetCount((CFDictionaryRef)uuid_dict); + if (kv_pair_count > 0) { + CFStringRef *keys = + (CFStringRef *)malloc(kv_pair_count * sizeof(CFStringRef)); + CFStringRef *values = + (CFStringRef *)malloc(kv_pair_count * sizeof(CFStringRef)); + if (keys != nullptr && values != nullptr) { + CFDictionaryGetKeysAndValues((CFDictionaryRef)uuid_dict, + (const void **)keys, + (const void **)values); + } + for (CFIndex i = 0; i < kv_pair_count; i++) { + DBGBuildSourcePath.clear(); + DBGSourcePath.clear(); + if (keys[i] && CFGetTypeID(keys[i]) == CFStringGetTypeID()) { + CFCString::FileSystemRepresentation(keys[i], DBGBuildSourcePath); + } + if (values[i] && CFGetTypeID(values[i]) == CFStringGetTypeID()) { + CFCString::FileSystemRepresentation(values[i], DBGSourcePath); + } ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/71267 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits