Stefan =?utf-8?q?Gränitz?= <[email protected]>, Stefan =?utf-8?q?Gränitz?= <[email protected]> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/[email protected]>
llvmbot wrote: <!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Stefan Gränitz (weliveindetail) <details> <summary>Changes</summary> Minimal infrastructure for a the SymbolLocator plugin that fetches debug info from Microsoft SymStore repositories. This can work cross-platform and for various debug info formats in principle, but the current plan is focussed on PE/COFF on Windows with debug info in PDB files. Once we have a stable first version, we'd like to add features like download, environment variables, caching and progress feedback for users. SymbolVendorPECOFF was tailored towards DWARF debug info so far. I added code to load the PDB path from the executable (it only checked `gnu_debuglink` so far) and not bail out if DWARF sections are missing, so that in the PDB case we still call `AddSymbolFileRepresentation()` in the very end of `CreateInstance()`. The API test in this patch mocks the directory layout from SymStore, so it doesn't depend on `SymStore.exe` from the Windows SDK. It runs on all platforms that link debug info in a PDB file, which is still just Windows, but it could be cross-platform in principle. --- Full diff: https://github.com/llvm/llvm-project/pull/183302.diff 11 Files Affected: - (modified) lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp (+15) - (modified) lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h (+2) - (modified) lldb/source/Plugins/SymbolLocator/CMakeLists.txt (+1) - (added) lldb/source/Plugins/SymbolLocator/SymStore/CMakeLists.txt (+20) - (added) lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp (+156) - (added) lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.h (+47) - (added) lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStoreProperties.td (+7) - (modified) lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp (+25-23) - (added) lldb/test/API/symstore/PDB/Makefile (+2) - (added) lldb/test/API/symstore/PDB/TestSymStoreLocalPDB.py (+112) - (added) lldb/test/API/symstore/PDB/main.c (+5) ``````````diff diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp index 3a17b4c46a788..2e169bb6e8a53 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp @@ -1108,6 +1108,21 @@ std::optional<FileSpec> ObjectFilePECOFF::GetDebugLink() { return std::nullopt; } +std::optional<FileSpec> ObjectFilePECOFF::GetPDBPath() { + llvm::StringRef pdb_file; + const llvm::codeview::DebugInfo *pdb_info = nullptr; + if (llvm::Error Err = m_binary->getDebugPDBInfo(pdb_info, pdb_file)) { + // Ignore corrupt DebugInfo sections + llvm::consumeError(std::move(Err)); + return std::nullopt; + } + if (pdb_file.empty()) { + // No DebugInfo section + return std::nullopt; + } + return FileSpec(pdb_file); +} + uint32_t ObjectFilePECOFF::ParseDependentModules() { ModuleSP module_sp(GetModule()); if (!module_sp) diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h index 8002e70e604bb..30bd672dc68f8 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h @@ -130,6 +130,8 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile { /// contains it. std::optional<lldb_private::FileSpec> GetDebugLink(); + std::optional<lldb_private::FileSpec> GetPDBPath(); + uint32_t GetDependentModules(lldb_private::FileSpecList &files) override; lldb_private::Address GetEntryPointAddress() override; diff --git a/lldb/source/Plugins/SymbolLocator/CMakeLists.txt b/lldb/source/Plugins/SymbolLocator/CMakeLists.txt index 3b466f71dca58..9b9ec470b86a9 100644 --- a/lldb/source/Plugins/SymbolLocator/CMakeLists.txt +++ b/lldb/source/Plugins/SymbolLocator/CMakeLists.txt @@ -6,6 +6,7 @@ set_property(DIRECTORY PROPERTY LLDB_PLUGIN_KIND SymbolLocator) # prevents an unstripped binary from being requested from the Debuginfod # provider. add_subdirectory(Debuginfod) +add_subdirectory(SymStore) add_subdirectory(Default) if (CMAKE_SYSTEM_NAME MATCHES "Darwin") add_subdirectory(DebugSymbols) diff --git a/lldb/source/Plugins/SymbolLocator/SymStore/CMakeLists.txt b/lldb/source/Plugins/SymbolLocator/SymStore/CMakeLists.txt new file mode 100644 index 0000000000000..b0da27f26c6a8 --- /dev/null +++ b/lldb/source/Plugins/SymbolLocator/SymStore/CMakeLists.txt @@ -0,0 +1,20 @@ +lldb_tablegen(SymbolLocatorSymStoreProperties.inc -gen-lldb-property-defs + SOURCE SymbolLocatorSymStoreProperties.td + TARGET LLDBPluginSymbolLocatorSymStorePropertiesGen) + +lldb_tablegen(SymbolLocatorSymStorePropertiesEnum.inc -gen-lldb-property-enum-defs + SOURCE SymbolLocatorSymStoreProperties.td + TARGET LLDBPluginSymbolLocatorSymStorePropertiesEnumGen) + +add_lldb_library(lldbPluginSymbolLocatorSymStore PLUGIN + SymbolLocatorSymStore.cpp + + LINK_LIBS + lldbCore + lldbHost + lldbSymbol + ) + +add_dependencies(lldbPluginSymbolLocatorSymStore + LLDBPluginSymbolLocatorSymStorePropertiesGen + LLDBPluginSymbolLocatorSymStorePropertiesEnumGen) diff --git a/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp b/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp new file mode 100644 index 0000000000000..0341249f0da0e --- /dev/null +++ b/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp @@ -0,0 +1,156 @@ +//===-- SymbolLocatorSymStore.cpp ---------------------------------------===// +// +// 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 "SymbolLocatorSymStore.h" + +#include "lldb/Core/ModuleList.h" +#include "lldb/Core/PluginManager.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Interpreter/OptionValueString.h" +#include "lldb/Utility/Args.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/Utility/Log.h" +#include "lldb/Utility/UUID.h" + +#include "llvm/ADT/StringExtras.h" +#include "llvm/Support/Endian.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/Path.h" + +using namespace lldb; +using namespace lldb_private; + +LLDB_PLUGIN_DEFINE(SymbolLocatorSymStore) + +namespace { + +#define LLDB_PROPERTIES_symbollocatorsymstore +#include "SymbolLocatorSymStoreProperties.inc" + +enum { +#define LLDB_PROPERTIES_symbollocatorsymstore +#include "SymbolLocatorSymStorePropertiesEnum.inc" +}; + +class PluginProperties : public Properties { +public: + static llvm::StringRef GetSettingName() { + return SymbolLocatorSymStore::GetPluginNameStatic(); + } + + PluginProperties() { + m_collection_sp = std::make_shared<OptionValueProperties>(GetSettingName()); + m_collection_sp->Initialize(g_symbollocatorsymstore_properties_def); + } + + Args GetURLs() const { + Args urls; + m_collection_sp->GetPropertyAtIndexAsArgs(ePropertySymStoreURLs, urls); + return urls; + } +}; + +} // namespace + +static PluginProperties &GetGlobalPluginProperties() { + static PluginProperties g_settings; + return g_settings; +} + +SymbolLocatorSymStore::SymbolLocatorSymStore() : SymbolLocator() {} + +void SymbolLocatorSymStore::Initialize() { + // First version can only locate PDB in local SymStore (no download yet) + PluginManager::RegisterPlugin( + GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance, + nullptr, LocateExecutableSymbolFile, nullptr, nullptr, + SymbolLocatorSymStore::DebuggerInitialize); +} + +void SymbolLocatorSymStore::DebuggerInitialize(Debugger &debugger) { + if (!PluginManager::GetSettingForSymbolLocatorPlugin( + debugger, PluginProperties::GetSettingName())) { + constexpr bool is_global_setting = true; + PluginManager::CreateSettingForSymbolLocatorPlugin( + debugger, GetGlobalPluginProperties().GetValueProperties(), + "Properties for the SymStore Symbol Locator plug-in.", + is_global_setting); + } +} + +void SymbolLocatorSymStore::Terminate() { + PluginManager::UnregisterPlugin(CreateInstance); +} + +llvm::StringRef SymbolLocatorSymStore::GetPluginDescriptionStatic() { + return "Symbol locator for PDB in SymStore SymStore"; +} + +SymbolLocator *SymbolLocatorSymStore::CreateInstance() { + return new SymbolLocatorSymStore(); +} + +// LLDB stores PDB identity as a 20-byte UUID composed of 16-byte GUID and +// 4-byte age: +// 12345678-1234-5678-9ABC-DEF012345678-00000001 +// +// SymStore key is a string with no separators and age as decimal: +// 12345678123456789ABCDEF0123456781 +// +static std::string formatSymStoreKey(const UUID &uuid) { + llvm::ArrayRef<uint8_t> bytes = uuid.GetBytes(); + uint32_t age = llvm::support::endian::read32be(bytes.data() + 16); + constexpr bool LowerCase = false; + return llvm::toHex(bytes.slice(0, 16), LowerCase) + std::to_string(age); +} + +std::optional<FileSpec> SymbolLocatorSymStore::LocateExecutableSymbolFile( + const ModuleSpec &module_spec, const FileSpecList &default_search_paths) { + const UUID &uuid = module_spec.GetUUID(); + if (!uuid.IsValid() || + !ModuleList::GetGlobalModuleListProperties().GetEnableExternalLookup()) + return {}; + + Log *log = GetLog(LLDBLog::Symbols); + std::string pdb_name = + module_spec.GetSymbolFileSpec().GetFilename().GetStringRef().str(); + if (pdb_name.empty()) { + LLDB_LOGV(log, "Failed to resolve symbol PDB module: PDB name empty"); + return {}; + } + + LLDB_LOGV(log, "LocateExecutableSymbolFile {0} with UUID {1}", pdb_name, + uuid.GetAsString()); + if (uuid.GetBytes().size() != 20) { + LLDB_LOGV(log, " Failed to resolve symbol PDB module: UUID invalid"); + return {}; + } + + // FIXME: We need this for the test executable, because it is loaded as DWARF + if (!llvm::StringRef(pdb_name).ends_with(".pdb")) { + auto last_dot = pdb_name.find_last_of('.'); + if (last_dot != llvm::StringRef::npos) { + pdb_name = pdb_name.substr(0, last_dot); + } + pdb_name += ".pdb"; + } + + std::string key = formatSymStoreKey(uuid); + Args sym_store_urls = GetGlobalPluginProperties().GetURLs(); + for (const Args::ArgEntry &url : sym_store_urls) { + llvm::SmallString<256> path; + llvm::sys::path::append(path, url.ref(), pdb_name, key, pdb_name); + FileSpec spec(path); + if (FileSystem::Instance().Exists(spec)) { + LLDB_LOGV(log, " Found {0} in SymStore {1}", pdb_name, url.ref()); + return spec; + } + } + + return {}; +} diff --git a/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.h b/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.h new file mode 100644 index 0000000000000..551b774dc9f3a --- /dev/null +++ b/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.h @@ -0,0 +1,47 @@ +//===-- SymbolLocatorSymStore.h ---------------------------------*- C++ -*-===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_SOURCE_PLUGINS_SYMBOLLOCATOR_SYMSTORE_SYMBOLLOCATORSYMSTORE_H +#define LLDB_SOURCE_PLUGINS_SYMBOLLOCATOR_SYMSTORE_SYMBOLLOCATORSYMSTORE_H + +#include "lldb/Core/Debugger.h" +#include "lldb/Symbol/SymbolLocator.h" +#include "lldb/lldb-private.h" + +namespace lldb_private { + +class SymbolLocatorSymStore : public SymbolLocator { +public: + SymbolLocatorSymStore(); + + static void Initialize(); + static void Terminate(); + static void DebuggerInitialize(Debugger &debugger); + + static llvm::StringRef GetPluginNameStatic() { return "symstore"; } + static llvm::StringRef GetPluginDescriptionStatic(); + + static lldb_private::SymbolLocator *CreateInstance(); + + /// PluginInterface protocol. + /// \{ + llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); } + /// \} + + // Locate the symbol file given a module specification. + // + // Locating the file should happen only on the local computer or using the + // current computers global settings. + static std::optional<FileSpec> + LocateExecutableSymbolFile(const ModuleSpec &module_spec, + const FileSpecList &default_search_paths); +}; + +} // namespace lldb_private + +#endif // LLDB_SOURCE_PLUGINS_SYMBOLLOCATOR_SYMSTORE_SYMBOLLOCATORSYMSTORE_H diff --git a/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStoreProperties.td b/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStoreProperties.td new file mode 100644 index 0000000000000..0cd631a80b90b --- /dev/null +++ b/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStoreProperties.td @@ -0,0 +1,7 @@ +include "../../../../include/lldb/Core/PropertiesBase.td" + +let Definition = "symbollocatorsymstore", Path = "plugin.symbol-locator.symstore" in { + def SymStoreURLs : Property<"urls", "Array">, + ElementType<"String">, + Desc<"List of local symstore directories to query for symbols">; +} diff --git a/lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp b/lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp index 20ccfa54a106c..1797e5b7677ee 100644 --- a/lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp +++ b/lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp @@ -71,6 +71,9 @@ SymbolVendorPECOFF::CreateInstance(const lldb::ModuleSP &module_sp, // If the module specified a filespec, use that. FileSpec fspec = module_sp->GetSymbolFileFileSpec(); + // Otherwise, use the PDB path from CodeView. + if (!fspec) + fspec = obj_file->GetPDBPath().value_or(FileSpec()); // Otherwise, try gnu_debuglink, if one exists. if (!fspec) fspec = obj_file->GetDebugLink().value_or(FileSpec()); @@ -101,31 +104,30 @@ SymbolVendorPECOFF::CreateInstance(const lldb::ModuleSP &module_sp, // This objfile is for debugging purposes. dsym_objfile_sp->SetType(ObjectFile::eTypeDebugInfo); - // Get the module unified section list and add our debug sections to - // that. + // For DWARF get the module unified section list and add our debug sections + // to that. SectionList *module_section_list = module_sp->GetSectionList(); SectionList *objfile_section_list = dsym_objfile_sp->GetSectionList(); - if (!objfile_section_list || !module_section_list) - return nullptr; - - static const SectionType g_sections[] = { - eSectionTypeDWARFDebugAbbrev, eSectionTypeDWARFDebugAranges, - eSectionTypeDWARFDebugFrame, eSectionTypeDWARFDebugInfo, - eSectionTypeDWARFDebugLine, eSectionTypeDWARFDebugLoc, - eSectionTypeDWARFDebugLocLists, eSectionTypeDWARFDebugMacInfo, - eSectionTypeDWARFDebugNames, eSectionTypeDWARFDebugPubNames, - eSectionTypeDWARFDebugPubTypes, eSectionTypeDWARFDebugRanges, - eSectionTypeDWARFDebugStr, eSectionTypeDWARFDebugTypes, - }; - for (SectionType section_type : g_sections) { - if (SectionSP section_sp = - objfile_section_list->FindSectionByType(section_type, true)) { - if (SectionSP module_section_sp = - module_section_list->FindSectionByType(section_type, true)) - module_section_list->ReplaceSection(module_section_sp->GetID(), - section_sp); - else - module_section_list->AddSection(section_sp); + if (objfile_section_list && module_section_list) { + static const SectionType g_sections[] = { + eSectionTypeDWARFDebugAbbrev, eSectionTypeDWARFDebugAranges, + eSectionTypeDWARFDebugFrame, eSectionTypeDWARFDebugInfo, + eSectionTypeDWARFDebugLine, eSectionTypeDWARFDebugLoc, + eSectionTypeDWARFDebugLocLists, eSectionTypeDWARFDebugMacInfo, + eSectionTypeDWARFDebugNames, eSectionTypeDWARFDebugPubNames, + eSectionTypeDWARFDebugPubTypes, eSectionTypeDWARFDebugRanges, + eSectionTypeDWARFDebugStr, eSectionTypeDWARFDebugTypes, + }; + for (SectionType section_type : g_sections) { + if (SectionSP section_sp = + objfile_section_list->FindSectionByType(section_type, true)) { + if (SectionSP module_section_sp = + module_section_list->FindSectionByType(section_type, true)) + module_section_list->ReplaceSection(module_section_sp->GetID(), + section_sp); + else + module_section_list->AddSection(section_sp); + } } } diff --git a/lldb/test/API/symstore/PDB/Makefile b/lldb/test/API/symstore/PDB/Makefile new file mode 100644 index 0000000000000..c9319d6e6888a --- /dev/null +++ b/lldb/test/API/symstore/PDB/Makefile @@ -0,0 +1,2 @@ +C_SOURCES := main.c +include Makefile.rules diff --git a/lldb/test/API/symstore/PDB/TestSymStoreLocalPDB.py b/lldb/test/API/symstore/PDB/TestSymStoreLocalPDB.py new file mode 100644 index 0000000000000..370c3000690bf --- /dev/null +++ b/lldb/test/API/symstore/PDB/TestSymStoreLocalPDB.py @@ -0,0 +1,112 @@ +import glob +import os +import shutil +import tempfile + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * + +""" +Test debug symbol acquisition from a local SymStore repository. We populate the +respective file structure in a temporary directory and run LLDB on it. This is +supposed to work cross-platform. The test can run on all platforms that can link +debug info in a PDB file with clang. +""" + +class SymStoreLocalPDBTests(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def try_breakpoint(self, should_have_loc): + target = self.dbg.CreateTarget(self.aout) + self.assertTrue(target and target.IsValid(), "Target is valid") + + bp = target.BreakpointCreateByName("func") + self.assertTrue(bp and bp.IsValid(), "Breakpoint is valid") + self.assertEqual(bp.GetNumLocations(), 1) + + loc = bp.GetLocationAtIndex(0) + self.assertTrue(loc and loc.IsValid(), "Location is valid") + addr = loc.GetAddress() + self.assertTrue(addr and addr.IsValid(), "Loc address is valid") + line_entry = addr.GetLineEntry() + self.assertEqual( + should_have_loc, + line_entry is not None and line_entry.IsValid(), + "Loc line entry validity", + ) + if should_have_loc: + self.assertEqual(line_entry.GetLine(), 2) + self.assertEqual( + line_entry.GetFileSpec().GetFilename(), + self.main_source_file.GetFilename(), + ) + self.dbg.DeleteTarget(target) + + def build_inferior_with_pdb(self): + self.main_source_file = lldb.SBFileSpec("main.c") + self.build() + pdbs = glob.glob(os.path.join(self.getBuildDir(), "*.pdb")) + return len(pdbs) > 0 + + def populate_symstore(self, tmp): + """ + Mock local symstore directory tree and fill in build artifacts: + * tmp/test/<exe> + * tmp/server/<pdb>/<key>/<pdb> + """ + binary_name = "a.out" + pdb_name = "a.pdb" + key = self.symstore_key(binary_name) + if key is None: + self.skipTest("Binary has no valid UUID for PDB") + + # Move exe to isolated directory + test_dir = os.path.join(tmp, "test") + os.makedirs(test_dir) + shutil.move(self.getBuildArtifact(binary_name), test_dir) + self.aout = os.path.join(test_dir, binary_name) + + # Move PDB to SymStore directory + server_dir = os.path.join(tmp, "server") + pdb_key_dir = os.path.join(server_dir, pdb_name, key) + os.makedirs(pdb_key_dir) + shutil.move( + self.getBuildArtifact(pdb_name), + os.path.join(pdb_key_dir, pdb_name), + ) + + return server_dir + + def symstore_key(self, exe): + """Load module UUID like: 12345678-1234-5678-9ABC-DEF012345678-00000001 + and transform to SymStore key: 12345678123456789ABCDEF0123456781""" + try: + spec = lldb.SBModuleSpec() + spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact(exe))) + module = lldb.SBModule(spec) + raw = module.GetUUIDString().replace("-", "").upper() + if len(raw) != 40: + return None + guid_hex = raw[:32] + age = int(raw[32:], 16) + return guid_hex + str(age) + except Exception: + return None + + # TODO: Add a test that fails if we don't set the URL + def test_basic(self): + """Check that breakpoint hits if LLDB fetches PDB from local SymStore""" + if not self.build_inferior_with_pdb(): + self.skipTest("Build did not produce a PDB file") + + tmp_dir = tempfile.mkdtemp() + symstore_dir = self.populate_symstore(tmp_dir) + + self.runCmd( + "settings set plugin.symbol-locator.symstore.urls %s" + % symstore_dir.replace("\\", "/") + ) + + self.try_breakpoint(should_have_loc=True) + shutil.rmtree(tmp_dir) diff --git a/lldb/test/API/symstore/PDB/main.c b/lldb/test/API/symstore/PDB/main.c new file mode 100644 index 0000000000000..a95762e80ea44 --- /dev/null +++ b/lldb/test/API/symstore/PDB/main.c @@ -0,0 +1,5 @@ +int func(int argc, const char *argv[]) { + return (argc + 1) * (argv[argc][0] + 2); +} + +int main(int argc, const char *argv[]) { return func(0, argv); } `````````` </details> https://github.com/llvm/llvm-project/pull/183302 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
