https://github.com/dmpots updated
https://github.com/llvm/llvm-project/pull/129593
>From bca07d666683152df179f7784d0003262fa54834 Mon Sep 17 00:00:00 2001
From: David Peixotto <p...@meta.com>
Date: Wed, 26 Feb 2025 13:55:35 -0800
Subject: [PATCH 1/3] Avoid force loading symbol files in statistics collection
This commit modifies the `DebuggerStats::ReportStatistics` implementation
to avoid loading symbol files for unloaded symbols. We collect stats
on debugger shutdown and without this change it can cause the debugger
to hang for a long while on shutdown if they symbols were not previously
loaded (e.g. `settings set target.preload-symbols false`).
The implementation is done by adding an optional parameter to
`Module::GetSymtab` to control if the corresponding symbol file will
be loaded in the same way that can control it for `Module::GetSymbolFile`.
---
lldb/include/lldb/Core/Module.h | 6 +++-
lldb/source/Core/Module.cpp | 4 +--
lldb/source/Target/Statistics.cpp | 4 +--
.../Commands/command-statistics-dump.test | 31 +++++++++++++++++++
4 files changed, 40 insertions(+), 5 deletions(-)
create mode 100644 lldb/test/Shell/Commands/command-statistics-dump.test
diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 90e0f4b6a3aac..9aa05ed3eb074 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -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.
///
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 33668c5d20dde..d70f292abaea4 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -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;
}
diff --git a/lldb/source/Target/Statistics.cpp
b/lldb/source/Target/Statistics.cpp
index 8173d20457e21..b5d2e7bda1edf 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -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)
@@ -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())
diff --git a/lldb/test/Shell/Commands/command-statistics-dump.test
b/lldb/test/Shell/Commands/command-statistics-dump.test
new file mode 100644
index 0000000000000..d490a0c374057
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-statistics-dump.test
@@ -0,0 +1,31 @@
+# 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 %t-main.exe \
+# RUN: -O "settings set plugin.jit-loader.gdb.enable off" \
+# RUN: -O "settings set target.preload-symbols true" \
+# 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 %t-main.exe \
+# RUN: -O "settings set plugin.jit-loader.gdb.enable off" \
+# RUN: -O "settings set target.preload-symbols false" \
+# RUN: -o "statistics dump" \
+# RUN: -o "q" \
+# RUN: | FileCheck %s -check-prefixes=CHECK,PRELOAD_FALSE
+
+# PRELOAD_FALSE: "symbolTableParseTime": 0,
>From 2a6475487ef7cc885d72b36436c8b2cff9775644 Mon Sep 17 00:00:00 2001
From: David Peixotto <p...@meta.com>
Date: Tue, 4 Mar 2025 15:03:25 -0800
Subject: [PATCH 2/3] Add unit test for new GetSymtab overload
Add a unit test to show that symbol tables are only
created on demand.
---
lldb/unittests/Symbol/SymtabTest.cpp | 41 +++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/lldb/unittests/Symbol/SymtabTest.cpp
b/lldb/unittests/Symbol/SymtabTest.cpp
index 7b8892e5b5c0f..76743c96cc502 100644
--- a/lldb/unittests/Symbol/SymtabTest.cpp
+++ b/lldb/unittests/Symbol/SymtabTest.cpp
@@ -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"
@@ -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;
};
@@ -718,3 +720,40 @@ 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);
+}
>From 86b8e935e69b94d434cc0819bc4481fe4b4aff56 Mon Sep 17 00:00:00 2001
From: David Peixotto <p...@meta.com>
Date: Tue, 4 Mar 2025 17:42:13 -0800
Subject: [PATCH 3/3] Create target without loading dependents
---
lldb/test/Shell/Commands/command-statistics-dump.test | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lldb/test/Shell/Commands/command-statistics-dump.test
b/lldb/test/Shell/Commands/command-statistics-dump.test
index d490a0c374057..b4e1252d01bbb 100644
--- a/lldb/test/Shell/Commands/command-statistics-dump.test
+++ b/lldb/test/Shell/Commands/command-statistics-dump.test
@@ -2,9 +2,9 @@
# 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 %t-main.exe \
-# RUN: -O "settings set plugin.jit-loader.gdb.enable off" \
+# 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
@@ -21,9 +21,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 %t-main.exe \
-# RUN: -O "settings set plugin.jit-loader.gdb.enable off" \
+# 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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
