llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) <details> <summary>Changes</summary> LoadBinaryWithUUIDAndAddress both searched for a binary and registered it with the Target. Split it into LocateBinaries, which only searches, and LoadBinaryInTarget, which mutates the Target, with LocateAndLoadBinary keeping the single binary case a one-liner. The eight binary parameters and the results of the search are bundled in a new BinarySpec struct, and both entry points return an llvm::Expected. The motivation is a follow-up that runs LocateBinaries in parallel on the thread pool. NFC, except for some small improvements to the error handling because we don't write to the async output stream directly (and fixed the newline). --- Patch is 34.07 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/214372.diff 5 Files Affected: - (modified) lldb/include/lldb/Target/DynamicLoader.h (+122-58) - (modified) lldb/source/Core/DynamicLoader.cpp (+174-139) - (modified) lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (+21-8) - (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+22-19) - (modified) lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp (+38-29) ``````````diff diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h index e85036c2887d1..e5500a5f0a2f1 100644 --- a/lldb/include/lldb/Target/DynamicLoader.h +++ b/lldb/include/lldb/Target/DynamicLoader.h @@ -20,8 +20,12 @@ #include "lldb/lldb-private-enumerations.h" #include "lldb/lldb-types.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/Support/Error.h" + #include <cstddef> #include <cstdint> +#include <string> namespace lldb_private { class ModuleList; class Process; @@ -212,72 +216,132 @@ class DynamicLoader : public PluginInterface { lldb::addr_t base_addr, bool base_addr_is_offset); - /// Find/load a binary into lldb given a UUID and the address where it is - /// loaded in memory, or a slide to be applied to the file address. - /// May force an expensive search on the computer to find the binary by - /// UUID, should not be used for a large number of binaries - intended for - /// an environment where there may be one, or a few, binaries resident in - /// memory. + /// A binary to find and load into a Target, and the state that finding it + /// produces. + /// + /// Loading a binary is split into LocateBinaries and LoadBinaryInTarget so + /// that a caller with many binaries can search for all of them before adding + /// any of them to the Target. + struct BinarySpec { + /// Name of the binary, if available. If no matching binary can be found on + /// the debug host, a module may be created out of live memory and given + /// this name. If empty, a name is constructed from the address the binary + /// is loaded at. + std::string name; + + /// UUID of the binary to be loaded. May be empty, in which case, if a load + /// address is supplied, the binary is read out of memory to get a UUID to + /// look for. There is a performance cost to doing this, it is not + /// preferable. + UUID uuid; + + /// Address where the binary should be loaded, or read out of memory. Or a + /// slide value, to be applied to the file addresses of the binary. + lldb::addr_t value = LLDB_INVALID_ADDRESS; + + /// A flag indicating that \a value is an address, or an offset to be + /// applied to the file addresses. + bool value_is_offset = false; + + /// Allow the search to do a possibly expensive external search for the + /// ObjectFile and/or SymbolFile. + bool force_symbol_search = false; + + /// Whether ModulesDidLoad should be called once the binary has been added + /// to the Target. A caller loading several binaries may prefer to batch + /// those up. + bool notify = false; + + /// Whether the address of the binary should be set in the Target if it is + /// added. A caller that wants to set the section addresses individually + /// leaves this clear, and is then responsible for setting the load address + /// for the binary or its segments in the Target. + bool set_address_in_target = false; + + /// If no better binary image can be found, allow reading the binary out of + /// memory, if possible, and create the Module based on that. May be slow + /// to read a binary out of memory, and for unusual environments, there may + /// be no symbols mapped in memory at all. + bool allow_memory_image_last_resort = false; + + /// The module the binary was found as, or empty if it was not found. It is + /// not registered with the Target until LoadBinaryInTarget. + lldb::ModuleSP module_sp; + + /// The binary as it was read out of the process' memory, if it had to be, + /// so that it is not read a second time. + lldb::ModuleSP memory_module_sp; + + /// What an external symbol server had to say about this binary. The search + /// records it rather than reporting it, so that it reaches the user in the + /// caller's order. + Status error; + }; + + /// Find a binary and load it into a Target. + /// + /// Given a UUID, search for a binary and load it at the address provided, or + /// with the slide applied, or at the file address unslid. + /// + /// Given an address, try to read the binary out of memory, get the UUID, find + /// the file if possible and load it unslid, or add the memory module. + /// + /// May force an expensive search on the computer to find the binary by UUID. + /// To load more than one binary, use LocateBinaries and LoadBinaryInTarget. + /// + /// \param[in] process + /// The process to add this binary to. + /// + /// \param[in,out] bin_spec + /// The binary to find and load, with its input fields filled in by the + /// caller. + /// + /// \return + /// The module that was added to the Target, or an error saying why the + /// binary could not be found and loaded. + static llvm::Expected<lldb::ModuleSP> + LocateAndLoadBinary(Process *process, BinarySpec &bin_spec); + + /// Search for a batch of binaries, without mutating the Target. + /// + /// The entries are independent: a binary that cannot be found leaves its + /// BinarySpec::module_sp empty and has no effect on the others. Nothing is + /// registered with the Target, see LoadBinaryInTarget. + /// + /// \param[in] process + /// The process the binaries belong to. Used to read a binary's header + /// out of memory when its UUID isn't known, and otherwise only read from. + /// + /// \param[in,out] bin_specs + /// The binaries to search for, with their input fields filled in by the + /// caller. + static void LocateBinaries(Process *process, + llvm::MutableArrayRef<BinarySpec> bin_specs); + + /// Add a binary that LocateBinaries searched for to the Target, and set its + /// load address. /// - /// Given a UUID, search for a binary and load it at the address provided, - /// or with the slide applied, or at the file address unslid. + /// This mutates the Target and may read the process' memory, so it has to be + /// called for one binary at a time. Call it in the caller's own order rather + /// than in the order the searches finished: the order decides the Target's + /// module order, which binary gets to set the Target's architecture, and the + /// order in which messages reach the user. /// - /// Given an address, try to read the binary out of memory, get the UUID, - /// find the file if possible and load it unslid, or add the memory module. + /// Whether a failure is worth telling the user about is left to the caller, + /// which knows whether it went looking for a binary that has to be there. A + /// symbol server's word on a binary that was found anyway is reported here. /// /// \param[in] process /// The process to add this binary to. /// - /// \param[in] name - /// Name of the binary, if available. If this method cannot find a - /// matching binary on the debug host, it may create a memory module - /// out of live memory, and the provided name will be used. If an - /// empty StringRef is provided, a name will be constructed for the module - /// based on the address it is loaded at. - /// - /// \param[in] uuid - /// UUID of the binary to be loaded. UUID may be empty, and if a - /// load address is supplied, will read the binary from memory, get - /// a UUID and try to find a local binary. There is a performance - /// cost to doing this, it is not preferable. - /// - /// \param[in] value - /// Address where the binary should be loaded, or read out of memory. - /// Or a slide value, to be applied to the file addresses of the binary. - /// - /// \param[in] value_is_offset - /// A flag indicating that \p value is an address, or an offset to - /// be applied to the file addresses. - /// - /// \param[in] force_symbol_search - /// Allow the search to do a possibly expensive external search for - /// the ObjectFile and/or SymbolFile. - /// - /// \param[in] notify - /// Whether ModulesDidLoad should be called when a binary has been added - /// to the Target. The caller may prefer to batch up these when loading - /// multiple binaries. - /// - /// \param[in] set_address_in_target - /// Whether the address of the binary should be set in the Target if it - /// is added. The caller may want to set the section addresses - /// individually, instead of loading the binary the entire based on the - /// start address or slide. The caller is responsible for setting the - /// load address for the binary or its segments in the Target if it passes - /// true. - /// - /// \param[in] allow_memory_image_last_resort - /// If no better binary image can be found, allow reading the binary - /// out of memory, if possible, and create the Module based on that. - /// May be slow to read a binary out of memory, and for unusual - /// environments, may be no symbols mapped in memory at all. + /// \param[in,out] bin_spec + /// A binary that LocateBinaries has searched for. /// /// \return - /// Returns a shared pointer for the Module that has been added. - static lldb::ModuleSP LoadBinaryWithUUIDAndAddress( - Process *process, llvm::StringRef name, UUID uuid, lldb::addr_t value, - bool value_is_offset, bool force_symbol_search, bool notify, - bool set_address_in_target, bool allow_memory_image_last_resort); + /// The module that was added to the Target, or an error saying why the + /// binary could not be found and loaded. + static llvm::Expected<lldb::ModuleSP> + LoadBinaryInTarget(Process *process, BinarySpec &bin_spec); /// Get information about the shared cache for a process, if possible. /// diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp index ac260607f6c86..96da588a0d644 100644 --- a/lldb/source/Core/DynamicLoader.cpp +++ b/lldb/source/Core/DynamicLoader.cpp @@ -26,8 +26,10 @@ #include "lldb/lldb-private-interfaces.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" #include <memory> +#include <string> #include <cassert> @@ -211,172 +213,205 @@ static ModuleSP ReadUnnamedMemoryModule(Process *process, addr_t addr, return *module_sp_or_err; } -ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress( - Process *process, llvm::StringRef name, UUID uuid, addr_t value, - bool value_is_offset, bool force_symbol_search, bool notify, - bool set_address_in_target, bool allow_memory_image_last_resort) { - ModuleSP memory_module_sp; - ModuleSP module_sp; - PlatformSP platform_sp = process->GetTarget().GetPlatform(); - Target &target = process->GetTarget(); - Status error; - - StreamString prog_str; - if (!name.empty()) { - prog_str << name.str() << " "; +static std::string +GetBinaryDescription(const DynamicLoader::BinarySpec &bin_spec) { + StreamString desc; + if (!bin_spec.name.empty()) + desc << bin_spec.name << " "; + if (bin_spec.uuid.IsValid()) + desc << bin_spec.uuid.GetAsString(); + if (!bin_spec.value_is_offset && bin_spec.value != LLDB_INVALID_ADDRESS) { + desc << " at 0x"; + desc.PutHex64(bin_spec.value); } - if (uuid.IsValid()) - prog_str << uuid.GetAsString(); - if (value_is_offset == 0 && value != LLDB_INVALID_ADDRESS) { - prog_str << " at 0x"; - prog_str.PutHex64(value); - } - - if (!uuid.IsValid() && !value_is_offset) { - memory_module_sp = ReadUnnamedMemoryModule(process, value, name); + return desc.GetString().str(); +} - if (memory_module_sp) { - uuid = memory_module_sp->GetUUID(); - if (uuid.IsValid()) { - prog_str << " "; - prog_str << uuid.GetAsString(); - } - } +static std::string +GetBinaryNotFoundMessage(const DynamicLoader::BinarySpec &bin_spec) { + StreamString msg; + msg << "Unable to find file"; + if (!bin_spec.name.empty()) + msg << " " << bin_spec.name; + if (bin_spec.uuid.IsValid()) + msg << " with UUID " << bin_spec.uuid.GetAsString(); + if (bin_spec.value != LLDB_INVALID_ADDRESS) { + if (bin_spec.value_is_offset) + msg.Printf(" with slide 0x%" PRIx64, bin_spec.value); + else + msg.Printf(" at address 0x%" PRIx64, bin_spec.value); } + return msg.GetString().str(); +} + +/// Search for a binary with a known UUID, and create a module for it. +/// +/// Does not mutate the Target, but does read from it, and reaches the global +/// shared module list, the symbol locator plugins, and a locate module callback +/// the user may have installed. +static void SearchForBinary(Target &target, DynamicLoader::BinarySpec &bin_spec, + const FileSpecList &search_paths) { ModuleSpec module_spec; module_spec.SetTarget(target.shared_from_this()); - module_spec.GetUUID() = uuid; - FileSpec name_filespec(name); + module_spec.GetUUID() = bin_spec.uuid; + FileSpec name_filespec(bin_spec.name); if (FileSystem::Instance().Exists(name_filespec)) module_spec.GetFileSpec() = name_filespec; - if (uuid.IsValid()) { - Progress progress("Locating binary", prog_str.GetString().str()); - - // Has lldb already seen a module with this UUID? - // Or have external lookup enabled in DebugSymbols on macOS. - if (!module_sp) - error = - ModuleList::GetSharedModule(module_spec, module_sp, nullptr, nullptr); - - // Can lldb's symbol/executable location schemes - // find an executable and symbol file. - if (!module_sp) { - FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths(); - StatisticsMap symbol_locator_map; - module_spec.GetSymbolFileSpec() = - PluginManager::LocateExecutableSymbolFile(module_spec, search_paths, - symbol_locator_map); - ModuleSpec objfile_module_spec = - PluginManager::LocateExecutableObjectFile(module_spec, - symbol_locator_map); - module_spec.GetFileSpec() = objfile_module_spec.GetFileSpec(); - if (FileSystem::Instance().Exists(module_spec.GetFileSpec()) && - FileSystem::Instance().Exists(module_spec.GetSymbolFileSpec())) { - module_sp = std::make_shared<Module>(module_spec); - } + // Has lldb already seen a module with this UUID? + // Or have external lookup enabled in DebugSymbols on macOS. + Status error = ModuleList::GetSharedModule(module_spec, bin_spec.module_sp, + nullptr, nullptr); + + // Can lldb's symbol/executable location schemes find an executable and + // symbol file. + if (!bin_spec.module_sp) { + StatisticsMap symbol_locator_map; + module_spec.GetSymbolFileSpec() = PluginManager::LocateExecutableSymbolFile( + module_spec, search_paths, symbol_locator_map); + ModuleSpec objfile_module_spec = PluginManager::LocateExecutableObjectFile( + module_spec, symbol_locator_map); + module_spec.GetFileSpec() = objfile_module_spec.GetFileSpec(); + if (FileSystem::Instance().Exists(module_spec.GetFileSpec()) && + FileSystem::Instance().Exists(module_spec.GetSymbolFileSpec())) { + bin_spec.module_sp = std::make_shared<Module>(module_spec); + } - if (module_sp) { - module_sp->GetSymbolLocatorStatistics().merge(symbol_locator_map); - } + if (bin_spec.module_sp) { + bin_spec.module_sp->GetSymbolLocatorStatistics().merge( + symbol_locator_map); } + } - // 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()) { - 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("") && - error.AsCString("")[0] != '\0') { - *target.GetDebugger().GetAsyncErrorStream() << error.AsCString(); - } + // 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 (!bin_spec.module_sp || !bin_spec.module_sp->GetSymbolFileFileSpec()) { + PluginManager::DownloadObjectAndSymbolFile(module_spec, error, + bin_spec.force_symbol_search); + if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) { + bin_spec.module_sp = std::make_shared<Module>(module_spec); + } else if (bin_spec.force_symbol_search && error.Fail()) { + bin_spec.error = std::move(error); } + } + + // If we only found the executable, create a Module based on that. + if (!bin_spec.module_sp && + FileSystem::Instance().Exists(module_spec.GetFileSpec())) + bin_spec.module_sp = std::make_shared<Module>(module_spec); +} - // If we only found the executable, create a Module based on that. - if (!module_sp && FileSystem::Instance().Exists(module_spec.GetFileSpec())) - module_sp = std::make_shared<Module>(module_spec); +static void FindBinaryUUIDInMemory(Process *process, + DynamicLoader::BinarySpec &bin_spec) { + bin_spec.memory_module_sp = + ReadUnnamedMemoryModule(process, bin_spec.value, bin_spec.name); + if (bin_spec.memory_module_sp) + bin_spec.uuid = bin_spec.memory_module_sp->GetUUID(); +} + +void DynamicLoader::LocateBinaries( + Process *process, llvm::MutableArrayRef<BinarySpec> bin_specs) { + Target &target = process->GetTarget(); + const FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths(); + + for (BinarySpec &bin_spec : bin_specs) { + if (!bin_spec.uuid.IsValid() && !bin_spec.value_is_offset) + FindBinaryUUIDInMemory(process, bin_spec); + if (!bin_spec.uuid.IsValid()) + continue; + Progress progress("Locating binary", GetBinaryDescription(bin_spec)); + SearchForBinary(target, bin_spec, search_paths); } +} + +llvm::Expected<ModuleSP> +DynamicLoader::LoadBinaryInTarget(Process *process, BinarySpec &bin_spec) { + Target &target = process->GetTarget(); + + // The error belongs to this function now: every path below either reports it + // or folds it into the failure. + llvm::Error search_error = bin_spec.error.takeError(); // If we couldn't find the binary anywhere else, as a last resort, // read it out of memory. - if (allow_memory_image_last_resort && !module_sp.get() && - value != LLDB_INVALID_ADDRESS && !value_is_offset) { - if (!memory_module_sp) - memory_module_sp = ReadUnnamedMemoryModule(process, value, name); - if (memory_module_sp) - module_sp = memory_module_sp; + if (bin_spec.allow_memory_image_last_resort && !bin_spec.module_sp && + bin_spec.value != LLDB_INVALID_ADDRESS && !bin_spec.value_is_offset) { + if (!bin_spec.memory_module_sp) + bin_spec.memory_module_sp = + ReadUnnamedMemoryModule(process, bin_spec.value, bin_spec.name); + if (bin_spec.memory_module_sp) + bin_spec.module_sp = bin_spec.memory_module_sp; } Log *log = GetLog(LLDBLog::DynamicLoader); - if (module_sp.get()) { - // Ensure the Target has an architecture set in case - // we need it while processing this binary/eh_frame/debug info. - if (!target.GetArchitecture().IsValid()) - target.SetArchitecture(module_sp->GetArchitecture()); - target.GetImages().AppendIfNeeded(module_sp, false); - - bool changed = false; - if (set_address_in_target) { - if (module_sp->GetObjectFile()) { - if (value != LLDB_INVALID_ADDRESS) { - LLDB_LOGF(log, - "DynamicLoader::LoadBinaryWithUUIDAndAddress Loading " - "binary %s UUID %s at %s 0x%" PRIx64, - name.str().c_str(), uuid.GetAsString().c_str(), - value_is_offset ? "offset" : "address", value); - module_sp->SetLoadAddress(target, value, value_is_offset, changed); - } else { - // No address/offset/slide, load the binary at file address, - // offset 0. - LLDB_LOGF(log, - "DynamicLoader::LoadBinaryWithUUIDAndAddress Loading " - "binary %s UUID %s at file addre... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/214372 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
