https://github.com/satyajanga updated https://github.com/llvm/llvm-project/pull/218030
>From a7fcef5c5648a1b3ffeeb4be84195ed976d2ce04 Mon Sep 17 00:00:00 2001 From: satya janga <[email protected]> Date: Fri, 21 Aug 2026 13:55:59 -0700 Subject: [PATCH] [lldb] Prefer readers with detailed debug information Add a Symbols ability for object-file symbol data and make SymbolFileSymtab advertise it instead of claiming full Functions or GlobalVariables. Order the ability bits so the existing numeric comparison prefers richer debug information. In particular, CompileUnits plus LineTables outranks Symbols plus CompileUnits. Add focused coverage for plugin selection and SymbolFileSymtab abilities. --- lldb/include/lldb/Symbol/SymbolFile.h | 24 +++--- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 7 +- .../Plugins/SymbolFile/PDB/SymbolFilePDB.cpp | 18 ++++- .../Plugins/SymbolFile/PDB/SymbolFilePDB.h | 4 + .../SymbolFile/Symtab/SymbolFileSymtab.cpp | 13 ++-- lldb/unittests/Symbol/LineTableTest.cpp | 77 ++++++++++++++++--- lldb/unittests/Symbol/SymtabTest.cpp | 5 ++ .../SymbolFile/PDB/SymbolFilePDBTests.cpp | 73 ++++++++++++++++++ 8 files changed, 190 insertions(+), 31 deletions(-) diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h index ae6504c016d7b..ab3f303d4c0af 100644 --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/lldb/include/lldb/Symbol/SymbolFile.h @@ -64,16 +64,22 @@ class SymbolFile : public PluginInterface { // Each symbol file can claim to support one or more symbol file abilities. // These get returned from SymbolFile::GetAbilities(). These help us to // determine which plug-in will be best to load the debug information found - // in files. + // in files. The values are ordered so that a simple numeric comparison + // prefers detailed debug information over data read directly from an object + // file's symbol table. enum Abilities { - CompileUnits = (1u << 0), - LineTables = (1u << 1), - Functions = (1u << 2), - Blocks = (1u << 3), - GlobalVariables = (1u << 4), - LocalVariables = (1u << 5), - VariableTypes = (1u << 6), - kAllAbilities = ((1u << 7) - 1u) + Symbols = (1u << 0), + CompileUnits = (1u << 1), + LineTables = (1u << 2), + Functions = (1u << 3), + Blocks = (1u << 4), + GlobalVariables = (1u << 5), + LocalVariables = (1u << 6), + VariableTypes = (1u << 7), + // All detailed debug-information abilities. Symbols is excluded because + // it describes information from the object file's symbol table. + kAllAbilities = CompileUnits | LineTables | Functions | Blocks | + GlobalVariables | LocalVariables | VariableTypes }; static SymbolFile *FindPlugin(lldb::ObjectFileSP objfile_sp); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 81cd4444161f7..7c41913de03c2 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -684,12 +684,13 @@ uint32_t SymbolFileDWARF::CalculateAbilities() { return 0; } - if (debug_abbrev_file_size > 0 && debug_info_file_size > 0) + if (debug_abbrev_file_size > 0 && debug_info_file_size > 0) { abilities |= CompileUnits | Functions | Blocks | GlobalVariables | LocalVariables | VariableTypes; - if (debug_line_file_size > 0) - abilities |= LineTables; + if (debug_line_file_size > 0) + abilities |= LineTables; + } } return abilities; } diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp index 37c2107219ab0..b6cc71a9e13e7 100644 --- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -233,7 +233,6 @@ SymbolFilePDB::SymbolFilePDB(lldb::ObjectFileSP objfile_sp) SymbolFilePDB::~SymbolFilePDB() = default; uint32_t SymbolFilePDB::CalculateAbilities() { - uint32_t abilities = 0; if (!m_objfile_sp) return 0; @@ -266,7 +265,15 @@ uint32_t SymbolFilePDB::CalculateAbilities() { auto enum_tables_up = m_session_up->getEnumTables(); if (!enum_tables_up) return 0; - while (auto table_up = enum_tables_up->getNext()) { + + return CalculateAbilitiesFromPDBTables(*enum_tables_up); +} + +uint32_t +SymbolFilePDB::CalculateAbilitiesFromPDBTables(IPDBEnumTables &tables) { + uint32_t abilities = 0; + bool has_line_tables = false; + while (auto table_up = tables.getNext()) { if (table_up->getItemCount() == 0) continue; auto type = table_up->getTableType(); @@ -278,12 +285,17 @@ uint32_t SymbolFilePDB::CalculateAbilities() { LocalVariables | VariableTypes); break; case PDB_TableType::LineNumbers: - abilities |= LineTables; + has_line_tables = true; break; default: break; } } + + // A line table is usable only when there are compile units to attach it to. + if (has_line_tables && (abilities & CompileUnits)) + abilities |= LineTables; + return abilities; } diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h index ccbf02db1159f..3f854249272ee 100644 --- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h +++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h @@ -162,6 +162,10 @@ class SymbolFilePDB : public lldb_private::SymbolFileCommon { void DumpClangAST(lldb_private::Stream &s, llvm::StringRef filter, bool show_color) override; +protected: + static uint32_t + CalculateAbilitiesFromPDBTables(llvm::pdb::IPDBEnumTables &tables); + private: struct SecContribInfo { uint32_t Offset; diff --git a/lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp b/lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp index 9c298374101fa..57fe9090aa694 100644 --- a/lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp +++ b/lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp @@ -60,9 +60,13 @@ uint32_t SymbolFileSymtab::CalculateAbilities() { if (m_objfile_sp) { const Symtab *symtab = m_objfile_sp->GetSymtab(); if (symtab) { - // The snippet of code below will get the indexes the module symbol table - // entries that are code, data, or function related (debug info), sort - // them by value (address) and dump the sorted symbols. + // Get the indexes of source, code, data, and function-related entries in + // the module symbol table. Only source-file entries provide a genuine + // debug-info ability. Code and data entries remain available as symbols + // but are not equivalent to debug-info functions or global variables. + if (symtab->GetNumSymbols() > 0) + abilities |= Symbols; + if (symtab->AppendSymbolIndexesWithType(eSymbolTypeSourceFile, m_source_indexes)) { abilities |= CompileUnits; @@ -72,20 +76,17 @@ uint32_t SymbolFileSymtab::CalculateAbilities() { eSymbolTypeCode, Symtab::eDebugYes, Symtab::eVisibilityAny, m_func_indexes)) { symtab->SortSymbolIndexesByValue(m_func_indexes, true); - abilities |= Functions; } if (symtab->AppendSymbolIndexesWithType(eSymbolTypeCode, Symtab::eDebugNo, Symtab::eVisibilityAny, m_code_indexes)) { symtab->SortSymbolIndexesByValue(m_code_indexes, true); - abilities |= Functions; } if (symtab->AppendSymbolIndexesWithType(eSymbolTypeData, m_data_indexes)) { symtab->SortSymbolIndexesByValue(m_data_indexes, true); - abilities |= GlobalVariables; } lldb_private::Symtab::IndexCollection objc_class_indexes; diff --git a/lldb/unittests/Symbol/LineTableTest.cpp b/lldb/unittests/Symbol/LineTableTest.cpp index 80f2f219d0e81..291b1bb356813 100644 --- a/lldb/unittests/Symbol/LineTableTest.cpp +++ b/lldb/unittests/Symbol/LineTableTest.cpp @@ -35,10 +35,22 @@ class FakeSymbolFile : public SymbolFile { /// \} static void Initialize() { - PluginManager::RegisterPlugin("FakeSymbolFile", "", CreateInstance, - DebuggerInitialize); + PluginManager::RegisterPlugin("LineTableFakeSymbolFile", "", + CreateLineTableInstance, DebuggerInitialize); + PluginManager::RegisterPlugin("SymbolOnlyFakeSymbolFile", "", + CreateSymbolOnlyInstance, DebuggerInitialize); + } + static void Terminate() { + PluginManager::UnregisterPlugin(CreateSymbolOnlyInstance); + PluginManager::UnregisterPlugin(CreateLineTableInstance); + } + + static void SetLineTableAbilities(uint32_t abilities) { + g_line_table_abilities = abilities; + } + static void SetSymbolAbilities(uint32_t abilities) { + g_symbol_abilities = abilities; } - static void Terminate() { PluginManager::UnregisterPlugin(CreateInstance); } void InjectCompileUnit(std::unique_ptr<CompileUnit> cu_up) { m_cu_sp = std::move(cu_up); @@ -48,14 +60,19 @@ class FakeSymbolFile : public SymbolFile { /// LLVM RTTI support. static char ID; - static SymbolFile *CreateInstance(ObjectFileSP objfile_sp) { - return new FakeSymbolFile(std::move(objfile_sp)); + static SymbolFile *CreateLineTableInstance(ObjectFileSP objfile_sp) { + return new FakeSymbolFile(std::move(objfile_sp), "LineTableFakeSymbolFile", + g_line_table_abilities); + } + static SymbolFile *CreateSymbolOnlyInstance(ObjectFileSP objfile_sp) { + return new FakeSymbolFile(std::move(objfile_sp), "SymbolOnlyFakeSymbolFile", + g_symbol_abilities); } static void DebuggerInitialize(Debugger &) {} - StringRef GetPluginName() override { return "FakeSymbolFile"; } - uint32_t GetAbilities() override { return UINT32_MAX; } - uint32_t CalculateAbilities() override { return UINT32_MAX; } + StringRef GetPluginName() override { return m_plugin_name; } + uint32_t GetAbilities() override { return m_abilities; } + uint32_t CalculateAbilities() override { return m_abilities; } uint32_t GetNumCompileUnits() override { return 1; } CompUnitSP GetCompileUnitAtIndex(uint32_t) override { return m_cu_sp; } Symtab *GetSymtab(bool can_create = true) override { return nullptr; } @@ -109,11 +126,17 @@ class FakeSymbolFile : public SymbolFile { } TypeSP CopyType(const TypeSP &) override { return nullptr; } - FakeSymbolFile(ObjectFileSP objfile_sp) - : m_objfile_sp(std::move(objfile_sp)) {} + FakeSymbolFile(ObjectFileSP objfile_sp, StringRef plugin_name, + uint32_t abilities) + : m_objfile_sp(std::move(objfile_sp)), m_plugin_name(plugin_name), + m_abilities(abilities) {} ObjectFileSP m_objfile_sp; CompUnitSP m_cu_sp; + StringRef m_plugin_name; + uint32_t m_abilities; + inline static uint32_t g_line_table_abilities = CompileUnits | LineTables; + inline static uint32_t g_symbol_abilities = Symbols; }; struct FakeModuleFixture { @@ -124,6 +147,14 @@ struct FakeModuleFixture { }; class LineTableTest : public testing::Test { +protected: + void SetUp() override { + FakeSymbolFile::SetLineTableAbilities(SymbolFile::CompileUnits | + SymbolFile::LineTables); + FakeSymbolFile::SetSymbolAbilities(SymbolFile::Symbols); + } + +private: SubsystemRAII<ObjectFileELF, FakeSymbolFile> subsystems; }; @@ -190,6 +221,32 @@ CreateFakeModule(std::vector<LineTable::Sequence> line_sequences) { std::move(text_sp), line_table}; } +TEST_F(LineTableTest, FindPluginPrefersLineTablesWithCompileUnits) { + llvm::Expected<FakeModuleFixture> fixture = CreateFakeModule({}); + ASSERT_THAT_EXPECTED(fixture, llvm::Succeeded()); + + SymbolFile *symbol_file = fixture->module_sp->GetSymbolFile(); + ASSERT_NE(symbol_file, nullptr); + EXPECT_EQ(symbol_file->GetPluginName(), "LineTableFakeSymbolFile"); + EXPECT_EQ( + symbol_file->GetAbilities(), + static_cast<uint32_t>(SymbolFile::CompileUnits | SymbolFile::LineTables)); +} + +TEST_F(LineTableTest, FindPluginPrefersLineTablesOverCompileUnits) { + FakeSymbolFile::SetSymbolAbilities(SymbolFile::Symbols | + SymbolFile::CompileUnits); + llvm::Expected<FakeModuleFixture> fixture = CreateFakeModule({}); + ASSERT_THAT_EXPECTED(fixture, llvm::Succeeded()); + + SymbolFile *symbol_file = fixture->module_sp->GetSymbolFile(); + ASSERT_NE(symbol_file, nullptr); + EXPECT_EQ(symbol_file->GetPluginName(), "LineTableFakeSymbolFile"); + EXPECT_EQ( + symbol_file->GetAbilities(), + static_cast<uint32_t>(SymbolFile::CompileUnits | SymbolFile::LineTables)); +} + TEST_F(LineTableTest, lower_bound) { LineSequenceBuilder builder; builder.Entry(0); diff --git a/lldb/unittests/Symbol/SymtabTest.cpp b/lldb/unittests/Symbol/SymtabTest.cpp index fda92e4044919..dba75c0f6e1f6 100644 --- a/lldb/unittests/Symbol/SymtabTest.cpp +++ b/lldb/unittests/Symbol/SymtabTest.cpp @@ -739,6 +739,11 @@ TEST_F(SymtabTest, TestSymbolFileCreatedOnDemand) { // And we should be able to get it again once it has been created. Symtab *cached_module_symtab = module_sp->GetSymtab(/*can_create=*/false); ASSERT_EQ(module_symtab, cached_module_symtab); + + SymbolFile *symbol_file = module_sp->GetSymbolFile(); + ASSERT_NE(symbol_file, nullptr); + EXPECT_EQ(symbol_file->GetAbilities(), + static_cast<uint32_t>(SymbolFile::Symbols)); } TEST_F(SymtabTest, TestSymbolTableCreatedOnDemand) { diff --git a/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp b/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp index bb1e0d8cd1fea..35c31a7c32421 100644 --- a/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp +++ b/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp @@ -9,6 +9,7 @@ #include "gtest/gtest.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/DebugInfo/PDB/IPDBTable.h" #include "llvm/DebugInfo/PDB/PDBSymbolData.h" #include "llvm/DebugInfo/PDB/PDBSymbolExe.h" #include "llvm/Support/FileSystem.h" @@ -39,9 +40,81 @@ #endif #include <algorithm> +#include <initializer_list> +#include <utility> +#include <vector> using namespace lldb_private; +namespace { + +class TestableSymbolFilePDB : public SymbolFilePDB { +public: + using SymbolFilePDB::CalculateAbilitiesFromPDBTables; +}; + +class FakePDBTable : public llvm::pdb::IPDBTable { +public: + FakePDBTable(llvm::pdb::PDB_TableType type, uint32_t item_count) + : m_type(type), m_item_count(item_count) {} + + std::string getName() const override { return {}; } + uint32_t getItemCount() const override { return m_item_count; } + llvm::pdb::PDB_TableType getTableType() const override { return m_type; } + +private: + llvm::pdb::PDB_TableType m_type; + uint32_t m_item_count; +}; + +class FakePDBEnumTables : public llvm::pdb::IPDBEnumTables { +public: + using Table = std::pair<llvm::pdb::PDB_TableType, uint32_t>; + + FakePDBEnumTables(std::initializer_list<Table> tables) : m_tables(tables) {} + + uint32_t getChildCount() const override { return m_tables.size(); } + + std::unique_ptr<llvm::pdb::IPDBTable> + getChildAtIndex(uint32_t index) const override { + if (index >= m_tables.size()) + return nullptr; + return std::make_unique<FakePDBTable>(m_tables[index].first, + m_tables[index].second); + } + + std::unique_ptr<llvm::pdb::IPDBTable> getNext() override { + if (m_next_index >= m_tables.size()) + return nullptr; + return getChildAtIndex(m_next_index++); + } + + void reset() override { m_next_index = 0; } + +private: + std::vector<Table> m_tables; + uint32_t m_next_index = 0; +}; + +} // namespace + +TEST(SymbolFilePDBAbilitiesTest, LineTablesRequireCompileUnits) { + FakePDBEnumTables line_tables_without_symbols( + {{llvm::pdb::PDB_TableType::Symbols, 0}, + {llvm::pdb::PDB_TableType::LineNumbers, 1}}); + EXPECT_EQ(0u, TestableSymbolFilePDB::CalculateAbilitiesFromPDBTables( + line_tables_without_symbols)); + + // Put line tables first to verify the result does not depend on DIA's table + // enumeration order. + FakePDBEnumTables line_tables_and_symbols( + {{llvm::pdb::PDB_TableType::LineNumbers, 1}, + {llvm::pdb::PDB_TableType::Symbols, 1}}); + EXPECT_EQ(SymbolFile::kAllAbilities, + TestableSymbolFilePDB::CalculateAbilitiesFromPDBTables( + line_tables_and_symbols)); +} + class SymbolFilePDBTests : public testing::Test { public: void SetUp() override { _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
