[llvm-branch-commits] [lldb] [lldb] Search for a corefile's images before loading any of them (PR #215393)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Aug 10 15:08:13 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
<details>
<summary>Changes</summary>
A userland or kernel corefile can list hundreds of images, and searching for
one can shell out to a symbol server or fetch over the network. Searching for
them one at a time is where loading such a corefile spends its time.
Add a batch form of SymbolLocator::Locate that runs the searches on the
debugger's thread pool, gated on target.parallel-module-load. Results come
back in the order the requests were given, since that order decides the
Target's module order. Only the results are ordered, and anything a search
reports to the user arrives in whatever order the searches finish in.
Only the plugin searches run concurrently, so a platform hook does not have to
be thread safe to take part, and reading a binary's UUID out of memory stays
on the calling thread.
Setting up a platform binary can replace the Target's platform and dynamic
loader, and now happens for every image before any of them is searched for, so
the platform a corefile asks for is the one all of its images are searched
with. Previously the images listed ahead of a platform binary were searched
with whatever platform preceded it.
Assisted-by: Claude
---
<sub>Stack created with <a href="https://github.com/github/gh-stack">GitHub Stacks CLI</a> • <a href="https://gh.io/stacks-feedback">Give Feedback 💬</a></sub>
---
Patch is 30.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/215393.diff
6 Files Affected:
- (modified) lldb/include/lldb/Symbol/SymbolLocator.h (+30)
- (modified) lldb/source/Core/DynamicLoader.cpp (+43-19)
- (modified) lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (+84-64)
- (modified) lldb/source/Symbol/SymbolLocator.cpp (+82-11)
- (modified) lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py (+25)
- (modified) lldb/unittests/Symbol/SymbolLocatorTest.cpp (+142-4)
``````````diff
diff --git a/lldb/include/lldb/Symbol/SymbolLocator.h b/lldb/include/lldb/Symbol/SymbolLocator.h
index d86538fd9a2ce..244b43946381e 100644
--- a/lldb/include/lldb/Symbol/SymbolLocator.h
+++ b/lldb/include/lldb/Symbol/SymbolLocator.h
@@ -15,9 +15,12 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/UUID.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/Error.h"
+#include <string>
#include <system_error>
+#include <vector>
namespace lldb_private {
@@ -48,6 +51,9 @@ class SymbolLocator : public PluginInterface {
/// Allow contacting an external symbol server when the local searches come
/// up empty.
bool external_lookup = false;
+
+ /// How to name this binary in a progress report.
+ std::string description;
};
/// What a search found.
@@ -80,12 +86,36 @@ class SymbolLocator : public PluginInterface {
static llvm::Expected<Result> Locate(const Request &request,
const FileSpecList &search_paths);
+ /// Search for a batch of binaries.
+ ///
+ /// Each request's platform hook runs on the calling thread, in order, so that
+ /// a platform does not have to be thread safe to take part. The plugin
+ /// searches may run concurrently, which is where the time goes: a single one
+ /// of them can shell out to a symbol server or fetch over the network.
+ ///
+ /// Blocks until every search has finished.
+ ///
+ /// \return
+ /// One result per request, in the order the requests were given, each as
+ /// described for the single request above. Only the results are ordered.
+ /// Anything a search reports to the user arrives in whatever order the
+ /// searches finish in.
+ static std::vector<llvm::Expected<Result>>
+ Locate(llvm::ArrayRef<Request> requests, const FileSpecList &search_paths,
+ bool parallel);
+
/// 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);
+
+private:
+ /// The symbol locator plugins, without the platform hook. This is the half
+ /// that is safe to run for several binaries at once.
+ static llvm::Expected<Result>
+ LocateWithPlugins(const Request &request, const FileSpecList &search_paths);
};
} // namespace lldb_private
diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp
index 09f66e38f2097..2a5852054e5f6 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/lldb/source/Core/DynamicLoader.cpp
@@ -13,7 +13,6 @@
#include "lldb/Core/ModuleList.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/PluginManager.h"
-#include "lldb/Core/Progress.h"
#include "lldb/Core/Section.h"
#include "lldb/Symbol/ObjectFile.h"
#include "lldb/Symbol/SymbolLocator.h"
@@ -26,10 +25,13 @@
#include "lldb/Utility/Log.h"
#include "lldb/lldb-private-interfaces.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include <memory>
+#include <optional>
#include <string>
#include <cassert>
@@ -245,13 +247,16 @@ GetBinaryNotFoundMessage(const DynamicLoader::BinarySpec &bin_spec) {
return msg.GetString().str();
}
-/// Search for a binary with a known UUID, and create a module for it.
+/// Work out what to search for, and answer the question the Target can answer
+/// on its own.
///
/// 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) {
+/// shared module list and a locate module callback the user may have installed,
+/// so it has to be called for one binary at a time.
+///
+/// \return What to search for, or nothing when the binary is already in hand.
+static std::optional<SymbolLocator::Request>
+PrepareSearch(Target &target, DynamicLoader::BinarySpec &bin_spec) {
ModuleSpec module_spec;
module_spec.SetTarget(target.shared_from_this());
module_spec.GetUUID() = bin_spec.uuid;
@@ -266,19 +271,24 @@ static void SearchForBinary(Target &target, DynamicLoader::BinarySpec &bin_spec,
/*invoke_locate_callback=*/true,
/*invoke_symbol_locators=*/false);
if (bin_spec.module_sp && bin_spec.module_sp->GetSymbolFileFileSpec())
- return;
+ return std::nullopt;
- // Search for the binary and its symbols.
SymbolLocator::Request request;
request.module_spec = module_spec;
request.platform = target.GetPlatform();
request.external_lookup = bin_spec.force_symbol_search;
+ request.description = GetBinaryDescription(bin_spec);
+ return request;
+}
- llvm::Expected<SymbolLocator::Result> located =
- SymbolLocator::Locate(request, search_paths);
+/// Create a module for what the search found.
+///
+/// The module is not registered with the Target until LoadBinaryInTarget.
+static void FinishSearch(DynamicLoader::BinarySpec &bin_spec,
+ llvm::Expected<SymbolLocator::Result> located) {
if (!located) {
- // This function's caller names the binary it could not find, so a plain
- // miss needs nothing added to it. An explanation from a symbol server does.
+ // LoadBinaryInTarget names the binary it could not find, so a plain miss
+ // needs nothing added to it. An explanation from a symbol server does.
llvm::Error error = located.takeError();
if (error.isA<SymbolLocator::NotFound>())
llvm::consumeError(std::move(error));
@@ -287,13 +297,12 @@ static void SearchForBinary(Target &target, DynamicLoader::BinarySpec &bin_spec,
return;
}
- // A binary was found. Its symbols are another matter, and the caller reports
- // that in its own order.
+ // A binary was found. Its symbols are another matter, and LoadBinaryInTarget
+ // reports that in its own order.
bin_spec.error = std::move(located->symbol_error);
- // Create a module for what was found, sharing it with any other Target that
- // asks for the same binary. The module is not registered with this Target
- // until LoadBinaryInTarget. The locators have run, so don't run them again.
+ // Share the module with any other Target that asks for the same binary. The
+ // locators have run, so don't run them again.
ModuleSP located_module_sp;
ModuleList::GetSharedModule(located->module_spec, located_module_sp, nullptr,
nullptr, /*invoke_locate_callback=*/false,
@@ -323,14 +332,29 @@ void DynamicLoader::LocateBinaries(
Target &target = process->GetTarget();
const FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
+ // Reading a binary's UUID out of memory has to happen on this thread, and
+ // before the searches, so that every binary that has a UUID to find is part
+ // of the batch.
+ llvm::SmallVector<BinarySpec *> to_search;
+ std::vector<SymbolLocator::Request> requests;
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);
+ if (std::optional<SymbolLocator::Request> request =
+ PrepareSearch(target, bin_spec)) {
+ to_search.push_back(&bin_spec);
+ requests.push_back(std::move(*request));
+ }
}
+
+ std::vector<llvm::Expected<SymbolLocator::Result>> located =
+ SymbolLocator::Locate(requests, search_paths,
+ target.GetParallelModuleLoad());
+
+ for (auto [bin_spec, result] : llvm::zip_equal(to_search, located))
+ FinishSearch(*bin_spec, std::move(result));
}
llvm::Expected<ModuleSP>
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 5a31f16c9a729..e852777039ac5 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6674,17 +6674,26 @@ ObjectFileMachO::GetCorefileAllImageInfos() {
bool ObjectFileMachO::LoadCoreFileImages(lldb_private::Process &process) {
MachOCorefileAllImageInfos image_infos = GetCorefileAllImageInfos();
Log *log = GetLog(LLDBLog::Object | LLDBLog::DynamicLoader);
- Status error;
bool found_platform_binary = false;
ModuleList added_modules;
- for (MachOCorefileImageEntry &image : image_infos.all_image_infos) {
- ModuleSP module_sp, local_filesystem_module_sp;
+ // Searching for a binary is slow, so gather them up and search for a whole
+ // batch at once. The images are installed in the order the corefile lists
+ // them, whatever order the searches finish in.
+ llvm::SmallVector<const MachOCorefileImageEntry *> pending_images;
+ std::vector<DynamicLoader::BinarySpec> pending_specs;
+
+ for (MachOCorefileImageEntry &image : image_infos.all_image_infos) {
// If this is a platform binary, it has been loaded (or registered with
// the DynamicLoader to be loaded), we don't need to do any further
// processing. We're not going to call ModulesDidLoad on this in this
// method, so notify==true.
+ //
+ // Setting one up can replace the Target's platform and dynamic loader, and
+ // no image is searched for until this loop has run to the end, so the
+ // platform a corefile asks for is the one all of its images are searched
+ // with.
if (process.GetTarget()
.GetDebugger()
.GetPlatformList()
@@ -6708,74 +6717,85 @@ bool ObjectFileMachO::LoadCoreFileImages(lldb_private::Process &process) {
// We have either a UUID, or we have a load address which
// and can try to read load commands and find a UUID.
- if (image.uuid.IsValid() ||
- (!value_is_offset && value != LLDB_INVALID_ADDRESS)) {
- DynamicLoader::BinarySpec bin_spec;
- bin_spec.name = image.filename;
- bin_spec.uuid = image.uuid;
- bin_spec.value = value;
- bin_spec.value_is_offset = value_is_offset;
- bin_spec.force_symbol_search = image.currently_executing;
- bin_spec.notify = false;
- // Userland Darwin binaries will have segment load addresses via
- // the `all image infos` LC_NOTE.
- bin_spec.set_address_in_target = image.segment_load_addresses.empty();
- bin_spec.allow_memory_image_last_resort =
- !image.segment_load_addresses.empty();
- if (llvm::Expected<ModuleSP> located =
- DynamicLoader::LocateAndLoadBinary(&process, bin_spec)) {
- module_sp = *located;
- } else if (bin_spec.force_symbol_search) {
- *process.GetTarget().GetDebugger().GetAsyncErrorStream()
- << llvm::toString(located.takeError()) << "\n";
- } else {
- // A corefile image that isn't on this machine is routine, and
- // LocateAndLoadBinary has already logged it.
- llvm::consumeError(located.takeError());
- }
+ if (!image.uuid.IsValid() &&
+ (value_is_offset || value == LLDB_INVALID_ADDRESS))
+ continue;
+
+ DynamicLoader::BinarySpec bin_spec;
+ bin_spec.name = image.filename;
+ bin_spec.uuid = image.uuid;
+ bin_spec.value = value;
+ bin_spec.value_is_offset = value_is_offset;
+ bin_spec.force_symbol_search = image.currently_executing;
+ bin_spec.notify = false;
+ // Userland Darwin binaries will have segment load addresses via
+ // the `all image infos` LC_NOTE.
+ bin_spec.set_address_in_target = image.segment_load_addresses.empty();
+ bin_spec.allow_memory_image_last_resort =
+ !image.segment_load_addresses.empty();
+
+ pending_images.push_back(&image);
+ pending_specs.push_back(std::move(bin_spec));
+ }
+
+ DynamicLoader::LocateBinaries(&process, pending_specs);
+
+ for (auto [image, bin_spec] :
+ llvm::zip_equal(pending_images, pending_specs)) {
+ ModuleSP module_sp;
+ if (llvm::Expected<ModuleSP> loaded =
+ DynamicLoader::LoadBinaryInTarget(&process, bin_spec)) {
+ module_sp = *loaded;
+ } else if (bin_spec.force_symbol_search) {
+ *process.GetTarget().GetDebugger().GetAsyncErrorStream()
+ << llvm::toString(loaded.takeError()) << "\n";
+ } else {
+ // A corefile image that isn't on this machine is routine, and
+ // LoadBinaryInTarget has already logged it.
+ llvm::consumeError(loaded.takeError());
}
- // We have a ModuleSP to load in the Target. Load it at the
- // correct address/slide and notify/load scripting resources.
- if (module_sp) {
- added_modules.Append(module_sp, false /* notify */);
-
- // We have a list of segment load address
- if (image.segment_load_addresses.size() > 0) {
- if (log) {
- std::string uuidstr = image.uuid.GetAsString();
- log->Printf("ObjectFileMachO::LoadCoreFileImages adding binary '%s' "
- "UUID %s with section load addresses",
- module_sp->GetFileSpec().GetPath().c_str(),
- uuidstr.c_str());
- }
- ObjectFile *objfile = module_sp->GetObjectFile();
- SectionList *sectlist = objfile ? objfile->GetSectionList() : nullptr;
- for (auto name_vmaddr_tuple : image.segment_load_addresses) {
- if (sectlist) {
- SectionSP sect_sp =
- sectlist->FindSectionByName(std::get<0>(name_vmaddr_tuple));
- if (sect_sp) {
- process.GetTarget().SetSectionLoadAddress(
- sect_sp, std::get<1>(name_vmaddr_tuple));
- }
+ if (!module_sp)
+ continue;
+
+ added_modules.Append(module_sp, false /* notify */);
+
+ // We have a list of segment load address
+ if (image->segment_load_addresses.size() > 0) {
+ if (log) {
+ std::string uuidstr = image->uuid.GetAsString();
+ log->Printf("ObjectFileMachO::LoadCoreFileImages adding binary '%s' "
+ "UUID %s with section load addresses",
+ module_sp->GetFileSpec().GetPath().c_str(),
+ uuidstr.c_str());
+ }
+ ObjectFile *objfile = module_sp->GetObjectFile();
+ SectionList *sectlist = objfile ? objfile->GetSectionList() : nullptr;
+ for (auto name_vmaddr_tuple : image->segment_load_addresses) {
+ if (sectlist) {
+ SectionSP sect_sp =
+ sectlist->FindSectionByName(std::get<0>(name_vmaddr_tuple));
+ if (sect_sp) {
+ process.GetTarget().SetSectionLoadAddress(
+ sect_sp, std::get<1>(name_vmaddr_tuple));
}
}
- } else {
- if (log) {
- std::string uuidstr = image.uuid.GetAsString();
- log->Printf("ObjectFileMachO::LoadCoreFileImages adding binary '%s' "
- "UUID %s with %s 0x%" PRIx64,
- module_sp->GetFileSpec().GetPath().c_str(),
- uuidstr.c_str(),
- value_is_offset ? "slide" : "load address", value);
- }
- bool changed;
- module_sp->SetLoadAddress(process.GetTarget(), value, value_is_offset,
- changed);
}
+ } else {
+ if (log) {
+ std::string uuidstr = image->uuid.GetAsString();
+ log->Printf("ObjectFileMachO::LoadCoreFileImages adding binary '%s' "
+ "UUID %s with %s 0x%" PRIx64,
+ module_sp->GetFileSpec().GetPath().c_str(), uuidstr.c_str(),
+ bin_spec.value_is_offset ? "slide" : "load address",
+ bin_spec.value);
+ }
+ bool changed;
+ module_sp->SetLoadAddress(process.GetTarget(), bin_spec.value,
+ bin_spec.value_is_offset, changed);
}
}
+
if (added_modules.GetSize() > 0) {
process.GetTarget().ModulesDidLoad(added_modules);
process.Flush();
diff --git a/lldb/source/Symbol/SymbolLocator.cpp b/lldb/source/Symbol/SymbolLocator.cpp
index 1f32d69a0b618..8eb120e2bf5a6 100644
--- a/lldb/source/Symbol/SymbolLocator.cpp
+++ b/lldb/source/Symbol/SymbolLocator.cpp
@@ -10,10 +10,12 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
#include "lldb/Target/Platform.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/Support/ThreadPool.h"
@@ -31,22 +33,13 @@ std::error_code SymbolLocator::NotFound::convertToErrorCode() const {
}
llvm::Expected<SymbolLocator::Result>
-SymbolLocator::Locate(const Request &request,
- const FileSpecList &search_paths) {
+SymbolLocator::LocateWithPlugins(const Request &request,
+ const FileSpecList &search_paths) {
FileSystem &fs = FileSystem::Instance();
Result result;
ModuleSpec &module_spec = result.module_spec;
module_spec = request.module_spec;
- // The locator plugins have no Platform to consult, so ask it here.
- if (request.platform) {
- if (std::optional<ModuleSpec> found = request.platform->FindModuleFiles(
- module_spec, search_paths, result.statistics)) {
- result.module_spec = *found;
- return result;
- }
- }
-
// Can lldb's symbol and executable location schemes find them locally?
module_spec.GetSymbolFileSpec() = PluginManager::LocateExecutableSymbolFile(
module_spec, search_paths, result.statistics);
@@ -77,6 +70,84 @@ SymbolLocator::Locate(const Request &request,
return result;
}
+/// Ask the request's platform where the binary is, if it has one. Answering
+/// with std::nullopt leaves the search to the plugins.
+static std::optional<SymbolLocator::Result>
+AskPlatform(const SymbolLocator::Request &request,
+ const FileSpecList &search_paths) {
+ if (!request.platform)
+ return std::nullopt;
+
+ SymbolLocator::Result result;
+ std::optional<ModuleSpec> found = request.platform->FindModuleFiles(
+ request.module_spec, search_paths, result.statistics);
+ if (!found)
+ return std::nullopt;
+
+ result.module_spec = *found;
+ return result;
+}
+
+llvm::Expected<SymbolLocator::Result>
+SymbolLocator::Locate(const Request &request,
+ const FileSpecList &search_paths) {
+ if (std::optional<Result> answer = AskPlatform(request, search_paths))
+ return std::move(*answer);
+ return LocateWithPlugins(request, search_paths);
+}
+
+std::vector<llvm::Expected<SymbolLocator::Result>>
+SymbolLocator::Locate(llvm::ArrayRef<Request> requests,
+ const FileSpecList &search_paths, bool parallel) {
+ // One slot per request, so that the searches can fill them in without
+ // contending, and so that the results come back in the caller's order.
+ std::vector<std::optional<llvm::Expected<Result>>> slots(requests.size());
+ std::vector<size_t> remaining;
+
+ if (!requests.empty()) {
+ // A search is slow enough that reporting every one of them individually is
+ // not worth the contention between the threads running them.
+ Progress progress("Locating binaries", "", requests.size(),
+ /*debugger=*/nullptr,
+ Progress::kDefaultHighFrequencyReportTime);
+
+ // The platform hooks run here, on this thread and in order.
+ for (auto [i, request] : llvm::enumerate(requests)) {
+ if (std::optional<Result> answer = AskPlatform(request, search_paths)) {
+ slots[i] = std::move(*answer);
+ progress.Increment(1, request.description);
+ } else {
+ remaining.push_back(i);
+ }
+ }
+
+ auto locate = [&](size_t i) {
+ slots[i] = LocateWithPlugins(requests[i], search_paths);
+ progress.Increment(1, requests[i].description);
+ };
+
+ // One search has nothing to overlap with, and handing it to the pool would
+ // only move it off this thread.
+ if (parallel && rem...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/215393
More information about the llvm-branch-commits
mailing list