-
Notifications
You must be signed in to change notification settings - Fork 14.4k
Defer loading all DWOs by default when dumping separate_debug-info #146166
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
base: main
Are you sure you want to change the base?
Conversation
The target modules dump separate separate-debug-info option automatically loads up all DWO files, even if deferred loading is enabled through debug_names. This adds the option to get the debug info without loading it up, if it hasn't been loaded yet, so that the correct DWO files will show up for each modules as "loaded" or not "loaded", which could be more informational to the user.
@llvm/pr-subscribers-lldb Author: None (qxy11) ChangesSummaryCurrently This change adds the option TestingUnit TestsAdded additional unit tests
Manual TestingCompiled a simple binary w/
Full diff: https://github.com/llvm/llvm-project/pull/146166.diff 10 Files Affected:
diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h
index b0b608d0a5e79..e95f95553c17c 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -467,8 +467,11 @@ class SymbolFile : public PluginInterface {
/// If true, then only return separate debug info files that encountered
/// errors during loading. If false, then return all expected separate
/// debug info files, regardless of whether they were successfully loaded.
+ /// \param load_all_debug_info
+ /// If true, force loading any symbol files if they are not yet loaded.
virtual bool GetSeparateDebugInfo(StructuredData::Dictionary &d,
- bool errors_only) {
+ bool errors_only,
+ bool load_all_debug_info = false) {
return false;
};
diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
index ba4a7f09afeaa..6e3c2477d1769 100644
--- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
+++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
@@ -223,9 +223,10 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile {
return m_sym_file_impl->SetDebugInfoHadFrameVariableErrors();
}
- bool GetSeparateDebugInfo(StructuredData::Dictionary &d,
- bool errors_only) override {
- return m_sym_file_impl->GetSeparateDebugInfo(d, errors_only);
+ bool GetSeparateDebugInfo(StructuredData::Dictionary &d, bool errors_only,
+ bool load_all_debug_info = false) override {
+ return m_sym_file_impl->GetSeparateDebugInfo(d, errors_only,
+ load_all_debug_info);
}
lldb::TypeSP MakeType(lldb::user_id_t uid, ConstString name,
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index a4ced37649ea0..89f93c82dea79 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1412,11 +1412,13 @@ static bool DumpModuleSymbolFile(Stream &strm, Module *module) {
}
static bool GetSeparateDebugInfoList(StructuredData::Array &list,
- Module *module, bool errors_only) {
+ Module *module, bool errors_only,
+ bool load_all_debug_info) {
if (module) {
if (SymbolFile *symbol_file = module->GetSymbolFile(/*can_create=*/true)) {
StructuredData::Dictionary d;
- if (symbol_file->GetSeparateDebugInfo(d, errors_only)) {
+ if (symbol_file->GetSeparateDebugInfo(d, errors_only,
+ load_all_debug_info)) {
list.AddItem(
std::make_shared<StructuredData::Dictionary>(std::move(d)));
return true;
@@ -2030,7 +2032,8 @@ class CommandObjectTargetModulesDumpSymtab
}
if (INTERRUPT_REQUESTED(GetDebugger(),
"Interrupted in dump all symtabs with {0} "
- "of {1} dumped.", num_dumped, num_modules))
+ "of {1} dumped.",
+ num_dumped, num_modules))
break;
num_dumped++;
@@ -2058,9 +2061,10 @@ class CommandObjectTargetModulesDumpSymtab
result.GetOutputStream().EOL();
result.GetOutputStream().EOL();
}
- if (INTERRUPT_REQUESTED(GetDebugger(),
- "Interrupted in dump symtab list with {0} of {1} dumped.",
- num_dumped, num_matches))
+ if (INTERRUPT_REQUESTED(
+ GetDebugger(),
+ "Interrupted in dump symtab list with {0} of {1} dumped.",
+ num_dumped, num_matches))
break;
num_dumped++;
@@ -2121,9 +2125,10 @@ class CommandObjectTargetModulesDumpSections
result.GetOutputStream().Format("Dumping sections for {0} modules.\n",
num_modules);
for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
- if (INTERRUPT_REQUESTED(GetDebugger(),
- "Interrupted in dump all sections with {0} of {1} dumped",
- image_idx, num_modules))
+ if (INTERRUPT_REQUESTED(
+ GetDebugger(),
+ "Interrupted in dump all sections with {0} of {1} dumped",
+ image_idx, num_modules))
break;
num_dumped++;
@@ -2142,9 +2147,10 @@ class CommandObjectTargetModulesDumpSections
FindModulesByName(&target, arg_cstr, module_list, true);
if (num_matches > 0) {
for (size_t i = 0; i < num_matches; ++i) {
- if (INTERRUPT_REQUESTED(GetDebugger(),
- "Interrupted in dump section list with {0} of {1} dumped.",
- i, num_matches))
+ if (INTERRUPT_REQUESTED(
+ GetDebugger(),
+ "Interrupted in dump section list with {0} of {1} dumped.",
+ i, num_matches))
break;
Module *module = module_list.GetModulePointerAtIndex(i);
@@ -2295,9 +2301,10 @@ class CommandObjectTargetModulesDumpClangAST
}
for (size_t i = 0; i < num_matches; ++i) {
- if (INTERRUPT_REQUESTED(GetDebugger(),
- "Interrupted in dump clang ast list with {0} of {1} dumped.",
- i, num_matches))
+ if (INTERRUPT_REQUESTED(
+ GetDebugger(),
+ "Interrupted in dump clang ast list with {0} of {1} dumped.", i,
+ num_matches))
break;
Module *m = module_list.GetModulePointerAtIndex(i);
@@ -2346,9 +2353,10 @@ class CommandObjectTargetModulesDumpSymfile
result.GetOutputStream().Format(
"Dumping debug symbols for {0} modules.\n", num_modules);
for (ModuleSP module_sp : target_modules.ModulesNoLocking()) {
- if (INTERRUPT_REQUESTED(GetDebugger(), "Interrupted in dumping all "
+ if (INTERRUPT_REQUESTED(GetDebugger(),
+ "Interrupted in dumping all "
8000
"debug symbols with {0} of {1} modules dumped",
- num_dumped, num_modules))
+ num_dumped, num_modules))
break;
if (DumpModuleSymbolFile(result.GetOutputStream(), module_sp.get()))
@@ -2365,9 +2373,10 @@ class CommandObjectTargetModulesDumpSymfile
FindModulesByName(&target, arg_cstr, module_list, true);
if (num_matches > 0) {
for (size_t i = 0; i < num_matches; ++i) {
- if (INTERRUPT_REQUESTED(GetDebugger(), "Interrupted dumping {0} "
- "of {1} requested modules",
- i, num_matches))
+ if (INTERRUPT_REQUESTED(GetDebugger(),
+ "Interrupted dumping {0} "
+ "of {1} requested modules",
+ i, num_matches))
break;
Module *module = module_list.GetModulePointerAtIndex(i);
if (module) {
@@ -2436,8 +2445,8 @@ class CommandObjectTargetModulesDumpLineTable
for (ModuleSP module_sp : target_modules.ModulesNoLocking()) {
if (INTERRUPT_REQUESTED(GetDebugger(),
"Interrupted in dump all line tables with "
- "{0} of {1} dumped", num_dumped,
- num_modules))
+ "{0} of {1} dumped",
+ num_dumped, num_modules))
break;
if (DumpCompileUnitLineTable(
@@ -2522,6 +2531,10 @@ class CommandObjectTargetModulesDumpSeparateDebugInfoFiles
const int short_option = m_getopt_table[option_idx].val;
switch (short_option) {
+ case 'd':
+ m_load_all_debug_info.SetCurrentValue(false);
+ m_load_all_debug_info.SetOptionWasSet();
+ break;
case 'j':
m_json.SetCurrentValue(true);
m_json.SetOptionWasSet();
@@ -2539,6 +2552,7 @@ class CommandObjectTargetModulesDumpSeparateDebugInfoFiles
void OptionParsingStarting(ExecutionContext *execution_context) override {
m_json.Clear();
m_errors_only.Clear();
+ m_load_all_debug_info.Clear();
}
llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
@@ -2547,6 +2561,7 @@ class CommandObjectTargetModulesDumpSeparateDebugInfoFiles
OptionValueBoolean m_json = false;
OptionValueBoolean m_errors_only = false;
+ OptionValueBoolean m_load_all_debug_info = true;
};
protected:
@@ -2578,7 +2593,8 @@ class CommandObjectTargetModulesDumpSeparateDebugInfoFiles
if (GetSeparateDebugInfoList(separate_debug_info_lists_by_module,
module_sp.get(),
- bool(m_options.m_errors_only)))
+ bool(m_options.m_errors_only),
+ bool(m_options.m_load_all_debug_info)))
num_dumped++;
}
} else {
@@ -2599,7 +2615,8 @@ class CommandObjectTargetModulesDumpSeparateDebugInfoFiles
break;
Module *module = module_list.GetModulePointerAtIndex(i);
if (GetSeparateDebugInfoList(separate_debug_info_lists_by_module,
- module, bool(m_options.m_errors_only)))
+ module, bool(m_options.m_errors_only),
+ bool(m_options.m_load_all_debug_info)))
num_dumped++;
}
} else
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 5327647d65cc4..42e6466d7a65e 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -13,6 +13,9 @@ let Command = "target modules dump separate debug info" in {
Desc<"Output the details in JSON format.">;
def tm_errors_only : Option<"errors-only", "e">, Group<1>,
Desc<"Filter to show only debug info files with errors.">;
+ def tm_defer_load_all_debug_info : Option<"defer-load-all-debug-info", "d">,
+ Group<1>,
+ Desc<"Load all debug info files.">;
}
let Command = "help" in {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index c83779c40a05b..5b16ce5f75138 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4139,7 +4139,8 @@ void SymbolFileDWARF::DumpClangAST(Stream &s, llvm::StringRef filter) {
}
bool SymbolFileDWARF::GetSeparateDebugInfo(StructuredData::Dictionary &d,
- bool errors_only) {
+ bool errors_only,
+ bool load_all_debug_info) {
StructuredData::Array separate_debug_info_files;
DWARFDebugInfo &info = DebugInfo();
const size_t num_cus = info.GetNumUnits();
@@ -4182,7 +4183,7 @@ bool SymbolFileDWARF::GetSeparateDebugInfo(StructuredData::Dictionary &d,
// If we have a DWO symbol file, that means we were able to successfully
// load it.
- SymbolFile *dwo_symfile = dwarf_cu->GetDwoSymbolFile();
+ SymbolFile *dwo_symfile = dwarf_cu->GetDwoSymbolFile(load_all_debug_info);
if (dwo_symfile) {
dwo_data->AddStringItem(
"resolved_dwo_path",
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index 8335f11712872..2dc862cccca14 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -279,8 +279,8 @@ class SymbolFileDWARF : public SymbolFileCommon {
void DumpClangAST(Stream &s, llvm::StringRef filter) override;
/// List separate dwo files.
- bool GetSeparateDebugInfo(StructuredData::Dictionary &d,
- bool errors_only) override;
+ bool GetSeparateDebugInfo(StructuredData::Dictionary &d, bool errors_only,
+ bool load_all_debug_info = false) override;
// Gets a pair of loaded and total dwo file counts.
// For split-dwarf files, this reports the counts for successfully loaded DWO
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
index f3a940b2ee396..dd94f0b36f2b2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -1278,7 +1278,8 @@ void SymbolFileDWARFDebugMap::DumpClangAST(Stream &s, llvm::StringRef filter) {
}
bool SymbolFileDWARFDebugMap::GetSeparateDebugInfo(
- lldb_private::StructuredData::Dictionary &d, bool errors_only) {
+ lldb_private::StructuredData::Dictionary &d, bool errors_only,
+ bool load_all_debug_info) {
StructuredData::Array separate_debug_info_files;
const uint32_t cu_count = GetNumCompileUnits();
for (uint32_t cu_idx = 0; cu_idx < cu_count; ++cu_idx) {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
index 35cbdbbb1692f..f074b17082e62 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -132,8 +132,8 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon {
void DumpClangAST(Stream &s, llvm::StringRef filter) override;
/// List separate oso files.
- bool GetSeparateDebugInfo(StructuredData::Dictionary &d,
- bool errors_only) override;
+ bool GetSeparateDebugInfo(StructuredData::Dictionary &d, bool errors_only,
+ bool load_all_debug_info = false) override;
// PluginInterface protocol
llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
diff --git a/lldb/test/API/commands/target/dump-separate-debug-info/dwo/Makefile b/lldb/test/API/commands/target/dump-separate-debug-info/dwo/Makefile
index 99b3fb3bd7762..00bbef5df3004 100644
--- a/lldb/test/API/commands/target/dump-separate-debug-info/dwo/Makefile
+++ b/lldb/test/API/commands/target/dump-separate-debug-info/dwo/Makefile
@@ -1,4 +1,4 @@
include Makefile.rules
a.out:
- $(CC) -target x86_64-pc-linux-elf -g -gsplit-dwarf -o $@ $(SRCDIR)/main.c $(SRCDIR)/foo.c
+ $(CC) $(CFLAGS) -target x86_64-pc-linux-elf -g -gsplit-dwarf -o $@ $(SRCDIR)/main.c $(SRCDIR)/foo.c
diff --git a/lldb/test/API/commands/target/dump-separate-debug-info/dwo/TestDumpDwo.py b/lldb/test/API/commands/target/dump-separate-debug-info/dwo/TestDumpDwo.py
index 13d12e3686a17..66e0808657fd2 100644
--- a/lldb/test/API/commands/target/dump-separate-debug-info/dwo/TestDumpDwo.py
+++ b/lldb/test/API/commands/target/dump-separate-debug-info/dwo/TestDumpDwo.py
@@ -24,9 +24,9 @@ def get_dwos_from_json_output(self):
result[symfile_entry["symfile"]] = dwo_dict
return result
- def build_and_skip_if_error(self):
+ def build_and_skip_if_error(self, debug_info=None):
try:
- self.build()
+ self.build(debug_info=debug_info)
except BuildError as e:
self.skipTest(f"Skipping test due to build exception: {e}")
@@ -148,3 +148,76 @@ def test_dwos_loaded_symbols_on_demand(self):
output = self.get_dwos_from_json_output()
self.assertTrue(output[exe]["a.out-main.dwo"]["loaded"])
self.assertTrue(output[exe]["a.out-foo.dwo"]["loaded"])
+
+ def test_dwos_loaded_symbols_on_demand_defer_load_all(self):
+ self.build_and_skip_if_error()
+ exe = self.getBuildArtifact("a.out")
+ main_dwo = self.getBuildArtifact("a.out-main.dwo")
+ foo_dwo = self.getBuildArtifact("a.out-foo.dwo")
+
+ # Make sure dwo files exist
+ self.assertTrue(os.path.exists(main_dwo), f'Make sure "{main_dwo}" file exists')
+ self.assertTrue(os.path.exists(foo_dwo), f'Make sure "{foo_dwo}" file exists')
+
+ # Load symbols on-demand
+ self.runCmd("settings set symbols.load-on-demand true")
+
+ target = self.dbg.CreateTarget(exe)
+ self.assertTrue(target, lldbtest.VALID_TARGET)
+
+ self.runCmd(
+ "target modules dump separate-debug-info --json --defer-load-all-debug-info"
+ )
+
+ # Check the output
+ output = self.get_dwos_from_json_output()
+ self.assertFalse(output[exe]["a.out-main.dwo"]["loaded"])
+ self.assertFalse(output[exe]["a.out-foo.dwo"]["loaded"])
+
+ # Set a breakpoint in main(). All DWO files should be loaded now
+ self.runCmd("b main")
+ self.runCmd(
+ "target modules dump separate-debug-info --json --defer-load-all-debug-info"
+ )
+ output = self.get_dwos_from_json_output()
+ self.assertTrue(output[exe]["a.out-main.dwo"]["loaded"])
+ self.assertTrue(output[exe]["a.out-foo.dwo"]["loaded"])
+
+ def test_dwos_defer_loaded_json_with_debug_names(self):
+ """
+ Test that DWO files are lazily loaded, and target module dump gives the expected output.
+ """
+ # Build with split DWARF, debug_names, and gpubnames
+ self.build_and_skip_if_error(debug_info=["debug_names"])
+ exe = self.getBuildArtifact("a.out")
+
+ main_dwo = self.getBuildArtifact("a.out-main.dwo")
+ foo_dwo = self.getBuildArtifact("a.out-foo.dwo")
+
+ # Make sure dwo files exist
+ self.assertTrue(os.path.exists(main_dwo), f'Make sure "{main_dwo}" file exists')
+ self.assertTrue(os.path.exists(foo_dwo), f'Make sure "{foo_dwo}" file exists')
+
+ target = self.dbg.CreateTarget(exe)
+ self.assertTrue(target, lldbtest.VALID_TARGET)
+
+ self.runCmd("target modules dump separate-debug-info --j --d")
+
+ # Check the output
+ output = self.get_dwos_from_json_output()
+ self.assertFalse(output[exe]["a.out-main.dwo"]["loaded"])
+ self.assertFalse(output[exe]["a.out-foo.dwo"]["loaded"])
+
+ # Set a breakpoint in main(). a.out-main.dwo should be loaded now
+ self.runCmd("b main")
+ self.runCmd("target modules dump separate-debug-info --j --d")
+ output = self.get_dwos_from_json_output()
+ self.assertTrue(output[exe]["a.out-main.dwo"]["loaded"])
+ self.assertFalse(output[exe]["a.out-foo.dwo"]["loaded"])
+
+ # Set a breakpoint in foo(). a.out-foo.dwo should be loaded now
+ self.runCmd("b foo")
+ self.runCmd("target modules dump separate-debug-info --j --d")
+ output = self.get_dwos_from_json_output()
+ self.assertTrue(output[exe]["a.out-main.dwo"]["loaded"])
+ self.assertTrue(output[exe]["a.out-foo.dwo"]["loaded"])
|
@@ -2030,7 +2032,8 @@ class CommandObjectTargetModulesDumpSymtab | |||
} | |||
if (INTERRUPT_REQUESTED(GetDebugger(), | |||
"Interrupted in dump all symtabs with {0} " | |||
"of {1} dumped.", num_dumped, num_modules)) | |||
"of {1} dumped.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like there are some formatting changes here that are not related to the functional change. We should split those out into a separate NFC PR if we want to include them.
It looks like all the INTERRRUPT_REQUESTED
changes fall into that category.
@@ -2547,6 +2561,7 @@ class CommandObjectTargetModulesDumpSeparateDebugInfoFiles | |||
|
|||
OptionValueBoolean m_json = false; | |||
OptionValueBoolean m_errors_only = false; | |||
OptionValueBoolean m_load_all_debug_info = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be better to disable loading all of the debug info by default. Then instead of adding the --defer-load-all-debug-info
flag we could add a flag like --force-load-all-debug-info
that would force loading it.
It seems a bit odd to me that a dump
command would modify state like this by default by loading up the debug info instead of reporting the current state. But this would be a behavior change from the current lldb so maybe not desirable.
@JDevlieghere @jeffreytan81 @clayborg any thoughts on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed to default loading all debug info to false and have the option to force loading. I think the only two cases this should effect are when symbols.load-on-demand=true
(as in the unit test, where I had to change expected behavior) and when debug_names are used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Summary
Currently
target modules dump separate separate-debug-info
automatically loads up all DWO files, even if deferred loadi 8000 ng is enabled through debug_names. Then, as expected all DWO files (assuming there is no error loading it), get marked as "loaded".This change adds the option
--force-load-all-debug-info
or-f
for short to force loading all debug_info up, if it hasn't been loaded yet. Otherwise, it will change default behavior to not load all debug info so that the correct DWO files will show up for each modules as "loaded" or not "loaded", which could be helpful in cases where we want to know which particular DWO files were loaded.Testing
Unit Tests
Added additional unit tests
test_dwos_load_json_with_debug_names_default
andtest_dwos_load_json_with_debug_names_force_load_all
to test both default behavior and loading with the new flag--force-load-all-debug-info
, and changed expected behavior intest_dwos_loaded_symbols_on_demand
.Manual Testing
Compiled a simple binary w/
--gsplit-dwarf --gpubnames
and loaded it up: