[Lldb-commits] [lldb] 08b4c52 - Revert "[lldb] Avoid force loading symbols in statistics collection (#136236)"
Shubham Sandeep Rastogi via lldb-commits
lldb-commits at lists.llvm.org
Mon Apr 21 17:20:40 PDT 2025
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);
-}
More information about the lldb-commits
mailing list