https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/214829
GetSharedModule falls back on the symbol locator plugins when the spec does not already name a binary it can open, and it does so while holding the global module list lock. Locating a binary or symbol file can potentially be quite slow, so we should have the option to opt out of that, for example, if we already did the search upfront. >From 5e4fe84b93494676ec6172330218d58e913071af Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <[email protected]> Date: Thu, 6 Aug 2026 22:10:40 -0700 Subject: [PATCH] [lldb] Let callers of GetSharedModule skip symbol locating (NFC) GetSharedModule falls back on the symbol locator plugins when the spec does not already name a binary it can open, and it does so while holding the global module list lock. Locating a binary or symbol file can potentially be quite slow, so we should have the option to opt out of that, for example, if we already did the search upfront. --- lldb/include/lldb/Core/ModuleList.h | 3 +- lldb/source/Core/ModuleList.cpp | 18 ++++++-- lldb/unittests/Core/ModuleListTest.cpp | 60 ++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 5 deletions(-) diff --git a/lldb/include/lldb/Core/ModuleList.h b/lldb/include/lldb/Core/ModuleList.h index bea88a6cf1f51..aa0e7c674c1d2 100644 --- a/lldb/include/lldb/Core/ModuleList.h +++ b/lldb/include/lldb/Core/ModuleList.h @@ -497,7 +497,8 @@ class ModuleList { static Status GetSharedModule(const ModuleSpec &module_spec, lldb::ModuleSP &module_sp, llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules, - bool *did_create_ptr, bool invoke_locate_callback = true); + bool *did_create_ptr, bool invoke_locate_callback = true, + bool invoke_symbol_locators = true); static bool RemoveSharedModule(lldb::ModuleSP &module_sp); diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp index 88ddc62407bb4..4f81d8db7198d 100644 --- a/lldb/source/Core/ModuleList.cpp +++ b/lldb/source/Core/ModuleList.cpp @@ -1053,7 +1053,8 @@ size_t ModuleList::RemoveOrphanSharedModules(bool mandatory) { Status ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp, llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules, - bool *did_create_ptr, bool invoke_locate_callback) { + bool *did_create_ptr, bool invoke_locate_callback, + bool invoke_symbol_locators) { SharedModuleList &shared_module_list = GetSharedModuleList(); std::lock_guard<std::recursive_mutex> guard(shared_module_list.GetMutex()); char path[PATH_MAX]; @@ -1203,9 +1204,18 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp, } } - // Either the file didn't exist where at the path, or no path was given, so - // we now have to use more extreme measures to try and find the appropriate - // module. + // Either the file didn't exist where at the path, or no path was given, so we + // now either have to use more extreme measures to try and find the + // appropriate module or end our search here. + if (!invoke_symbol_locators) { + std::string uuid_str; + if (uuid_ptr && uuid_ptr->IsValid()) + uuid_str = uuid_ptr->GetAsString(); + if (!uuid_str.empty()) + return Status::FromErrorStringWithFormatv( + "cannot locate module for UUID '{}'", uuid_str); + return Status::FromErrorString("cannot locate module"); + } // Fixup the incoming path in case the path points to a valid file, yet the // arch or UUID (if one was passed in) don't match. diff --git a/lldb/unittests/Core/ModuleListTest.cpp b/lldb/unittests/Core/ModuleListTest.cpp index b4747b7987059..399036ca64c5a 100644 --- a/lldb/unittests/Core/ModuleListTest.cpp +++ b/lldb/unittests/Core/ModuleListTest.cpp @@ -11,7 +11,9 @@ #include "TestingSupport/TestUtilities.h" #include "lldb/Core/Module.h" #include "lldb/Core/ModuleSpec.h" +#include "lldb/Core/PluginManager.h" #include "lldb/Host/FileSystem.h" +#include "lldb/Symbol/SymbolLocator.h" #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/UUID.h" @@ -173,3 +175,61 @@ TEST(ModuleListTest, GetSharedModuleByUUIDIgnoresPath) { } } } + +// A symbol locator that records whether it was asked to find anything. It never +// finds a binary, so it does not otherwise change what GetSharedModule does. +static bool g_locate_executable_object_file_called = false; + +static std::optional<ModuleSpec> LocateExecutableObjectFile(const ModuleSpec &) { + g_locate_executable_object_file_called = true; + return {}; +} + +static SymbolLocator *CreateSymbolLocator() { return nullptr; } + +class SymbolLocatorGate : public testing::Test { +public: + void SetUp() override { + g_locate_executable_object_file_called = false; + ASSERT_TRUE(PluginManager::RegisterPlugin( + "test", "test symbol locator", CreateSymbolLocator, + LocateExecutableObjectFile)); + } + + void TearDown() override { + PluginManager::UnregisterPlugin(CreateSymbolLocator); + } + + // A spec that names no file on disk, so GetSharedModule has to fall through + // to the symbol locators to have any chance of finding a binary. + static ModuleSpec MissingModuleSpec() { + ModuleSpec spec; + spec.GetFileSpec() = FileSpec("/nonexistent/libtest.so"); + spec.GetArchitecture() = ArchSpec("x86_64-pc-linux"); + spec.GetUUID() = UUID("0123456789ABCDEF", 16); + return spec; + } +}; + +TEST_F(SymbolLocatorGate, GetSharedModuleInvokesSymbolLocatorsByDefault) { + SubsystemRAII<FileSystem, ObjectFileELF> subsystems; + + ModuleSP module_sp; + ModuleList::GetSharedModule(MissingModuleSpec(), module_sp, nullptr, nullptr); + + EXPECT_TRUE(g_locate_executable_object_file_called); +} + +TEST_F(SymbolLocatorGate, GetSharedModuleCanSkipSymbolLocators) { + SubsystemRAII<FileSystem, ObjectFileELF> subsystems; + + ModuleSP module_sp; + Status error = ModuleList::GetSharedModule( + MissingModuleSpec(), module_sp, nullptr, nullptr, + /*invoke_locate_callback=*/true, /*invoke_symbol_locators=*/false); + + EXPECT_FALSE(g_locate_executable_object_file_called); + // The caller still learns that nothing was found. + EXPECT_FALSE(module_sp); + EXPECT_TRUE(error.Fail()); +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
