Author: Jonas Devlieghere Date: 2026-08-14T09:21:58-07:00 New Revision: ea051fd558d94cd08348834ebecac4af231efeda
URL: https://github.com/llvm/llvm-project/commit/ea051fd558d94cd08348834ebecac4af231efeda DIFF: https://github.com/llvm/llvm-project/commit/ea051fd558d94cd08348834ebecac4af231efeda.diff LOG: [lldb] Search for a corefile's images before loading any of them (#215393) 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: llvm/test/tools/dsymutil/Inputs/subprogram-high-pc-past-symbol-dwarf2.s llvm/test/tools/dsymutil/Inputs/subprogram-high-pc-past-symbol.s llvm/test/tools/dsymutil/subprogram-high-pc-past-symbol.test 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 llvm/include/llvm/DWARFLinker/AddressesMap.h llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp llvm/lib/DWARFLinker/Parallel/DIEAttributeCloner.cpp llvm/lib/DWARFLinker/Parallel/DependencyTracker.cpp llvm/tools/dsymutil/DwarfLinkerForBinary.h 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); +} diff --git a/llvm/include/llvm/DWARFLinker/AddressesMap.h b/llvm/include/llvm/DWARFLinker/AddressesMap.h index 38b9a67b124c1..443e6f5bb4186 100644 --- a/llvm/include/llvm/DWARFLinker/AddressesMap.h +++ b/llvm/include/llvm/DWARFLinker/AddressesMap.h @@ -83,22 +83,47 @@ class AddressesMap { /// Erases all data. virtual void clear() = 0; - /// This is used for assembly files where labels may not have high_pc - /// but the debug map has range information from symbols. - struct AssemblyRange { - AssemblyRange(uint64_t LowPC, uint64_t HighPC) + /// The extent the linker gave a symbol, in source address space. + struct SymbolRange { + SymbolRange(uint64_t LowPC, uint64_t HighPC) : LowPC(LowPC), HighPC(HighPC) {} uint64_t LowPC; uint64_t HighPC; }; - /// Returns the address range containing \p Addr if available. - /// \returns the range [LowPC, HighPC) containing Addr. - virtual std::optional<AssemblyRange> - getAssemblyRangeForAddress(uint64_t Addr) { + /// Returns the symbol range [LowPC, HighPC) containing \p Addr, if known. + virtual std::optional<SymbolRange> getSymbolRangeForAddress(uint64_t Addr) { return std::nullopt; } + /// Returns the linked address of the first symbol placed at or after + /// \p LinkedAddr, if one is known. + virtual std::optional<uint64_t> + getNextLinkedSymbolStart(uint64_t LinkedAddr) { + return std::nullopt; + } + + /// Constrains the source-space end of a code range, given its start \p LowPC, + /// its end \p HighPC as an address, and the \p Adjustment all of its + /// addresses shift by in the output. + /// + /// Neighbouring symbols shift by diff erent amounts, so a range reaching past + /// the symbol holding its start can land inside the next symbol in the + /// output. Only that collision is repaired. Coverage that overlaps nothing is + /// left alone, and a symbol nested in the same extent is never a neighbour, + /// so it cannot shorten a range that legitimately spans it. + uint64_t constrainCodeRangeHighPC(uint64_t LowPC, uint64_t HighPC, + int64_t Adjustment) { + std::optional<SymbolRange> Symbol = getSymbolRangeForAddress(LowPC); + if (!Symbol) + return HighPC; + std::optional<uint64_t> NextStart = + getNextLinkedSymbolStart(Symbol->HighPC + Adjustment); + if (!NextStart) + return HighPC; + return std::min(HighPC, *NextStart - Adjustment); + } + /// This function checks whether variable has DWARF expression containing /// operation referencing live address(f.e. DW_OP_addr, DW_OP_addrx...). /// \returns first is true if the expression has an operation referencing an diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp index 24289e0906586..5c8f718c8670f 100644 --- a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp +++ b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp @@ -672,7 +672,7 @@ unsigned DWARFLinker::shouldKeepSubprogramDIE( // function ranges when available, falling back to labels otherwise. if (Unit.getLanguage() == dwarf::DW_LANG_Mips_Assembler || Unit.getLanguage() == dwarf::DW_LANG_Assembly) { - if (auto Range = RelocMgr.getAssemblyRangeForAddress(*LowPc)) { + if (auto Range = RelocMgr.getSymbolRangeForAddress(*LowPc)) { Unit.addFunctionRange(Range->LowPC, Range->HighPC, MyInfo.AddrAdjust); } else { Unit.addLabelLowPc(*LowPc, MyInfo.AddrAdjust); @@ -698,7 +698,10 @@ unsigned DWARFLinker::shouldKeepSubprogramDIE( } // Replace the debug map range with a more accurate one. - Unit.addFunctionRange(*LowPc, *HighPc, MyInfo.AddrAdjust); + Unit.addFunctionRange( + *LowPc, + RelocMgr.constrainCodeRangeHighPC(*LowPc, *HighPc, MyInfo.AddrAdjust), + MyInfo.AddrAdjust); return Flags; } @@ -1471,6 +1474,14 @@ unsigned DWARFLinker::DIECloner::cloneAddressAttribute( else return 0; } else { + // A nested scope inherits the range its parent function overran, so every + // range is constrained, not just the subprogram's own. + if (AttrSpec.Attr == dwarf::DW_AT_high_pc) { + if (std::optional<uint64_t> LowPC = + dwarf::toAddress(InputDIE.find(dwarf::DW_AT_low_pc))) + Addr = ObjFile.Addresses->constrainCodeRangeHighPC(*LowPC, *Addr, + Info.PCOffset); + } *Addr += Info.PCOffset; } @@ -1629,6 +1640,14 @@ unsigned DWARFLinker::DIECloner::cloneScalarAttribute( return 0; } + if (AttrSpec.Attr == dwarf::DW_AT_high_pc) { + if (std::optional<uint64_t> LowPC = + dwarf::toAddress(InputDIE.find(dwarf::DW_AT_low_pc))) + Value = File.Addresses->constrainCodeRangeHighPC(*LowPC, *LowPC + Value, + Info.PCOffset) - + *LowPC; + } + DIE::value_iterator Patch = Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr), dwarf::Form(AttrSpec.Form), DIEInteger(Value)); diff --git a/llvm/lib/DWARFLinker/Parallel/DIEAttributeCloner.cpp b/llvm/lib/DWARFLinker/Parallel/DIEAttributeCloner.cpp index f58869d20291c..77a1d85edb772 100644 --- a/llvm/lib/DWARFLinker/Parallel/DIEAttributeCloner.cpp +++ b/llvm/lib/DWARFLinker/Parallel/DIEAttributeCloner.cpp @@ -525,6 +525,16 @@ size_t DIEAttributeCloner::cloneScalarAttr( !OutUnit.isCompileUnit()) return 0; + // A nested scope inherits the range its parent function overran, so every + // range is constrained, not just the subprogram's own. + if (AttrSpec.Attr == dwarf::DW_AT_high_pc && FuncAddressAdjustment) { + if (std::optional<uint64_t> LowPC = + dwarf::toAddress(InUnit.find(InputDieEntry, dwarf::DW_AT_low_pc))) + Value = InUnit.getContaingFile().Addresses->constrainCodeRangeHighPC( + *LowPC, *LowPC + Value, *FuncAddressAdjustment) - + *LowPC; + } + auto Result = Generator.addScalarAttribute(AttrSpec.Attr, ResultingForm, Value); // Record DW_AT_LLVM_stmt_sequence so the attribute value can be @@ -679,6 +689,12 @@ size_t DIEAttributeCloner::cloneAddressAttr( else return 0; } else { + if (AttrSpec.Attr == dwarf::DW_AT_high_pc && FuncAddressAdjustment) { + if (std::optional<uint64_t> LowPC = + dwarf::toAddress(InUnit.find(InputDieEntry, dwarf::DW_AT_low_pc))) + Addr = InUnit.getContaingFile().Addresses->constrainCodeRangeHighPC( + *LowPC, *Addr, *FuncAddressAdjustment); + } if (VarAddressAdjustment) *Addr += *VarAddressAdjustment; else if (FuncAddressAdjustment) diff --git a/llvm/lib/DWARFLinker/Parallel/DependencyTracker.cpp b/llvm/lib/DWARFLinker/Parallel/DependencyTracker.cpp index 124e73b1a15bd..0993497ff080b 100644 --- a/llvm/lib/DWARFLinker/Parallel/DependencyTracker.cpp +++ b/llvm/lib/DWARFLinker/Parallel/DependencyTracker.cpp @@ -952,14 +952,15 @@ bool DependencyTracker::isLiveSubprogramEntry(const UnitEntryPairTy &Entry) { // For assembly-language CUs there are typically no DW_TAG_subprogram // DIEs, so labels are the only addresses we see. Fall back to the - // assembly-range lookup to recover a function range for the line-table + // symbol-range lookup to recover a function range for the line-table // filter; otherwise the output line table would be empty. uint16_t Language = dwarf::toUnsigned( Entry.CU->getOrigUnit().getUnitDIE().find(dwarf::DW_AT_language), 0); if (Language == dwarf::DW_LANG_Mips_Assembler || Language == dwarf::DW_LANG_Assembly) { - if (auto Range = Entry.CU->getContaingFile() - .Addresses->getAssemblyRangeForAddress(*LowPc)) + if (auto Range = + Entry.CU->getContaingFile().Addresses->getSymbolRangeForAddress( + *LowPc)) Entry.CU->addFunctionRange(Range->LowPC, Range->HighPC, *RelocAdjustment); } @@ -974,6 +975,10 @@ bool DependencyTracker::isLiveSubprogramEntry(const UnitEntryPairTy &Entry) { if (!Info.getTrackLiveness() || DIE.getTag() == dwarf::DW_TAG_label) return true; - Entry.CU->addFunctionRange(*LowPc, *HighPc, *RelocAdjustment); + Entry.CU->addFunctionRange( + *LowPc, + Entry.CU->getContaingFile().Addresses->constrainCodeRangeHighPC( + *LowPc, *HighPc, *RelocAdjustment), + *RelocAdjustment); return true; } diff --git a/llvm/test/tools/dsymutil/Inputs/subprogram-high-pc-past-symbol-dwarf2.s b/llvm/test/tools/dsymutil/Inputs/subprogram-high-pc-past-symbol-dwarf2.s new file mode 100644 index 0000000000000..49933495d7dc6 --- /dev/null +++ b/llvm/test/tools/dsymutil/Inputs/subprogram-high-pc-past-symbol-dwarf2.s @@ -0,0 +1,72 @@ +; The DWARF 2 form of subprogram-high-pc-past-symbol.s, where DW_AT_high_pc is +; an address rather than a length. + + .text + .globl _a + .p2align 2 +_a: + ret +_filler: + nop + .globl _b + .p2align 2 +_b: + ret + + .section __DWARF,__debug_abbrev,regular,debug + .byte 1 ; abbrev 1: DW_TAG_compile_unit + .byte 0x11 + .byte 1 ; DW_CHILDREN_yes + .byte 0x25, 0x08 ; DW_AT_producer, DW_FORM_string + .byte 0x13, 0x0b ; DW_AT_language, DW_FORM_data1 + .byte 0x03, 0x08 ; DW_AT_name, DW_FORM_string + .byte 0x11, 0x01 ; DW_AT_low_pc, DW_FORM_addr + .byte 0x12, 0x01 ; DW_AT_high_pc, DW_FORM_addr + .byte 0, 0 + .byte 2 ; abbrev 2: DW_TAG_subprogram + .byte 0x2e + .byte 1 ; DW_CHILDREN_yes + .byte 0x03, 0x08 ; DW_AT_name, DW_FORM_string + .byte 0x11, 0x01 ; DW_AT_low_pc, DW_FORM_addr + .byte 0x12, 0x01 ; DW_AT_high_pc, DW_FORM_addr + .byte 0x3f, 0x0c ; DW_AT_external, DW_FORM_flag + .byte 0, 0 + .byte 3 ; abbrev 3: DW_TAG_lexical_block + .byte 0x0b + .byte 0 ; DW_CHILDREN_no + .byte 0x11, 0x01 ; DW_AT_low_pc, DW_FORM_addr + .byte 0x12, 0x01 ; DW_AT_high_pc, DW_FORM_addr + .byte 0, 0 + .byte 0 + + .section __DWARF,__debug_info,regular,debug +Lcu_begin: + .long Lcu_end-Lcu_version +Lcu_version: + .short 2 + .long 0 + .byte 8 + .byte 1 ; DW_TAG_compile_unit + .asciz "hand-written" + .byte 0x0c ; DW_LANG_C99 + .asciz "t.c" + .quad _a + .quad 0xc ; _a, _filler and _b together + .byte 2 ; DW_TAG_subprogram "a" + .asciz "a" + .quad _a + .quad 0x8 ; four bytes past the end of _a + .byte 1 + .byte 3 ; DW_TAG_lexical_block in "a" + .quad _a + .quad 0x8 ; reaches as far as its parent + .byte 0 ; end of "a"'s children + .byte 2 ; DW_TAG_subprogram "b" + .asciz "b" + .quad _b + .quad 0xc + .byte 1 + .byte 0 ; end of "b"'s children + .byte 0 ; end of the compile unit's children +Lcu_end: + .subsections_via_symbols diff --git a/llvm/test/tools/dsymutil/Inputs/subprogram-high-pc-past-symbol.s b/llvm/test/tools/dsymutil/Inputs/subprogram-high-pc-past-symbol.s new file mode 100644 index 0000000000000..f7d02714a2940 --- /dev/null +++ b/llvm/test/tools/dsymutil/Inputs/subprogram-high-pc-past-symbol.s @@ -0,0 +1,76 @@ +; _filler is an unreferenced atom between the two functions, so the linker drops +; it and places _b four bytes after _a. DW_AT_high_pc for _a reaches all the way +; to _b, past the code the linker keeps for it, and the lexical block inside _a +; ends with its parent. +; +; The DWARF is hand written because a producer normally agrees with the linker. + + .text + .globl _a + .p2align 2 +_a: + ret +_filler: + nop + .globl _b + .p2align 2 +_b: + ret + + .section __DWARF,__debug_abbrev,regular,debug + .byte 1 ; abbrev 1: DW_TAG_compile_unit + .byte 0x11 + .byte 1 ; DW_CHILDREN_yes + .byte 0x25, 0x08 ; DW_AT_producer, DW_FORM_string + .byte 0x13, 0x0b ; DW_AT_language, DW_FORM_data1 + .byte 0x03, 0x08 ; DW_AT_name, DW_FORM_string + .byte 0x11, 0x01 ; DW_AT_low_pc, DW_FORM_addr + .byte 0x12, 0x06 ; DW_AT_high_pc, DW_FORM_data4 + .byte 0, 0 + .byte 2 ; abbrev 2: DW_TAG_subprogram + .byte 0x2e + .byte 1 ; DW_CHILDREN_yes + .byte 0x03, 0x08 ; DW_AT_name, DW_FORM_string + .byte 0x11, 0x01 ; DW_AT_low_pc, DW_FORM_addr + .byte 0x12, 0x06 ; DW_AT_high_pc, DW_FORM_data4 + .byte 0x3f, 0x0c ; DW_AT_external, DW_FORM_flag + .byte 0, 0 + .byte 3 ; abbrev 3: DW_TAG_lexical_block + .byte 0x0b + .byte 0 ; DW_CHILDREN_no + .byte 0x11, 0x01 ; DW_AT_low_pc, DW_FORM_addr + .byte 0x12, 0x06 ; DW_AT_high_pc, DW_FORM_data4 + .byte 0, 0 + .byte 0 + + .section __DWARF,__debug_info,regular,debug +Lcu_begin: + .long Lcu_end-Lcu_version +Lcu_version: + .short 4 + .long 0 + .byte 8 + .byte 1 ; DW_TAG_compile_unit + .asciz "hand-written" + .byte 0x0c ; DW_LANG_C99 + .asciz "t.c" + .quad _a + .long 0xc ; _a, _filler and _b together + .byte 2 ; DW_TAG_subprogram "a" + .asciz "a" + .quad _a + .long 0x8 ; four bytes past the end of _a + .byte 1 + .byte 3 ; DW_TAG_lexical_block in "a" + .quad _a + .long 0x8 ; reaches as far as its parent + .byte 0 ; end of "a"'s children + .byte 2 ; DW_TAG_subprogram "b" + .asciz "b" + .quad _b + .long 0x4 + .byte 1 + .byte 0 ; end of "b"'s children + .byte 0 ; end of the compile unit's children +Lcu_end: + .subsections_via_symbols diff --git a/llvm/test/tools/dsymutil/subprogram-high-pc-past-symbol.test b/llvm/test/tools/dsymutil/subprogram-high-pc-past-symbol.test new file mode 100644 index 0000000000000..c58f75b0f9159 --- /dev/null +++ b/llvm/test/tools/dsymutil/subprogram-high-pc-past-symbol.test @@ -0,0 +1,57 @@ +# REQUIRES: aarch64-registered-target + +# A DW_AT_high_pc reaching past the code the linker kept for a function must be +# cut short at the function the linker placed next, otherwise the two overlap in +# the output and verification fails with "DIEs have overlapping address ranges". +# A scope nested in the function inherits the overrun, so it has to be cut short +# with its parent to stay inside it. + +# RUN: llvm-mc -triple arm64-apple-darwin -filetype=obj \ +# RUN: %p/Inputs/subprogram-high-pc-past-symbol.s -o %t.o +# RUN: echo '---' > %t.map +# RUN: echo "triple: 'arm64-apple-darwin'" >> %t.map +# RUN: echo 'objects:' >> %t.map +# RUN: echo " - filename: '%/t.o'" >> %t.map +# RUN: echo ' symbols:' >> %t.map +# RUN: echo ' - { sym: _a, objAddr: 0x0, binAddr: 0x1000, size: 0x4 }' >> %t.map +# RUN: echo ' - { sym: _b, objAddr: 0x8, binAddr: 0x1004, size: 0x4 }' >> %t.map + +# _inside starts within the code the linker kept for _a, so it is nested rather +# than adjacent and must not cut _a short. + +# RUN: echo ' - { sym: _inside, binAddr: 0x1002, size: 0x2 }' >> %t.map +# RUN: echo '...' >> %t.map + +# RUN: dsymutil --linker classic -y %t.map -f -o %t-classic.out +# RUN: llvm-dwarfdump -a %t-classic.out | FileCheck %s +# RUN: llvm-dwarfdump --verify %t-classic.out | FileCheck %s --check-prefix=VERIFY + +# RUN: dsymutil --linker parallel -y %t.map -f -o %t-parallel.out +# RUN: llvm-dwarfdump -a %t-parallel.out | FileCheck %s +# RUN: llvm-dwarfdump --verify %t-parallel.out | FileCheck %s --check-prefix=VERIFY + +# Same again with DW_AT_high_pc as an address rather than a length. + +# RUN: llvm-mc -triple arm64-apple-darwin -filetype=obj \ +# RUN: %p/Inputs/subprogram-high-pc-past-symbol-dwarf2.s -o %t.o +# RUN: dsymutil --linker classic -y %t.map -f -o %t-classic2.out +# RUN: llvm-dwarfdump -a %t-classic2.out | FileCheck %s +# RUN: llvm-dwarfdump --verify %t-classic2.out | FileCheck %s --check-prefix=VERIFY + +# RUN: dsymutil --linker parallel -y %t.map -f -o %t-parallel2.out +# RUN: llvm-dwarfdump -a %t-parallel2.out | FileCheck %s +# RUN: llvm-dwarfdump --verify %t-parallel2.out | FileCheck %s --check-prefix=VERIFY + +# CHECK: DW_TAG_subprogram +# CHECK: DW_AT_name{{.*}}"a" +# CHECK-NEXT: DW_AT_low_pc{{.*}}(0x0000000000001000) +# CHECK-NEXT: DW_AT_high_pc{{.*}}(0x0000000000001004) +# CHECK: DW_TAG_lexical_block +# CHECK-NEXT: DW_AT_low_pc{{.*}}(0x0000000000001000) +# CHECK-NEXT: DW_AT_high_pc{{.*}}(0x0000000000001004) +# CHECK: DW_TAG_subprogram +# CHECK: DW_AT_name{{.*}}"b" +# CHECK-NEXT: DW_AT_low_pc{{.*}}(0x0000000000001004) +# CHECK-NEXT: DW_AT_high_pc{{.*}}(0x0000000000001008) + +# VERIFY: No errors. diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.h b/llvm/tools/dsymutil/DwarfLinkerForBinary.h index 446860abe150c..5b422e065a430 100644 --- a/llvm/tools/dsymutil/DwarfLinkerForBinary.h +++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.h @@ -128,6 +128,9 @@ class DwarfLinkerForBinary { /// Address ranges for symbols with sizes (used for assembly file support). RangesTy AddressRanges; + /// Sorted linked start addresses of the symbols with a known size. + std::vector<uint64_t> LinkedSymbolStarts; + /// Returns list of valid relocations from \p Relocs, /// between \p StartOffset and \p NextOffset. /// @@ -172,15 +175,22 @@ class DwarfLinkerForBinary { } else { findValidRelocsInDebugSections(Obj, DMO); } - // Populate address ranges from debug map symbols that have sizes. - // This is used for assembly files where labels may not have high_pc. + // A sizeless symbol has no known extent, so it can bound neither a range + // of its own nor a neighbour's. The ranges stand in for the high_pc that + // assembly files lack. for (const auto &Entry : DMO.symbols()) { const auto &Mapping = Entry.getValue(); - if (Mapping.Size && Mapping.ObjectAddress) + if (!Mapping.Size) + continue; + LinkedSymbolStarts.push_back(Mapping.BinaryAddress); + if (Mapping.ObjectAddress) AddressRanges.insert( {*Mapping.ObjectAddress, *Mapping.ObjectAddress + Mapping.Size}, int64_t(Mapping.BinaryAddress) - *Mapping.ObjectAddress); } + llvm::sort(LinkedSymbolStarts); + LinkedSymbolStarts.erase(llvm::unique(LinkedSymbolStarts), + LinkedSymbolStarts.end()); } ~AddressManager() override { clear(); } @@ -241,14 +251,23 @@ class DwarfLinkerForBinary { ValidDebugInfoRelocs.clear(); ValidDebugAddrRelocs.clear(); AddressRanges.clear(); + LinkedSymbolStarts.clear(); } - std::optional<AssemblyRange> - getAssemblyRangeForAddress(uint64_t Addr) override { + std::optional<SymbolRange> + getSymbolRangeForAddress(uint64_t Addr) override { if (auto Range = AddressRanges.getRangeThatContains(Addr)) - return AssemblyRange(Range->Range.start(), Range->Range.end()); + return SymbolRange(Range->Range.start(), Range->Range.end()); return std::nullopt; } + + std::optional<uint64_t> + getNextLinkedSymbolStart(uint64_t LinkedAddr) override { + auto It = llvm::lower_bound(LinkedSymbolStarts, LinkedAddr); + if (It == LinkedSymbolStarts.end()) + return std::nullopt; + return *It; + } }; private: _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
