Skip to content

[lldb] Avoid force loading symbols in statistics collection #136236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Apr 21, 2025
2 changes: 1 addition & 1 deletion lldb/include/lldb/Symbol/ObjectFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
///
/// \return
/// The symbol table for this object file.
Symtab *GetSymtab();
Symtab *GetSymtab(bool can_create = true);

/// Parse the symbol table into the provides symbol table object.
///
Expand Down
4 changes: 2 additions & 2 deletions lldb/include/lldb/Symbol/SymbolFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class SymbolFile : public PluginInterface {
virtual uint32_t GetNumCompileUnits() = 0;
virtual lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx) = 0;

virtual Symtab *GetSymtab() = 0;
virtual Symtab *GetSymtab(bool can_create = true) = 0;

virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
/// Return the Xcode SDK comp_unit was compiled against.
Expand Down Expand Up @@ -533,7 +533,7 @@ class SymbolFileCommon : public SymbolFile {
return m_abilities;
}

Symtab *GetSymtab() override;
Symtab *GetSymtab(bool can_create = true) override;

ObjectFile *GetObjectFile() override { return m_objfile_sp.get(); }
const ObjectFile *GetObjectFile() const override {
Expand Down
4 changes: 3 additions & 1 deletion lldb/include/lldb/Symbol/SymbolFileOnDemand.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,9 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile {

uint32_t GetAbilities() override;

Symtab *GetSymtab() override { return m_sym_file_impl->GetSymtab(); }
Symtab *GetSymtab(bool can_create = true) override {
return m_sym_file_impl->GetSymtab(can_create);
}

ObjectFile *GetObjectFile() override {
return m_sym_file_impl->GetObjectFile();
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Core/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
return symbols->GetSymtab(can_create);
return nullptr;
}

Expand Down
5 changes: 2 additions & 3 deletions lldb/source/Symbol/ObjectFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -734,10 +734,9 @@ void llvm::format_provider<ObjectFile::Strata>::format(
}
}


Symtab *ObjectFile::GetSymtab() {
Symtab *ObjectFile::GetSymtab(bool can_create) {
ModuleSP module_sp(GetModule());
if (module_sp) {
if (module_sp && can_create) {
// 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
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Symbol/SymbolFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,10 @@ void SymbolFile::AssertModuleLock() {

SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default;

Symtab *SymbolFileCommon::GetSymtab() {
Symtab *SymbolFileCommon::GetSymtab(bool can_create) {
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
// Fetch the symtab from the main object file.
auto *symtab = GetMainObjectFile()->GetSymtab();
auto *symtab = GetMainObjectFile()->GetSymtab(can_create);
if (m_symtab != symtab) {
m_symtab = symtab;

Expand Down
2 changes: 1 addition & 1 deletion lldb/unittests/Symbol/LineTableTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() override { return nullptr; }
Symtab *GetSymtab(bool can_create = true) override { return nullptr; }
LanguageType ParseLanguage(CompileUnit &) override { return eLanguageTypeC; }
size_t ParseFunctions(CompileUnit &) override { return 0; }
bool ParseLineTable(CompileUnit &) override { return true; }
Expand Down
14 changes: 12 additions & 2 deletions lldb/unittests/Symbol/SymtabTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ TEST_F(SymtabTest, TestDecodeCStringMaps) {
ASSERT_NE(symbol, nullptr);
}

TEST_F(SymtabTest, TestSymtabCreatedOnDemand) {
TEST_F(SymtabTest, TestSymbolFileAndSymbolTableCreatedOnDemand) {
auto ExpectedFile = TestFile::fromYaml(R"(
--- !ELF
FileHeader:
Expand Down Expand Up @@ -749,10 +749,20 @@ TEST_F(SymtabTest, TestSymtabCreatedOnDemand) {
ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());

// The symbol table should not be loaded by default.
// The symbol file should not be created by default.
Symtab *module_symtab = module_sp->GetSymtab(/*can_create=*/false);
ASSERT_EQ(module_symtab, nullptr);

// Even if the symbol file is created, the symbol table should not be created by default.

// TODO:
// I need to create a symbol file here, but without causing it to parse the symbol table.
// See next line as a failed attempt.

// module_sp->GetSymbolFile(/*can_create=*/true); // Cannot do this because it will parse the symbol table.
module_symtab = module_sp->GetSymtab(/*can_create=*/false);
ASSERT_EQ(module_symtab, nullptr);

// But it should be created on demand.
module_symtab = module_sp->GetSymtab(/*can_create=*/true);
ASSERT_NE(module_symtab, nullptr);
Expand Down
Loading