Skip to content

[lldb] Avoid force loading symbol files in statistics collection #129593

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 5 commits into from
Mar 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lldb/include/lldb/Core/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,11 @@ class Module : public std::enable_shared_from_this<Module>,
virtual SymbolFile *GetSymbolFile(bool can_create = true,
Stream *feedback_strm = nullptr);

Symtab *GetSymtab();
/// Get the module's symbol table
///
/// If the symbol table has already been loaded, this function returns it.
/// Otherwise, it will only be loaded when can_create is true.
Symtab *GetSymtab(bool can_create = true);

/// Get a reference to the UUID value contained in this object.
///
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Core/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1022,8 +1022,8 @@ SymbolFile *Module::GetSymbolFile(bool can_create, Stream *feedback_strm) {
return m_symfile_up ? m_symfile_up->GetSymbolFile() : nullptr;
}

Symtab *Module::GetSymtab() {
if (SymbolFile *symbols = GetSymbolFile())
Symtab *Module::GetSymtab(bool can_create) {
if (SymbolFile *symbols = GetSymbolFile(can_create))
return symbols->GetSymtab();
return nullptr;
}
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Target/Statistics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
ModuleStats module_stat;
module_stat.symtab_parse_time = module->GetSymtabParseTime().get().count();
module_stat.symtab_index_time = module->GetSymtabIndexTime().get().count();
Symtab *symtab = module->GetSymtab();
Symtab *symtab = module->GetSymtab(/*can_create=*/false);
if (symtab) {
module_stat.symtab_loaded_from_cache = symtab->GetWasLoadedFromCache();
if (module_stat.symtab_loaded_from_cache)
Expand All @@ -325,7 +325,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
if (module_stat.symtab_saved_to_cache)
++symtabs_saved;
}
SymbolFile *sym_file = module->GetSymbolFile();
SymbolFile *sym_file = module->GetSymbolFile(/*can_create=*/false);
if (sym_file) {
if (!summary_only) {
if (sym_file->GetObjectFile() != module->GetObjectFile())
Expand Down
40 changes: 40 additions & 0 deletions lldb/test/Shell/Commands/command-statistics-dump.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# This test validates that statistics generation does not force loading
# symbol tables. In order to avoid other sources of symbol loading we
# create the target without loading dependents and do not actually
# run it. Running the target is a problem because there are various
# instrumentation plugins (e.g. ASAN) that are always enabled and force
# symbol loading. If you see this test start to fail we may have added
# a new source of symbol loading unexpectedly.

# Build a simple test executable.
# RUN: %clang_host -g %S/Inputs/main.c -o %t-main.exe

# When we enable symbol preload and dump stats there should be a non-zero
# time for parsing symbol tables for the main module.
# RUN: %lldb -O "settings set plugin.jit-loader.gdb.enable off" \
# RUN: -O "settings set target.preload-symbols true" \
# RUN: -o 'target create --no-dependents "%t-main.exe"' \
# RUN: -o "statistics dump" \
# RUN: -o "q" \
# RUN: | FileCheck %s -check-prefixes=CHECK,PRELOAD_TRUE

# Find the module stats for the main executable and make sure
# we are looking at the symbol parse time for that module.
# CHECK: "modules": [
# CHECK: {
# CHECK: "path": {{.*}}-main.exe
# CHECK-NOT: }

# PRELOAD_TRUE: "symbolTableParseTime":
# PRELOAD_TRUE-SAME: {{[1-9]+}}

# When we disable symbol preload and dump stats the symbol table
# for main should not be parsed and have a time of 0.
# RUN: %lldb -O "settings set plugin.jit-loader.gdb.enable off" \
# RUN: -O "settings set target.preload-symbols false" \
# RUN: -o 'target create --no-dependents "%t-main.exe"' \
# RUN: -o "statistics dump" \
# RUN: -o "q" \
# RUN: | FileCheck %s -check-prefixes=CHECK,PRELOAD_FALSE

# PRELOAD_FALSE: "symbolTableParseTime": 0,
45 changes: 44 additions & 1 deletion lldb/unittests/Symbol/SymtabTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
//
//===----------------------------------------------------------------------===//

#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
#include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
#include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
#include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
#include "TestingSupport/SubsystemRAII.h"
#include "TestingSupport/TestUtilities.h"
Expand All @@ -31,7 +33,7 @@ using namespace lldb_private::plugin::dwarf;

class SymtabTest : public testing::Test {
SubsystemRAII<FileSystem, HostInfo, ObjectFileMachO, SymbolFileDWARF,
TypeSystemClang>
ObjectFileELF, SymbolFileSymtab, TypeSystemClang>
subsystem;
};

Expand Down Expand Up @@ -718,3 +720,44 @@ TEST_F(SymtabTest, TestDecodeCStringMaps) {
Symtab::eVisibilityAny);
ASSERT_NE(symbol, nullptr);
}

TEST_F(SymtabTest, TestSymtabCreatedOnDemand) {
auto ExpectedFile = TestFile::fromYaml(R"(
--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_EXEC
Machine: EM_X86_64
Entry: 0x0000000000400180
Sections:
- Name: .text
Type: SHT_PROGBITS
Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
Address: 0x0000000000400180
AddressAlign: 0x0000000000000010
Content: 554889E58B042500106000890425041060005DC3
Symbols:
- Name: _start
Type: STT_FUNC
Section: .text
Value: 0x0000000000400180
Size: 0x0000000000000014
Binding: STB_GLOBAL
...
)");
ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());

// The symbol table should not be loaded by default.
Symtab *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);

// 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);
}