Author: Shubham Sandeep Rastogi Date: 2025-04-21T17:19:54-07:00 New Revision: 08b4c52540727455194b0cf0f6310f391e87c2a5
URL: https://github.com/llvm/llvm-project/commit/08b4c52540727455194b0cf0f6310f391e87c2a5 DIFF: https://github.com/llvm/llvm-project/commit/08b4c52540727455194b0cf0f6310f391e87c2a5.diff LOG: Revert "[lldb] Avoid force loading symbols in statistics collection (#136236)" This reverts commit d5b40c71f6be972f677de5d9886f91866df007b5. This change broke greendragon lldb test: lldb-api :: commands/statistics/basic/TestStats.py And is therefore being reverted. Added: Modified: lldb/include/lldb/Symbol/ObjectFile.h lldb/include/lldb/Symbol/SymbolFile.h lldb/include/lldb/Symbol/SymbolFileOnDemand.h lldb/source/Core/Module.cpp lldb/source/Symbol/ObjectFile.cpp lldb/source/Symbol/SymbolFile.cpp lldb/test/API/commands/statistics/basic/TestStats.py lldb/unittests/Symbol/LineTableTest.cpp lldb/unittests/Symbol/SymtabTest.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Symbol/ObjectFile.h b/lldb/include/lldb/Symbol/ObjectFile.h index 7fca6383fa9f3..cfcca04a76de8 100644 --- a/lldb/include/lldb/Symbol/ObjectFile.h +++ b/lldb/include/lldb/Symbol/ObjectFile.h @@ -319,7 +319,7 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>, /// /// \return /// The symbol table for this object file. - Symtab *GetSymtab(bool can_create = true); + Symtab *GetSymtab(); /// Parse the symbol table into the provides symbol table object. /// diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h index df2f263b18e17..f35d3ee9f22ae 100644 --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/lldb/include/lldb/Symbol/SymbolFile.h @@ -144,7 +144,7 @@ class SymbolFile : public PluginInterface { virtual uint32_t GetNumCompileUnits() = 0; virtual lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx) = 0; - virtual Symtab *GetSymtab(bool can_create = true) = 0; + virtual Symtab *GetSymtab() = 0; virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0; /// Return the Xcode SDK comp_unit was compiled against. @@ -533,7 +533,7 @@ class SymbolFileCommon : public SymbolFile { return m_abilities; } - Symtab *GetSymtab(bool can_create = true) override; + Symtab *GetSymtab() override; ObjectFile *GetObjectFile() override { return m_objfile_sp.get(); } const ObjectFile *GetObjectFile() const override { diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h index 6ed389a48880c..7a366bfabec86 100644 --- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h +++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h @@ -186,9 +186,7 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile { uint32_t GetAbilities() override; - Symtab *GetSymtab(bool can_create = true) override { - return m_sym_file_impl->GetSymtab(can_create); - } + Symtab *GetSymtab() override { return m_sym_file_impl->GetSymtab(); } ObjectFile *GetObjectFile() override { return m_sym_file_impl->GetObjectFile(); diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index 625c14e4a2153..ad7046e596278 100644 --- a/lldb/source/Core/Module.cpp +++ b/lldb/source/Core/Module.cpp @@ -997,7 +997,7 @@ SymbolFile *Module::GetSymbolFile(bool can_create, Stream *feedback_strm) { Symtab *Module::GetSymtab(bool can_create) { if (SymbolFile *symbols = GetSymbolFile(can_create)) - return symbols->GetSymtab(can_create); + return symbols->GetSymtab(); return nullptr; } diff --git a/lldb/source/Symbol/ObjectFile.cpp b/lldb/source/Symbol/ObjectFile.cpp index afd0d298e675e..2f2c59d6af620 100644 --- a/lldb/source/Symbol/ObjectFile.cpp +++ b/lldb/source/Symbol/ObjectFile.cpp @@ -734,9 +734,10 @@ void llvm::format_provider<ObjectFile::Strata>::format( } } -Symtab *ObjectFile::GetSymtab(bool can_create) { + +Symtab *ObjectFile::GetSymtab() { ModuleSP module_sp(GetModule()); - if (module_sp && can_create) { + if (module_sp) { // We can't take the module lock in ObjectFile::GetSymtab() or we can // deadlock in DWARF indexing when any file asks for the symbol table from // an object file. This currently happens in the preloading of symbols in diff --git a/lldb/source/Symbol/SymbolFile.cpp b/lldb/source/Symbol/SymbolFile.cpp index 870d778dca740..94e32b55572dd 100644 --- a/lldb/source/Symbol/SymbolFile.cpp +++ b/lldb/source/Symbol/SymbolFile.cpp @@ -152,10 +152,10 @@ void SymbolFile::AssertModuleLock() { SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default; -Symtab *SymbolFileCommon::GetSymtab(bool can_create) { +Symtab *SymbolFileCommon::GetSymtab() { std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); // Fetch the symtab from the main object file. - auto *symtab = GetMainObjectFile()->GetSymtab(can_create); + auto *symtab = GetMainObjectFile()->GetSymtab(); if (m_symtab != symtab) { m_symtab = symtab; diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py index e2c539bfaeddb..0265a0d7c9948 100644 --- a/lldb/test/API/commands/statistics/basic/TestStats.py +++ b/lldb/test/API/commands/statistics/basic/TestStats.py @@ -209,29 +209,6 @@ def test_default_no_run(self): ) self.assertGreater(module_stats["symbolsLoaded"], 0) - def test_default_no_run_no_preload_symbols(self): - """Test "statistics dump" without running the target and without - preloading symbols. - - Checks that symbol count are zero. - """ - # Make sure symbols will not be preloaded. - self.runCmd("settings set target.preload-symbols false") - - # Build and load the target - self.build() - target = self.createTestTarget() - - # Get statistics - debug_stats = self.get_stats() - - # No symbols should be loaded - self.assertEqual(debug_stats["totalSymbolsLoaded"], 0) - - # No symbols should be loaded in each module - for module_stats in debug_stats["modules"]: - self.assertEqual(module_stats["symbolsLoaded"], 0) - def test_default_with_run(self): """Test "statistics dump" when running the target to a breakpoint. diff --git a/lldb/unittests/Symbol/LineTableTest.cpp b/lldb/unittests/Symbol/LineTableTest.cpp index eadab40a37fac..f67ac46c738e7 100644 --- a/lldb/unittests/Symbol/LineTableTest.cpp +++ b/lldb/unittests/Symbol/LineTableTest.cpp @@ -58,7 +58,7 @@ class FakeSymbolFile : public SymbolFile { uint32_t CalculateAbilities() override { return UINT32_MAX; } uint32_t GetNumCompileUnits() override { return 1; } CompUnitSP GetCompileUnitAtIndex(uint32_t) override { return m_cu_sp; } - Symtab *GetSymtab(bool can_create = true) override { return nullptr; } + Symtab *GetSymtab() override { return nullptr; } LanguageType ParseLanguage(CompileUnit &) override { return eLanguageTypeC; } size_t ParseFunctions(CompileUnit &) override { return 0; } bool ParseLineTable(CompileUnit &) override { return true; } diff --git a/lldb/unittests/Symbol/SymtabTest.cpp b/lldb/unittests/Symbol/SymtabTest.cpp index d3f462fab86a1..e431c31e1eac1 100644 --- a/lldb/unittests/Symbol/SymtabTest.cpp +++ b/lldb/unittests/Symbol/SymtabTest.cpp @@ -721,7 +721,7 @@ TEST_F(SymtabTest, TestDecodeCStringMaps) { ASSERT_NE(symbol, nullptr); } -TEST_F(SymtabTest, TestSymbolFileCreatedOnDemand) { +TEST_F(SymtabTest, TestSymtabCreatedOnDemand) { auto ExpectedFile = TestFile::fromYaml(R"( --- !ELF FileHeader: @@ -749,7 +749,7 @@ TEST_F(SymtabTest, TestSymbolFileCreatedOnDemand) { ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded()); auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec()); - // The symbol file should not be created by default. + // The symbol table should not be loaded by default. Symtab *module_symtab = module_sp->GetSymtab(/*can_create=*/false); ASSERT_EQ(module_symtab, nullptr); @@ -761,77 +761,3 @@ TEST_F(SymtabTest, TestSymbolFileCreatedOnDemand) { Symtab *cached_module_symtab = module_sp->GetSymtab(/*can_create=*/false); ASSERT_EQ(module_symtab, cached_module_symtab); } - -TEST_F(SymtabTest, TestSymbolTableCreatedOnDemand) { - const char *yamldata = R"( ---- !ELF -FileHeader: - Class: ELFCLASS64 - Data: ELFDATA2LSB - Type: ET_EXEC - Machine: EM_386 -DWARF: - debug_abbrev: - - Table: - - Code: 0x00000001 - Tag: DW_TAG_compile_unit - Children: DW_CHILDREN_no - Attributes: - - Attribute: DW_AT_addr_base - Form: DW_FORM_sec_offset - debug_info: - - Version: 5 - AddrSize: 4 - UnitType: DW_UT_compile - Entries: - - AbbrCode: 0x00000001 - Values: - - Value: 0x8 # Offset of the first Address past the header - - AbbrCode: 0x0 - debug_addr: - - Version: 5 - AddressSize: 4 - Entries: - - Address: 0x1234 - - Address: 0x5678 - debug_line: - - Length: 42 - Version: 2 - PrologueLength: 36 - MinInstLength: 1 - DefaultIsStmt: 1 - LineBase: 251 - LineRange: 14 - OpcodeBase: 13 - StandardOpcodeLengths: [ 0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 ] - IncludeDirs: - - '/tmp' - Files: - - Name: main.cpp - DirIdx: 1 - ModTime: 0 - Length: 0 -)"; - llvm::Expected<TestFile> file = TestFile::fromYaml(yamldata); - EXPECT_THAT_EXPECTED(file, llvm::Succeeded()); - auto module_sp = std::make_shared<Module>(file->moduleSpec()); - - SymbolFile *symbol_file = module_sp->GetSymbolFile(); - // At this point, the symbol table is not created. This is because the above - // yaml data contains the necessary sections in order for - // SymbolFileDWARF::CalculateAbilities() to identify all abilities, saving it - // from calling SymbolFileDWARFDebugMap::CalculateAbilities(), which - // eventually loads the symbol table, which we don't want. - - // The symbol table should not be created if asked not to. - Symtab *symtab = symbol_file->GetSymtab(/*can_create=*/false); - ASSERT_EQ(symtab, nullptr); - - // But it should be created on demand. - symtab = symbol_file->GetSymtab(/*can_create=*/true); - ASSERT_NE(symtab, nullptr); - - // And we should be able to get it again once it has been created. - Symtab *cached_symtab = symbol_file->GetSymtab(/*can_create=*/false); - ASSERT_EQ(symtab, cached_symtab); -} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
