Author: Jonas Devlieghere Date: 2026-08-14T23:49:24Z New Revision: 5aec94a1869b870428eac645ba9c7c34c76495da
URL: https://github.com/llvm/llvm-project/commit/5aec94a1869b870428eac645ba9c7c34c76495da DIFF: https://github.com/llvm/llvm-project/commit/5aec94a1869b870428eac645ba9c7c34c76495da.diff LOG: [lldb] Search for a corefile's images before loading any of them (#216431) 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 Added: Modified: lldb/include/lldb/Symbol/SymbolLocator.h lldb/source/Core/DynamicLoader.cpp lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp lldb/source/Symbol/SymbolLocator.cpp lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py lldb/unittests/Symbol/SymbolLocatorTest.cpp Removed: ################################################################################ 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..9420c5fd27def 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,23 @@ 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. + // Loading a binary that was never found already reports that, so a bare + // not-found error would only say it a second time. Any other error says + // something that report cannot. llvm::Error error = located.takeError(); if (error.isA<SymbolLocator::NotFound>()) llvm::consumeError(std::move(error)); @@ -287,14 +291,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 +323,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 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..9d2fe2d321407 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6674,17 +6674,22 @@ 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 up a platform binary 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 +6713,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); +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
