[llvm-branch-commits] [lldb] [lldb] Search for a corefile's images before loading any of them (PR #215393)
Jonas Devlieghere via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Aug 13 13:56:33 PDT 2026
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/215393
>From efe6f488361723b62b77123f329833b07276efd4 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 10 Aug 2026 12:52:58 -0700
Subject: [PATCH] [lldb] Search for a corefile's images before loading any of
them
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
---
lldb/include/lldb/Symbol/SymbolLocator.h | 19 +++
lldb/source/Core/DynamicLoader.cpp | 52 ++++---
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 143 +++++++++--------
lldb/source/Symbol/SymbolLocator.cpp | 87 +++++++++--
.../TestMultipleBinaryCorefile.py | 25 +++
lldb/unittests/Symbol/SymbolLocatorTest.cpp | 145 +++++++++++++++++-
6 files changed, 372 insertions(+), 99 deletions(-)
diff --git a/lldb/include/lldb/Symbol/SymbolLocator.h b/lldb/include/lldb/Symbol/SymbolLocator.h
index 5fc9c161af50a..93915d2321c8a 100644
--- a/lldb/include/lldb/Symbol/SymbolLocator.h
+++ b/lldb/include/lldb/Symbol/SymbolLocator.h
@@ -15,10 +15,13 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/UUID.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/Error.h"
#include <optional>
+#include <string>
#include <system_error>
+#include <vector>
namespace lldb_private {
@@ -49,6 +52,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.
@@ -82,12 +88,25 @@ class SymbolLocator : public PluginInterface {
static llvm::Expected<Result> Locate(const Request &request,
const FileSpecList &search_paths);
+ /// The platform hooks run on the calling thread, in order. Only the plugin
+ /// searches may run concurrently.
+ ///
+ /// \return One result per request, in the order the requests were given.
+ 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:
+ /// Must stay callable concurrently.
+ 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 a948fefa4c83e..4178177a995a7 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,11 @@ GetBinaryNotFoundMessage(const DynamicLoader::BinarySpec &bin_spec) {
return msg.GetString().str();
}
-/// Search for a binary with a known UUID, and create a module for it.
+/// Reads the Target, so it has to be called for one binary at a time.
///
-/// 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) {
+/// \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 +266,22 @@ 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);
+/// 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.
+ // Only an explanation from a symbol server adds anything to the miss the
+ // caller already reports.
llvm::Error error = located.takeError();
if (error.isA<SymbolLocator::NotFound>())
llvm::consumeError(std::move(error));
@@ -287,14 +290,9 @@ 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.
if (located->symbol_error)
bin_spec.error = Status::FromError(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.
ModuleSP located_module_sp;
ModuleList::GetSharedModule(located->module_spec, located_module_sp, nullptr,
nullptr, /*invoke_locate_callback=*/false,
@@ -324,14 +322,28 @@ void DynamicLoader::LocateBinaries(
Target &target = process->GetTarget();
const FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
+ // Has to happen on this thread, and before any search, so that a binary whose
+ // UUID is not known yet still joins 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..01f56f0c3ee17 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6674,17 +6674,21 @@ 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;
+ 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, so
+ // no image is searched for until this loop has run to the end.
if (process.GetTarget()
.GetDebugger()
.GetPlatformList()
@@ -6708,74 +6712,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 has
+ // already been logged.
+ 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 4b8bc7405fdbb..58304aad24100 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,78 @@ SymbolLocator::Locate(const Request &request,
return result;
}
+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 concurrent searches never contend.
+ std::vector<std::optional<llvm::Expected<Result>>> slots(requests.size());
+ std::vector<size_t> remaining;
+
+ if (!requests.empty()) {
+ // Throttled because every search reports through this from its own thread.
+ Progress progress("Locating binaries", "", requests.size(),
+ /*debugger=*/nullptr,
+ Progress::kDefaultHighFrequencyReportTime);
+
+ 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.
+ if (parallel && remaining.size() > 1) {
+ llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
+ for (size_t i : remaining)
+ task_group.async(locate, i);
+ task_group.wait();
+ } else {
+ for (size_t i : remaining)
+ locate(i);
+ }
+ }
+
+ std::vector<llvm::Expected<Result>> results;
+ results.reserve(slots.size());
+ for (std::optional<llvm::Expected<Result>> &slot : slots) {
+ assert(slot && "every request has a result");
+ results.emplace_back(std::move(*slot));
+ }
+ return results;
+}
+
void SymbolLocator::DownloadSymbolFileAsync(const UUID &uuid) {
static llvm::SmallSet<UUID, 8> g_seen_uuids;
static std::mutex g_mutex;
diff --git a/lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py b/lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
index 8a35536b27301..96553a866bb43 100644
--- a/lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
+++ b/lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
@@ -196,6 +196,31 @@ def test_corefile_binaries_dsymforuuid(self):
self.load_corefile_and_test()
+ @skipIf(archs=no_match(["x86_64", "arm64", "arm64e", "aarch64"]))
+ @skipIfRemote
+ @requireDarwin
+ def test_corefile_binaries_serial_search(self):
+ """The corefile's binaries are searched for in parallel by default.
+ Searching for them one at a time has to give the same answer, in the
+ same order, since load_corefile_and_test indexes into the module
+ list."""
+ self.initial_setup()
+
+ self.runCmd("settings set target.parallel-module-load false")
+ self.addTearDownHook(
+ lambda: self.runCmd("settings clear target.parallel-module-load")
+ )
+
+ # Register the binaries in lldb's global module cache, as
+ # test_corefile_binaries_preloaded does, so the corefile's images can
+ # be found without a symbol server.
+ target = self.dbg.CreateTarget(self.aout_exe, "", "", False, lldb.SBError())
+ self.dbg.DeleteTarget(target)
+ target = self.dbg.CreateTarget(self.libtwo_exe, "", "", False, lldb.SBError())
+ self.dbg.DeleteTarget(target)
+
+ self.load_corefile_and_test()
+
@skipIf(archs=no_match(["x86_64", "arm64", "arm64e", "aarch64"]))
@skipIfRemote
@requireDarwin
diff --git a/lldb/unittests/Symbol/SymbolLocatorTest.cpp b/lldb/unittests/Symbol/SymbolLocatorTest.cpp
index d86808702da5e..cc7e8854cf1b7 100644
--- a/lldb/unittests/Symbol/SymbolLocatorTest.cpp
+++ b/lldb/unittests/Symbol/SymbolLocatorTest.cpp
@@ -7,27 +7,42 @@
//===----------------------------------------------------------------------===//
#include "lldb/Symbol/SymbolLocator.h"
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Core/Debugger.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Target/Platform.h"
#include "lldb/Utility/FileSpecList.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/ThreadPool.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/Testing/Support/Error.h"
#include "gtest/gtest.h"
+#include <atomic>
+#include <condition_variable>
+#include <mutex>
+
using namespace lldb;
using namespace lldb_private;
namespace {
/// Which steps of the search ran, so a test can tell where an answer came from.
+/// Written from every thread of a batch, so the flags have to be atomic.
struct LocatorCalls {
- bool located_symbol_file = false;
- bool located_object_file = false;
- bool downloaded = false;
+ std::atomic<bool> located_symbol_file = false;
+ std::atomic<bool> located_object_file = false;
+ std::atomic<bool> downloaded = false;
+
+ void Clear() {
+ located_symbol_file = false;
+ located_object_file = false;
+ downloaded = false;
+ }
};
LocatorCalls g_calls;
@@ -42,6 +57,13 @@ std::optional<FileSpec> g_symbol_file;
/// an errno rather than a message.
bool g_symbol_server_errno = false;
+/// When set, the fake locator only answers for requests carrying a UUID.
+bool g_only_with_uuid = false;
+
+/// Run by the fake locator, to let a test hold every search of a batch open at
+/// once.
+std::function<void()> g_barrier;
+
std::optional<FileSpec> LocateExecutableSymbolFile(const ModuleSpec &,
const FileSpecList &) {
g_calls.located_symbol_file = true;
@@ -50,8 +72,12 @@ std::optional<FileSpec> LocateExecutableSymbolFile(const ModuleSpec &,
std::optional<ModuleSpec> LocateExecutableObjectFile(const ModuleSpec &spec) {
g_calls.located_object_file = true;
+ if (g_barrier)
+ g_barrier();
if (!g_object_file)
return {};
+ if (g_only_with_uuid && !spec.GetUUID().IsValid())
+ return {};
ModuleSpec located(spec);
located.GetFileSpec() = *g_object_file;
return located;
@@ -113,6 +139,11 @@ class SymbolLocatorTest : public testing::Test {
m_fs(new llvm::vfs::InMemoryFileSystem()) {}
void SetUp() override {
+ // The batch runs on the debugger's thread pool. Debugger::Initialize takes
+ // an argument, so SubsystemRAII cannot call it.
+ std::call_once(TestUtilities::g_debugger_initialize_flag,
+ []() { Debugger::Initialize(nullptr); });
+
// Locate reports a binary it cannot find as an error, so a test that wants
// a hit has to point the fake locator at a file that exists.
FileSystem::Initialize(m_fs);
@@ -120,10 +151,12 @@ class SymbolLocatorTest : public testing::Test {
m_fs->addFileNoOwn(m_binary.GetPath(), 0, m_empty_buffer);
m_fs->addFileNoOwn(m_symbols.GetPath(), 0, m_empty_buffer);
- g_calls = LocatorCalls();
+ g_calls.Clear();
g_object_file = std::nullopt;
g_symbol_file = std::nullopt;
g_symbol_server_errno = false;
+ g_only_with_uuid = false;
+ g_barrier = nullptr;
ASSERT_TRUE(PluginManager::RegisterPlugin(
"test", "test symbol locator", CreateSymbolLocator,
LocateExecutableObjectFile, LocateExecutableSymbolFile,
@@ -131,6 +164,7 @@ class SymbolLocatorTest : public testing::Test {
}
void TearDown() override {
+ g_barrier = nullptr;
PluginManager::UnregisterPlugin(CreateSymbolLocator);
HostInfo::Terminate();
FileSystem::Terminate();
@@ -144,6 +178,10 @@ class SymbolLocatorTest : public testing::Test {
FileSpec m_symbols = FileSpec("/binary.dSYM", FileSpec::Style::posix);
};
+std::vector<SymbolLocator::Request> MakeRequests(size_t count) {
+ return std::vector<SymbolLocator::Request>(count);
+}
+
} // namespace
TEST_F(SymbolLocatorTest, MissRunsEveryStep) {
@@ -269,3 +307,102 @@ TEST_F(SymbolLocatorTest, ThePluginsRunWhenThePlatformHasNothingToSay) {
EXPECT_EQ(1u, platform->find_module_files_calls);
EXPECT_TRUE(g_calls.located_object_file);
}
+
+TEST_F(SymbolLocatorTest, TheBatchSearchesEveryRequest) {
+ g_object_file = m_binary;
+ std::vector<SymbolLocator::Request> requests = MakeRequests(8);
+
+ std::vector<llvm::Expected<SymbolLocator::Result>> results =
+ SymbolLocator::Locate(requests, FileSpecList(), /*parallel=*/true);
+
+ ASSERT_EQ(requests.size(), results.size());
+ for (llvm::Expected<SymbolLocator::Result> &result : results) {
+ ASSERT_THAT_EXPECTED(result, llvm::Succeeded());
+ EXPECT_EQ(m_binary, result->module_spec.GetFileSpec());
+ }
+}
+
+TEST_F(SymbolLocatorTest, TheBatchKeepsResultsInRequestOrder) {
+ // Every other request is one the locator will answer, so the results can only
+ // line up with the requests if the order is kept.
+ g_object_file = m_binary;
+ g_only_with_uuid = true;
+ std::vector<SymbolLocator::Request> requests = MakeRequests(6);
+ for (auto [i, request] : llvm::enumerate(requests))
+ if (i % 2 == 0)
+ request.module_spec.GetUUID() = UUID("0123456789ABCDEF", 16);
+
+ std::vector<llvm::Expected<SymbolLocator::Result>> results =
+ SymbolLocator::Locate(requests, FileSpecList(), /*parallel=*/true);
+
+ ASSERT_EQ(requests.size(), results.size());
+ for (auto [i, result] : llvm::enumerate(results)) {
+ if (i % 2 == 0)
+ EXPECT_THAT_EXPECTED(result, llvm::Succeeded()) << "request " << i;
+ else
+ EXPECT_THAT_EXPECTED(result, llvm::Failed()) << "request " << i;
+ }
+}
+
+TEST_F(SymbolLocatorTest, TheBatchSearchesConcurrently) {
+ // A serial batch could never get every task inside the locator at once. No
+ // more tasks than the pool can run, or the ones left queued would hang it.
+ const size_t num_requests =
+ std::min<size_t>(4, Debugger::GetThreadPool().getMaxConcurrency());
+ if (num_requests < 2)
+ GTEST_SKIP() << "the thread pool runs one task at a time";
+
+ std::mutex mutex;
+ std::condition_variable cv;
+ size_t arrived = 0;
+ bool everyone_arrived = false;
+
+ g_barrier = [&] {
+ std::unique_lock<std::mutex> lock(mutex);
+ if (++arrived == num_requests) {
+ everyone_arrived = true;
+ cv.notify_all();
+ return;
+ }
+ // Assert on what the waiter observed, not on the count: a late arrival
+ // would set the flag either way.
+ bool released = cv.wait_for(lock, std::chrono::seconds(10),
+ [&] { return everyone_arrived; });
+ EXPECT_TRUE(released) << "the batch did not run concurrently";
+ };
+
+ std::vector<SymbolLocator::Request> requests = MakeRequests(num_requests);
+ std::vector<llvm::Expected<SymbolLocator::Result>> results =
+ SymbolLocator::Locate(requests, FileSpecList(), /*parallel=*/true);
+ for (llvm::Expected<SymbolLocator::Result> &result : results)
+ if (!result)
+ llvm::consumeError(result.takeError());
+
+ EXPECT_EQ(num_requests, arrived);
+}
+
+TEST_F(SymbolLocatorTest, TheSerialBatchGivesTheSameAnswers) {
+ g_object_file = m_binary;
+ std::vector<SymbolLocator::Request> requests = MakeRequests(4);
+
+ std::vector<llvm::Expected<SymbolLocator::Result>> parallel =
+ SymbolLocator::Locate(requests, FileSpecList(), /*parallel=*/true);
+ std::vector<llvm::Expected<SymbolLocator::Result>> serial =
+ SymbolLocator::Locate(requests, FileSpecList(), /*parallel=*/false);
+
+ ASSERT_EQ(parallel.size(), serial.size());
+ for (auto [p, s] : llvm::zip_equal(parallel, serial)) {
+ EXPECT_THAT_EXPECTED(p, llvm::Succeeded());
+ EXPECT_THAT_EXPECTED(s, llvm::Succeeded());
+ if (p && s)
+ EXPECT_EQ(p->module_spec.GetFileSpec(), s->module_spec.GetFileSpec());
+ }
+}
+
+TEST_F(SymbolLocatorTest, AnEmptyBatchIsNoWork) {
+ std::vector<llvm::Expected<SymbolLocator::Result>> results =
+ SymbolLocator::Locate({}, FileSpecList(), /*parallel=*/true);
+
+ EXPECT_TRUE(results.empty());
+ EXPECT_FALSE(g_calls.located_object_file);
+}
More information about the llvm-branch-commits
mailing list