[Lldb-commits] [lldb] [lldb] Add a unified entry point for locating a binary and its symbols (PR #215391)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Aug 10 15:06:29 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
<details>
<summary>Changes</summary>
The three-plugin composition that finds a binary and its symbol file is open
coded in several places. Give it one home, as a pure function of a module
spec, so that a caller holding several binaries can search for all of them
before creating any.
Nothing on this path takes a lock, where ModuleList::GetSharedModule holds the
shared module list's lock across the plugin search. That is what will make
searching for several binaries at once worth doing.
A miss that nothing could explain gets its own error type rather than an error
code, because a Status carrying an errno converts to the same llvm::ECError,
and a caller composing its own message for a plain miss must not swallow a
failure to reach a symbol server.
DynamicLoader's search no longer runs twice for a binary that is not already
known, and a module created from a located binary is now registered in the
shared module list, so a second Target asking for the same binary reuses 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>
---
Full diff: https://github.com/llvm/llvm-project/pull/215391.diff
5 Files Affected:
- (modified) lldb/include/lldb/Symbol/SymbolLocator.h (+58)
- (modified) lldb/source/Core/DynamicLoader.cpp (+47-39)
- (modified) lldb/source/Symbol/SymbolLocator.cpp (+49)
- (modified) lldb/unittests/Symbol/CMakeLists.txt (+1)
- (added) lldb/unittests/Symbol/SymbolLocatorTest.cpp (+199)
``````````diff
diff --git a/lldb/include/lldb/Symbol/SymbolLocator.h b/lldb/include/lldb/Symbol/SymbolLocator.h
index 400b4adfd73c4..82e142ab18d9a 100644
--- a/lldb/include/lldb/Symbol/SymbolLocator.h
+++ b/lldb/include/lldb/Symbol/SymbolLocator.h
@@ -9,15 +9,73 @@
#ifndef LLDB_SYMBOL_SYMBOLLOCATOR_H
#define LLDB_SYMBOL_SYMBOLLOCATOR_H
+#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/PluginInterface.h"
+#include "lldb/Target/Statistics.h"
+#include "lldb/Utility/Status.h"
#include "lldb/Utility/UUID.h"
+#include "llvm/Support/Error.h"
+
+#include <system_error>
+
namespace lldb_private {
class SymbolLocator : public PluginInterface {
public:
SymbolLocator() = default;
+ /// A binary was not found and nothing could say why. Its own error type, so
+ /// that a caller composing its own message for that case cannot mistake a
+ /// real failure for it.
+ class NotFound : public llvm::ErrorInfo<NotFound> {
+ public:
+ static char ID;
+
+ void log(llvm::raw_ostream &os) const override;
+ std::error_code convertToErrorCode() const override;
+ };
+
+ /// One binary to search for.
+ struct Request {
+ /// What to look for.
+ ModuleSpec module_spec;
+
+ /// Allow contacting an external symbol server when the local searches come
+ /// up empty.
+ bool external_lookup = false;
+ };
+
+ /// What a search found.
+ struct Result {
+ /// The binary, and its symbol file if there is one to be had.
+ ModuleSpec module_spec;
+
+ /// What an external symbol server had to say about the symbols, even
+ /// though the binary itself was found. Recorded rather than reported, so
+ /// that it reaches the user in the caller's order.
+ Status symbol_error;
+
+ /// Where the time went, to be merged into the Module once it exists.
+ StatisticsMap statistics;
+ };
+
+ /// Find a binary and, if possible, its symbols.
+ ///
+ /// Mutates no Target and no Process, which is what lets a caller holding
+ /// several binaries search for all of them before installing any.
+ ///
+ /// \return
+ /// Where the binary is, or an error if it could not be found at all.
+ /// Finding the binary but not its symbols is a success, with
+ /// Result::symbol_error carrying whatever a symbol server had to say
+ /// about them.
+ ///
+ /// A miss carries an external symbol server's explanation if it gave
+ /// one, and is a NotFound when nothing could say why.
+ static llvm::Expected<Result> Locate(const Request &request,
+ const FileSpecList &search_paths);
+
/// 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
diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp
index 1f85b655ba116..5c12598ee4325 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/lldb/source/Core/DynamicLoader.cpp
@@ -16,6 +16,7 @@
#include "lldb/Core/Progress.h"
#include "lldb/Core/Section.h"
#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Symbol/SymbolLocator.h"
#include "lldb/Target/MemoryRegionInfo.h"
#include "lldb/Target/Platform.h"
#include "lldb/Target/Process.h"
@@ -258,47 +259,54 @@ static void SearchForBinary(Target &target, DynamicLoader::BinarySpec &bin_spec,
if (FileSystem::Instance().Exists(name_filespec))
module_spec.GetFileSpec() = name_filespec;
- // 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 (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 (!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);
- }
+ // Has lldb already seen a module with this UUID? A module whose symbols are
+ // already in hand is the answer, and searching would only find them again.
+ // Without them the search still has something to add.
+ ModuleList::GetSharedModule(module_spec, bin_spec.module_sp, nullptr, nullptr,
+ /*invoke_locate_callback=*/true,
+ /*invoke_symbol_locators=*/false);
+ if (bin_spec.module_sp && bin_spec.module_sp->GetSymbolFileFileSpec())
+ return;
+
+ // Search for the binary and its symbols.
+ SymbolLocator::Request request;
+ request.module_spec = module_spec;
+ request.external_lookup = bin_spec.force_symbol_search;
+
+ llvm::Expected<SymbolLocator::Result> located =
+ SymbolLocator::Locate(request, search_paths);
+ 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.
+ llvm::Error error = located.takeError();
+ if (error.isA<SymbolLocator::NotFound>())
+ llvm::consumeError(std::move(error));
+ else
+ bin_spec.error = Status::FromError(std::move(error));
+ return;
}
- // 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);
+ // A binary was found. Its symbols are another matter, and the caller 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.
+ ModuleSP located_module_sp;
+ ModuleList::GetSharedModule(located->module_spec, located_module_sp, nullptr,
+ nullptr, /*invoke_locate_callback=*/false,
+ /*invoke_symbol_locators=*/false);
+
+ // A located binary always yields a module, whatever ObjectFile makes of the
+ // file, because the caller has nowhere else to record what the search found.
+ if (!located_module_sp)
+ located_module_sp = std::make_shared<Module>(located->module_spec);
+
+ // Published only now, so that a search that came up empty leaves whatever the
+ // shared module list had in hand.
+ bin_spec.module_sp = std::move(located_module_sp);
+ bin_spec.module_sp->GetSymbolLocatorStatistics().merge(located->statistics);
}
static void FindBinaryUUIDInMemory(Process *process,
diff --git a/lldb/source/Symbol/SymbolLocator.cpp b/lldb/source/Symbol/SymbolLocator.cpp
index 93a5bc428b614..8f22d32d72db9 100644
--- a/lldb/source/Symbol/SymbolLocator.cpp
+++ b/lldb/source/Symbol/SymbolLocator.cpp
@@ -10,6 +10,7 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Core/PluginManager.h"
+#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
#include "llvm/ADT/SmallSet.h"
@@ -18,6 +19,54 @@
using namespace lldb;
using namespace lldb_private;
+char SymbolLocator::NotFound::ID = 0;
+
+void SymbolLocator::NotFound::log(llvm::raw_ostream &os) const {
+ os << "binary not found";
+}
+
+std::error_code SymbolLocator::NotFound::convertToErrorCode() const {
+ return std::make_error_code(std::errc::no_such_file_or_directory);
+}
+
+llvm::Expected<SymbolLocator::Result>
+SymbolLocator::Locate(const Request &request,
+ const FileSpecList &search_paths) {
+ FileSystem &fs = FileSystem::Instance();
+ Result result;
+ ModuleSpec &module_spec = result.module_spec;
+ module_spec = request.module_spec;
+
+ // Can lldb's symbol and executable location schemes find them locally?
+ module_spec.GetSymbolFileSpec() = PluginManager::LocateExecutableSymbolFile(
+ module_spec, search_paths, result.statistics);
+ module_spec.GetFileSpec() =
+ PluginManager::LocateExecutableObjectFile(module_spec, result.statistics)
+ .GetFileSpec();
+
+ // If we're still missing either file, see if an external symbol server can
+ // provide it.
+ if (!fs.Exists(module_spec.GetFileSpec()) ||
+ !fs.Exists(module_spec.GetSymbolFileSpec())) {
+ Status error;
+ PluginManager::DownloadObjectAndSymbolFile(module_spec, error,
+ request.external_lookup);
+ if (error.Fail() && request.external_lookup)
+ result.symbol_error = std::move(error);
+ }
+
+ // Not finding the binary is the one hard failure. Whatever a symbol server
+ // had to say is the best explanation available for it. Without one there is
+ // nothing to add beyond the fact of the miss.
+ if (!fs.Exists(module_spec.GetFileSpec())) {
+ if (llvm::Error error = result.symbol_error.takeError())
+ return std::move(error);
+ return llvm::make_error<NotFound>();
+ }
+
+ return result;
+}
+
void SymbolLocator::DownloadSymbolFileAsync(const UUID &uuid) {
static llvm::SmallSet<UUID, 8> g_seen_uuids;
static std::mutex g_mutex;
diff --git a/lldb/unittests/Symbol/CMakeLists.txt b/lldb/unittests/Symbol/CMakeLists.txt
index f9794369a89f4..0fdbc9d72445e 100644
--- a/lldb/unittests/Symbol/CMakeLists.txt
+++ b/lldb/unittests/Symbol/CMakeLists.txt
@@ -4,6 +4,7 @@ add_lldb_unittest(SymbolTests
LocateSymbolFileTest.cpp
MangledTest.cpp
PostfixExpressionTest.cpp
+ SymbolLocatorTest.cpp
SymbolTest.cpp
SymtabTest.cpp
SymStoreTest.cpp
diff --git a/lldb/unittests/Symbol/SymbolLocatorTest.cpp b/lldb/unittests/Symbol/SymbolLocatorTest.cpp
new file mode 100644
index 0000000000000..6456f4fc18ad0
--- /dev/null
+++ b/lldb/unittests/Symbol/SymbolLocatorTest.cpp
@@ -0,0 +1,199 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Symbol/SymbolLocator.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/FileSpecList.h"
+
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Testing/Support/Error.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+/// Which steps of the search ran, so a test can tell where an answer came from.
+struct LocatorCalls {
+ bool located_symbol_file = false;
+ bool located_object_file = false;
+ bool downloaded = false;
+};
+
+LocatorCalls g_calls;
+
+/// What the object file locator claims to have found, if anything.
+std::optional<FileSpec> g_object_file;
+
+/// What the symbol file locator claims to have found, if anything.
+std::optional<FileSpec> g_symbol_file;
+
+/// When set, the symbol server fails the way a failure to launch it does, with
+/// an errno rather than a message.
+bool g_symbol_server_errno = false;
+
+std::optional<FileSpec> LocateExecutableSymbolFile(const ModuleSpec &,
+ const FileSpecList &) {
+ g_calls.located_symbol_file = true;
+ return g_symbol_file;
+}
+
+std::optional<ModuleSpec> LocateExecutableObjectFile(const ModuleSpec &spec) {
+ g_calls.located_object_file = true;
+ if (!g_object_file)
+ return {};
+ ModuleSpec located(spec);
+ located.GetFileSpec() = *g_object_file;
+ return located;
+}
+
+bool DownloadObjectAndSymbolFile(ModuleSpec &, Status &error, bool force_lookup,
+ bool) {
+ g_calls.downloaded = true;
+ if (!force_lookup)
+ return false;
+ if (g_symbol_server_errno)
+ error = Status(std::make_error_code(std::errc::too_many_files_open));
+ else
+ error = Status::FromErrorString("the symbol server said no");
+ return false;
+}
+
+SymbolLocator *CreateSymbolLocator() { return nullptr; }
+
+class SymbolLocatorTest : public testing::Test {
+public:
+ SymbolLocatorTest()
+ : m_empty_buffer("", "<empty buffer>"),
+ m_fs(new llvm::vfs::InMemoryFileSystem()) {}
+
+ void SetUp() override {
+ // 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);
+ HostInfo::Initialize();
+ m_fs->addFileNoOwn(m_binary.GetPath(), 0, m_empty_buffer);
+ m_fs->addFileNoOwn(m_symbols.GetPath(), 0, m_empty_buffer);
+
+ g_calls = LocatorCalls();
+ g_object_file = std::nullopt;
+ g_symbol_file = std::nullopt;
+ g_symbol_server_errno = false;
+ ASSERT_TRUE(PluginManager::RegisterPlugin(
+ "test", "test symbol locator", CreateSymbolLocator,
+ LocateExecutableObjectFile, LocateExecutableSymbolFile,
+ DownloadObjectAndSymbolFile));
+ }
+
+ void TearDown() override {
+ PluginManager::UnregisterPlugin(CreateSymbolLocator);
+ HostInfo::Terminate();
+ FileSystem::Terminate();
+ }
+
+ llvm::MemoryBufferRef m_empty_buffer;
+ llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> m_fs;
+
+ /// Files that exist, for the cases that want a hit.
+ FileSpec m_binary = FileSpec("/binary", FileSpec::Style::posix);
+ FileSpec m_symbols = FileSpec("/binary.dSYM", FileSpec::Style::posix);
+};
+
+} // namespace
+
+TEST_F(SymbolLocatorTest, MissRunsEveryStep) {
+ llvm::Expected<SymbolLocator::Result> result =
+ SymbolLocator::Locate(SymbolLocator::Request(), FileSpecList());
+
+ EXPECT_TRUE(g_calls.located_symbol_file);
+ EXPECT_TRUE(g_calls.located_object_file);
+ // Neither file was found, so the symbol server is the last resort.
+ EXPECT_TRUE(g_calls.downloaded);
+
+ // Nothing was found and no symbol server was asked, so there is nothing to
+ // say beyond the fact of the miss. The caller composes its own message.
+ ASSERT_FALSE(result);
+ llvm::Error error = result.takeError();
+ EXPECT_TRUE(error.isA<SymbolLocator::NotFound>());
+ llvm::consumeError(std::move(error));
+}
+
+TEST_F(SymbolLocatorTest, ReportsWhatThePluginsFound) {
+ g_object_file = m_binary;
+ g_symbol_file = m_symbols;
+
+ llvm::Expected<SymbolLocator::Result> result =
+ SymbolLocator::Locate(SymbolLocator::Request(), FileSpecList());
+
+ ASSERT_THAT_EXPECTED(result, llvm::Succeeded());
+ EXPECT_EQ(m_binary, result->module_spec.GetFileSpec());
+ EXPECT_EQ(m_symbols, result->module_spec.GetSymbolFileSpec());
+ // Both files were in hand, so there was nothing to ask a symbol server for.
+ EXPECT_FALSE(g_calls.downloaded);
+}
+
+TEST_F(SymbolLocatorTest, MissCarriesSymbolServerError) {
+ SymbolLocator::Request request;
+ request.external_lookup = true;
+
+ llvm::Expected<SymbolLocator::Result> result =
+ SymbolLocator::Locate(request, FileSpecList());
+
+ ASSERT_FALSE(result);
+ llvm::Error error = result.takeError();
+ EXPECT_FALSE(error.isA<SymbolLocator::NotFound>());
+ EXPECT_EQ("the symbol server said no", llvm::toString(std::move(error)));
+}
+
+TEST_F(SymbolLocatorTest, BinaryWithoutSymbolsIsASuccess) {
+ // A binary without symbols is still worth loading, and the caller still gets
+ // to tell the user why the symbols are missing.
+ g_object_file = m_binary;
+ SymbolLocator::Request request;
+ request.external_lookup = true;
+
+ llvm::Expected<SymbolLocator::Result> result =
+ SymbolLocator::Locate(request, FileSpecList());
+
+ ASSERT_THAT_EXPECTED(result, llvm::Succeeded());
+ EXPECT_EQ(m_binary, result->module_spec.GetFileSpec());
+ EXPECT_FALSE(result->module_spec.GetSymbolFileSpec());
+ EXPECT_TRUE(result->symbol_error.Fail());
+}
+
+TEST_F(SymbolLocatorTest, SymbolFileWithoutBinaryIsAMiss) {
+ // Symbols with no binary to apply them to are of no use, so this is the hard
+ // failure and not a success carrying half an answer.
+ g_symbol_file = m_symbols;
+
+ llvm::Expected<SymbolLocator::Result> result =
+ SymbolLocator::Locate(SymbolLocator::Request(), FileSpecList());
+
+ EXPECT_THAT_EXPECTED(result, llvm::Failed());
+}
+
+TEST_F(SymbolLocatorTest, AnErrnoFromTheSymbolServerIsNotAPlainMiss) {
+ // A Status carrying an errno converts to an llvm::ECError, so an error code
+ // is not enough to tell a real failure from the miss sentinel. Failing to
+ // even launch a symbol server has to reach the user.
+ SymbolLocator::Request request;
+ request.external_lookup = true;
+ g_symbol_server_errno = true;
+
+ llvm::Expected<SymbolLocator::Result> result =
+ SymbolLocator::Locate(request, FileSpecList());
+
+ ASSERT_FALSE(result);
+ llvm::Error error = result.takeError();
+ EXPECT_FALSE(error.isA<SymbolLocator::NotFound>());
+ llvm::consumeError(std::move(error));
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/215391
More information about the lldb-commits
mailing list