This revision was automatically updated to reflect the committed changes.
Closed by commit rG3db7cc1ba41f: Fix a double debug info size counting in top
level stats for "statistics dump". (authored by clayborg).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119400/new/
https://reviews.llvm.org/D119400
Files:
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Target/Statistics.h
lldb/source/Core/Section.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Target/Statistics.cpp
lldb/test/API/commands/statistics/basic/TestStats.py
Index: lldb/test/API/commands/statistics/basic/TestStats.py
===================================================================
--- lldb/test/API/commands/statistics/basic/TestStats.py
+++ lldb/test/API/commands/statistics/basic/TestStats.py
@@ -1,5 +1,6 @@
import lldb
import json
+import os
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil
@@ -8,10 +9,6 @@
mydir = TestBase.compute_mydir(__file__)
- def setUp(self):
- TestBase.setUp(self)
- self.build()
-
NO_DEBUG_INFO_TESTCASE = True
def test_enable_disable(self):
@@ -22,6 +19,7 @@
of LLDB and test that enabling and disabling stops expesive information
from being gathered.
"""
+ self.build()
target = self.createTestTarget()
self.expect("statistics disable", substrs=['need to enable statistics before disabling'], error=True)
@@ -89,6 +87,7 @@
return None
def test_expressions_frame_var_counts(self):
+ self.build()
lldbutil.run_to_source_breakpoint(self, "// break here",
lldb.SBFileSpec("main.c"))
@@ -158,6 +157,7 @@
"totalSymbolTableIndexTime": 0.234,
}
"""
+ self.build()
target = self.createTestTarget()
debug_stats = self.get_stats()
debug_stat_keys = [
@@ -225,6 +225,7 @@
}
"""
+ self.build()
target = self.createTestTarget()
lldbutil.run_to_source_breakpoint(self, "// break here",
lldb.SBFileSpec("main.c"))
@@ -262,6 +263,7 @@
"""
Test "statistics dump" and the memory information.
"""
+ self.build()
exe = self.getBuildArtifact("a.out")
target = self.createTestTarget(file_path=exe)
debug_stats = self.get_stats()
@@ -303,10 +305,18 @@
return module
return None
+ def find_module_by_id_in_metrics(self, id, stats):
+ modules = stats['modules']
+ for module in modules:
+ if module['identifier'] == id:
+ return module
+ return None
+
def test_modules(self):
"""
Test "statistics dump" and the module information.
"""
+ self.build()
exe = self.getBuildArtifact("a.out")
target = self.createTestTarget(file_path=exe)
debug_stats = self.get_stats()
@@ -394,6 +404,7 @@
}
"""
+ self.build()
target = self.createTestTarget()
self.runCmd("b main.cpp:7")
self.runCmd("b a_function")
@@ -436,3 +447,108 @@
for breakpoint in breakpoints:
self.verify_keys(breakpoint, 'target_stats["breakpoints"]',
bp_keys_exist, None)
+
+
+ @skipUnlessDarwin
+ @no_debug_info_test
+ def test_dsym_binary_has_symfile_in_stats(self):
+ """
+ Test that if our executable has a stand alone dSYM file containing
+ debug information, that the dSYM file path is listed as a key/value
+ pair in the "a.out" binaries module stats. Also verify the the main
+ executable's module statistics has a debug info size that is greater
+ than zero as the dSYM contains debug info.
+ """
+ self.build(debug_info="dsym")
+ exe_name = 'a.out'
+ exe = self.getBuildArtifact(exe_name)
+ dsym = self.getBuildArtifact(exe_name + ".dSYM")
+ # Make sure the executable file exists after building.
+ self.assertEqual(os.path.exists(exe), True)
+ # Make sure the dSYM file exists after building.
+ self.assertEqual(os.path.isdir(dsym), True)
+
+ # Create the target
+ target = self.createTestTarget(file_path=exe)
+
+ debug_stats = self.get_stats()
+
+ exe_stats = self.find_module_in_metrics(exe, debug_stats)
+ # If we have a dSYM file, there should be a key/value pair in the module
+ # statistics and the path should match the dSYM file path in the build
+ # artifacts.
+ self.assertIn('symbolFilePath', exe_stats)
+ stats_dsym = exe_stats['symbolFilePath']
+
+ # Make sure main executable's module info has debug info size that is
+ # greater than zero as the dSYM file and main executable work together
+ # in the lldb.SBModule class to provide the data.
+ self.assertGreater(exe_stats['debugInfoByteSize'], 0)
+
+ # The "dsym" variable contains the bundle directory for the dSYM, while
+ # the "stats_dsym" will have the
+ self.assertIn(dsym, stats_dsym)
+ # Since we have a dSYM file, we should not be loading DWARF from the .o
+ # files and the .o file module identifiers should NOT be in the module
+ # statistics.
+ self.assertNotIn('symbolFileModuleIdentifiers', exe_stats)
+
+ @skipUnlessDarwin
+ @no_debug_info_test
+ def test_no_dsym_binary_has_symfile_identifiers_in_stats(self):
+ """
+ Test that if our executable loads debug info from the .o files,
+ that the module statistics contains a 'symbolFileModuleIdentifiers'
+ key which is a list of module identifiers, and verify that the
+ module identifier can be used to find the .o file's module stats.
+ Also verify the the main executable's module statistics has a debug
+ info size that is zero, as the main executable itself has no debug
+ info, but verify that the .o files have debug info size that is
+ greater than zero. This test ensures that we don't double count
+ debug info.
+ """
+ self.build(debug_info="dwarf")
+ exe_name = 'a.out'
+ exe = self.getBuildArtifact(exe_name)
+ dsym = self.getBuildArtifact(exe_name + ".dSYM")
+ print("carp: dsym = '%s'" % (dsym))
+ # Make sure the executable file exists after building.
+ self.assertEqual(os.path.exists(exe), True)
+ # Make sure the dSYM file doesn't exist after building.
+ self.assertEqual(os.path.isdir(dsym), False)
+
+ # Create the target
+ target = self.createTestTarget(file_path=exe)
+
+ # Force the 'main.o' .o file's DWARF to be loaded so it will show up
+ # in the stats.
+ self.runCmd("b main.cpp:7")
+
+ debug_stats = self.get_stats()
+
+ exe_stats = self.find_module_in_metrics(exe, debug_stats)
+ # If we don't have a dSYM file, there should not be a key/value pair in
+ # the module statistics.
+ self.assertNotIn('symbolFilePath', exe_stats)
+
+ # Make sure main executable's module info has debug info size that is
+ # zero as there is no debug info in the main executable, only in the
+ # .o files. The .o files will also only be loaded if something causes
+ # them to be loaded, so we set a breakpoint to force the .o file debug
+ # info to be loaded.
+ self.assertEqual(exe_stats['debugInfoByteSize'], 0)
+
+ # When we don't have a dSYM file, the SymbolFileDWARFDebugMap class
+ # should create modules for each .o file that contains DWARF that the
+ # symbol file creates, so we need to verify that we have a valid module
+ # identifier for main.o that is we should not be loading DWARF from the .o
+ # files and the .o file module identifiers should NOT be in the module
+ # statistics.
+ self.assertIn('symbolFileModuleIdentifiers', exe_stats)
+
+ symfileIDs = exe_stats['symbolFileModuleIdentifiers']
+ for symfileID in symfileIDs:
+ o_module = self.find_module_by_id_in_metrics(symfileID, debug_stats)
+ self.assertNotEqual(o_module, None)
+ # Make sure each .o file has some debug info bytes.
+ self.assertGreater(o_module['debugInfoByteSize'], 0)
Index: lldb/source/Target/Statistics.cpp
===================================================================
--- lldb/source/Target/Statistics.cpp
+++ lldb/source/Target/Statistics.cpp
@@ -62,6 +62,15 @@
debug_info_index_loaded_from_cache);
module.try_emplace("debugInfoIndexSavedToCache",
debug_info_index_saved_to_cache);
+ if (!symfile_path.empty())
+ module.try_emplace("symbolFilePath", symfile_path);
+
+ if (!symfile_modules.empty()) {
+ json::Array symfile_ids;
+ for (const auto symfile_id: symfile_modules)
+ symfile_ids.emplace_back(symfile_id);
+ module.try_emplace("symbolFileModuleIdentifiers", std::move(symfile_ids));
+ }
return module;
}
@@ -200,6 +209,10 @@
}
SymbolFile *sym_file = module->GetSymbolFile();
if (sym_file) {
+
+ if (sym_file->GetObjectFile() != module->GetObjectFile())
+ module_stat.symfile_path =
+ sym_file->GetObjectFile()->GetFileSpec().GetPath();
module_stat.debug_index_time = sym_file->GetDebugInfoIndexTime().count();
module_stat.debug_parse_time = sym_file->GetDebugInfoParseTime().count();
module_stat.debug_info_size = sym_file->GetDebugInfoSize();
@@ -211,6 +224,9 @@
sym_file->GetDebugInfoIndexWasSavedToCache();
if (module_stat.debug_info_index_saved_to_cache)
++debug_index_saved;
+ ModuleList symbol_modules = sym_file->GetDebugInfoModules();
+ for (const auto &symbol_module: symbol_modules.Modules())
+ module_stat.symfile_modules.push_back((intptr_t)symbol_module.get());
}
symtab_parse_time += module_stat.symtab_parse_time;
symtab_index_time += module_stat.symtab_index_time;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -142,9 +142,8 @@
// PluginInterface protocol
llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
- uint64_t GetDebugInfoSize() override;
- lldb_private::StatsDuration::Duration GetDebugInfoParseTime() override;
- lldb_private::StatsDuration::Duration GetDebugInfoIndexTime() override;
+ // Statistics overrides.
+ lldb_private::ModuleList GetDebugInfoModules() override;
protected:
enum { kHaveInitializedOSOs = (1 << 0), kNumFlags };
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -1430,53 +1430,16 @@
return num_line_entries_added;
}
-uint64_t SymbolFileDWARFDebugMap::GetDebugInfoSize() {
- uint64_t debug_info_size = 0;
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool {
- ObjectFile *oso_objfile = oso_dwarf->GetObjectFile();
- if (!oso_objfile)
- return false; // Keep iterating
- ModuleSP module_sp = oso_objfile->GetModule();
- if (!module_sp)
- return false; // Keep iterating
- SectionList *section_list = module_sp->GetSectionList();
- if (section_list)
- debug_info_size += section_list->GetDebugInfoSize();
- return false; // Keep iterating
- });
- return debug_info_size;
-}
-
-StatsDuration::Duration SymbolFileDWARFDebugMap::GetDebugInfoParseTime() {
- StatsDuration::Duration elapsed(0.0);
+ModuleList SymbolFileDWARFDebugMap::GetDebugInfoModules() {
+ ModuleList oso_modules;
ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool {
ObjectFile *oso_objfile = oso_dwarf->GetObjectFile();
if (oso_objfile) {
ModuleSP module_sp = oso_objfile->GetModule();
- if (module_sp) {
- SymbolFile *symfile = module_sp->GetSymbolFile();
- if (symfile)
- elapsed += symfile->GetDebugInfoParseTime();
- }
- }
- return false; // Keep iterating
- });
- return elapsed;
-}
-
-StatsDuration::Duration SymbolFileDWARFDebugMap::GetDebugInfoIndexTime() {
- StatsDuration::Duration elapsed(0.0);
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool {
- ObjectFile *oso_objfile = oso_dwarf->GetObjectFile();
- if (oso_objfile) {
- ModuleSP module_sp = oso_objfile->GetModule();
- if (module_sp) {
- SymbolFile *symfile = module_sp->GetSymbolFile();
- if (symfile)
- elapsed += symfile->GetDebugInfoIndexTime();
- }
+ if (module_sp)
+ oso_modules.Append(module_sp);
}
return false; // Keep iterating
});
- return elapsed;
+ return oso_modules;
}
Index: lldb/source/Core/Section.cpp
===================================================================
--- lldb/source/Core/Section.cpp
+++ lldb/source/Core/Section.cpp
@@ -423,9 +423,15 @@
case eSectionTypeGoSymtab:
case eSectionTypeAbsoluteAddress:
case eSectionTypeOther:
+ // Used for "__dof_cache" in mach-o or ".debug" for COFF which isn't debug
+ // information that we parse at all. This was causing system files with no
+ // debug info to show debug info byte sizes in the "statistics dump" output
+ // for each module. New "eSectionType" enums should be created for dedicated
+ // debug info that has a predefined format if we wish for these sections to
+ // show up as debug info.
+ case eSectionTypeDebug:
return false;
- case eSectionTypeDebug:
case eSectionTypeDWARFDebugAbbrev:
case eSectionTypeDWARFDebugAbbrevDwo:
case eSectionTypeDWARFDebugAddr:
Index: lldb/include/lldb/Target/Statistics.h
===================================================================
--- lldb/include/lldb/Target/Statistics.h
+++ lldb/include/lldb/Target/Statistics.h
@@ -100,6 +100,13 @@
std::string path;
std::string uuid;
std::string triple;
+ // Path separate debug info file, or empty if none.
+ std::string symfile_path;
+ // If the debug info is contained in multiple files where each one is
+ // represented as a separate lldb_private::Module, then these are the
+ // identifiers of these modules in the global module list. This allows us to
+ // track down all of the stats that contribute to this module.
+ std::vector<intptr_t> symfile_modules;
double symtab_parse_time = 0.0;
double symtab_index_time = 0.0;
double debug_parse_time = 0.0;
Index: lldb/include/lldb/Symbol/SymbolFile.h
===================================================================
--- lldb/include/lldb/Symbol/SymbolFile.h
+++ lldb/include/lldb/Symbol/SymbolFile.h
@@ -9,6 +9,7 @@
#ifndef LLDB_SYMBOL_SYMBOLFILE_H
#define LLDB_SYMBOL_SYMBOLFILE_H
+#include "lldb/Core/ModuleList.h"
#include "lldb/Core/PluginInterface.h"
#include "lldb/Core/SourceLocationSpec.h"
#include "lldb/Symbol/CompilerDecl.h"
@@ -325,6 +326,15 @@
/// hasn't been indexed yet, or a valid duration if it has.
virtual StatsDuration::Duration GetDebugInfoIndexTime() { return {}; }
+ /// Get the additional modules that this symbol file uses to parse debug info.
+ ///
+ /// Some debug info is stored in stand alone object files that are represented
+ /// by unique modules that will show up in the statistics module list. Return
+ /// a list of modules that are not in the target module list that this symbol
+ /// file is currently using so that they can be tracked and assoicated with
+ /// the module in the statistics.
+ virtual ModuleList GetDebugInfoModules() { return ModuleList(); }
+
/// Accessors for the bool that indicates if the debug info index was loaded
/// from, or saved to the module index cache.
///
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
