-
Notifications
You must be signed in to change notification settings - Fork 14.9k
Reland"[lldb] Add count for errors of DWO files in statistics and combine DWO file count functions" #156980
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
…ine DWO file count functions
@llvm/pr-subscribers-lldb Author: Ziyi Wang (zw3917) ChangesThis relands changes in #155023 for adding a count of dwo errors and combine all the dwo related stats into one struct. The previous PR was reverted in #156777 as the newly added unit test Original PR: https://github.com/llvm/llvm-project/pull/155023 TestingRan unit tests
Full diff: https://github.com/llvm/llvm-project/pull/156980.diff 8 Files Affected:
diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h
index bbc615d9fdc38..ad35b1defb5c9 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -488,13 +488,16 @@ class SymbolFile : public PluginInterface {
return false;
};
- /// Get number of loaded/parsed DWO files. This is emitted in "statistics
- /// dump"
+ /// Retrieves statistics about DWO files associated with this symbol file.
+ /// This function returns a DWOStats struct containing:
+ /// - The number of successfully loaded/parsed DWO files.
+ /// - The total number of DWO files encountered.
+ /// - The number of DWO CUs that failed to load due to errors.
+ /// If this symbol file does not support DWO files, all counts will be zero.
///
/// \returns
- /// A pair containing (loaded_dwo_count, total_dwo_count). If this
- /// symbol file doesn't support DWO files, both counts will be 0.
- virtual std::pair<uint32_t, uint32_t> GetDwoFileCounts() { return {0, 0}; }
+ /// A DWOStats struct with loaded, total, and error counts for DWO files.
+ virtual DWOStats GetDwoStats() { return {}; }
virtual lldb::TypeSP
MakeType(lldb::user_id_t uid, ConstString name,
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index 55dff8861a9ab..c288917edbc6a 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -123,6 +123,19 @@ struct StatsSuccessFail {
uint32_t failures = 0;
};
+/// Holds statistics about DWO (Debug With Object) files.
+struct DWOStats {
+ uint32_t loaded_dwo_file_count = 0;
+ uint32_t dwo_file_count = 0;
+ uint32_t dwo_error_count = 0;
+
+ DWOStats operator+(const DWOStats &rhs) const {
+ return DWOStats{loaded_dwo_file_count + rhs.loaded_dwo_file_count,
+ dwo_file_count + rhs.dwo_file_count,
+ dwo_error_count + rhs.dwo_error_count};
+ }
+};
+
/// A class that represents statistics for a since lldb_private::Module.
struct ModuleStats {
llvm::json::Value ToJSON() const;
@@ -153,8 +166,7 @@ struct ModuleStats {
bool symtab_stripped = false;
bool debug_info_had_variable_errors = false;
bool debug_info_had_incomplete_types = false;
- uint32_t dwo_file_count = 0;
- uint32_t loaded_dwo_file_count = 0;
+ DWOStats dwo_stats;
};
struct ConstStringStats {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index b15e0c15fedb8..7108ac72eb030 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4502,9 +4502,8 @@ void SymbolFileDWARF::GetCompileOptions(
}
}
-std::pair<uint32_t, uint32_t> SymbolFileDWARF::GetDwoFileCounts() {
- uint32_t total_dwo_count = 0;
- uint32_t loaded_dwo_count = 0;
+DWOStats SymbolFileDWARF::GetDwoStats() {
+ DWOStats stats;
DWARFDebugInfo &info = DebugInfo();
const size_t num_cus = info.GetNumUnits();
@@ -4517,16 +4516,21 @@ std::pair<uint32_t, uint32_t> SymbolFileDWARF::GetDwoFileCounts() {
if (!dwarf_cu->GetDWOId().has_value())
continue;
- total_dwo_count++;
+ stats.dwo_file_count++;
// If we have a DWO symbol file, that means we were able to successfully
// load it.
SymbolFile *dwo_symfile =
dwarf_cu->GetDwoSymbolFile(/*load_all_debug_info=*/false);
if (dwo_symfile) {
- loaded_dwo_count++;
+ stats.loaded_dwo_file_count++;
}
+
+ // Check if this unit has a DWO load error, false by default.
+ const Status &dwo_error = dwarf_cu->GetDwoError();
+ if (dwo_error.Fail())
+ stats.dwo_error_count++;
}
- return {loaded_dwo_count, total_dwo_count};
+ return stats;
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index d7db8a3c0869f..8b8cb5859e8a0 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -283,10 +283,11 @@ class SymbolFileDWARF : public SymbolFileCommon {
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
- // CUs and total DWO CUs. For non-split-dwarf files, this reports 0 for both.
- std::pair<uint32_t, uint32_t> GetDwoFileCounts() override;
+ /// Gets statistics about dwo files associated with this symbol file.
+ /// For split-dwarf files, this reports the counts for successfully loaded DWO
+ /// CUs, total DWO CUs, and the number of DWO CUs with loading errors.
+ /// For non-split-dwarf files, this reports 0 for all.
+ DWOStats GetDwoStats() override;
DWARFContext &GetDWARFContext() { return m_context; }
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 909f335687b21..3c16df5440f25 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -73,8 +73,9 @@ json::Value ModuleStats::ToJSON() const {
debug_info_had_incomplete_types);
module.try_emplace("symbolTableStripped", symtab_stripped);
module.try_emplace("symbolTableSymbolCount", symtab_symbol_count);
- module.try_emplace("dwoFileCount", dwo_file_count);
- module.try_emplace("loadedDwoFileCount", loaded_dwo_file_count);
+ module.try_emplace("dwoFileCount", dwo_stats.dwo_file_count);
+ module.try_emplace("loadedDwoFileCount", dwo_stats.loaded_dwo_file_count);
+ module.try_emplace("dwoErrorCount", dwo_stats.dwo_error_count);
if (!symbol_locator_time.map.empty()) {
json::Object obj;
@@ -324,8 +325,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
uint32_t num_modules_with_incomplete_types = 0;
uint32_t num_stripped_modules = 0;
uint32_t symtab_symbol_count = 0;
- uint32_t total_loaded_dwo_file_count = 0;
- uint32_t total_dwo_file_count = 0;
+ DWOStats total_dwo_stats;
for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
Module *module = target != nullptr
? target->GetImages().GetModuleAtIndex(image_idx).get()
@@ -357,10 +357,9 @@ llvm::json::Value DebuggerStats::ReportStatistics(
for (const auto &symbol_module : symbol_modules.Modules())
module_stat.symfile_modules.push_back((intptr_t)symbol_module.get());
}
- std::tie(module_stat.loaded_dwo_file_count, module_stat.dwo_file_count) =
- sym_file->GetDwoFileCounts();
- total_dwo_file_count += module_stat.dwo_file_count;
- total_loaded_dwo_file_count += module_stat.loaded_dwo_file_count;
+ DWOStats current_dwo_stats = sym_file->GetDwoStats();
+ module_stat.dwo_stats = module_stat.dwo_stats + current_dwo_stats;
+ total_dwo_stats = total_dwo_stats + current_dwo_stats;
module_stat.debug_info_index_loaded_from_cache =
sym_file->GetDebugInfoIndexWasLoadedFromCache();
if (module_stat.debug_info_index_loaded_from_cache)
@@ -435,8 +434,9 @@ llvm::json::Value DebuggerStats::ReportStatistics(
{"totalDebugInfoEnabled", num_debug_info_enabled_modules},
{"totalSymbolTableStripped", num_stripped_modules},
{"totalSymbolTableSymbolCount", symtab_symbol_count},
- {"totalLoadedDwoFileCount", total_loaded_dwo_file_count},
- {"totalDwoFileCount", total_dwo_file_count},
+ {"totalLoadedDwoFileCount", total_dwo_stats.loaded_dwo_file_count},
+ {"totalDwoFileCount", total_dwo_stats.dwo_file_count},
+ {"totalDwoErrorCount", total_dwo_stats.dwo_error_count},
};
if (include_targets) {
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index e9ee8b8661e5a..c38a40ce8e560 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -1,6 +1,7 @@
import json
import os
import re
+import shutil
import lldb
from lldbsuite.test.decorators import *
@@ -179,6 +180,7 @@ def test_default_no_run(self):
"totalDebugInfoParseTime",
"totalDwoFileCount",
"totalLoadedDwoFileCount",
+ "totalDwoErrorCount",
]
self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
if self.getPlatform() != "windows":
@@ -291,6 +293,7 @@ def test_default_with_run(self):
"totalDebugInfoParseTime",
"totalDwoFileCount",
"totalLoadedDwoFileCount",
+ "totalDwoErrorCount",
]
self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
stats = debug_stats["targets"][0]
@@ -331,6 +334,7 @@ def test_memory(self):
"totalDebugInfoByteSize",
"totalDwoFileCount",
"totalLoadedDwoFileCount",
+ "totalDwoErrorCount",
]
self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
@@ -385,6 +389,7 @@ def test_modules(self):
"totalDebugInfoByteSize",
"totalDwoFileCount",
"totalLoadedDwoFileCount",
+ "totalDwoErrorCount",
]
self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
stats = debug_stats["targets"][0]
@@ -407,6 +412,7 @@ def test_modules(self):
"symbolTableSavedToCache",
"dwoFileCount",
"loadedDwoFileCount",
+ "dwoErrorCount",
"triple",
"uuid",
]
@@ -497,6 +503,7 @@ def test_breakpoints(self):
"totalDebugInfoByteSize",
"totalDwoFileCount",
"totalLoadedDwoFileCount",
+ "totalDwoErrorCount",
]
self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
target_stats = debug_stats["targets"][0]
@@ -655,6 +662,134 @@ def test_dwp_dwo_file_count(self):
self.assertEqual(debug_stats["totalDwoFileCount"], 2)
self.assertEqual(debug_stats["totalLoadedDwoFileCount"], 2)
+ @add_test_categories(["dwo"])
+ def test_dwo_missing_error_stats(self):
+ """
+ Test that DWO missing errors are reported correctly in statistics.
+ This test:
+ 1) Builds a program with split DWARF (.dwo files)
+ 2) Delete all two .dwo files
+ 3) Verify that 2 DWO errors are reported in statistics
+ """
+ da = {
+ "CXX_SOURCES": "dwo_error_main.cpp dwo_error_foo.cpp",
+ "EXE": self.getBuildArtifact("a.out"),
+ }
+ # -gsplit-dwarf creates separate .dwo files,
+ # Expected output: dwo_error_main.dwo (contains main) and dwo_error_foo.dwo (contains foo struct/function)
+ self.build(dictionary=da, debug_info="dwo")
+ self.addTearDownCleanup(dictionary=da)
+ exe = self.getBuildArtifact("a.out")
+
+ expected_dwo_files = [
+ self.getBuildArtifact("dwo_error_main.dwo"),
+ self.getBuildArtifact("dwo_error_foo.dwo"),
+ ]
+
+ # Verify expected files exist
+ for dwo_file in expected_dwo_files:
+ self.assertTrue(
+ os.path.exists(dwo_file),
+ f"Expected .dwo file does not exist: {dwo_file}",
+ )
+
+ # Remove the two .dwo files to trigger DWO load errors
+ for dwo_file in expected_dwo_files:
+ os.rename(dwo_file, dwo_file + ".bak")
+
+ target = self.createTestTarget(file_path=exe)
+ debug_stats = self.get_stats()
+
+ # Check DWO load error statistics are reported
+ self.assertIn("totalDwoErrorCount", debug_stats)
+ self.assertEqual(debug_stats["totalDwoErrorCount"], 2)
+
+ # Since there's only one module, module stats should have the same count as total count
+ self.assertIn("dwoErrorCount", debug_stats["modules"][0])
+ self.assertEqual(debug_stats["modules"][0]["dwoErrorCount"], 2)
+
+ # Restore the original .dwo file
+ for dwo_file in expected_dwo_files:
+ os.rename(dwo_file + ".bak", dwo_file)
+
+ @add_test_categories(["dwo"])
+ def test_dwo_id_mismatch_error_stats(self):
+ """
+ Test that DWO ID mismatch errors are reported correctly in statistics.
+ This test:
+ 1) Builds a program with split DWARF (.dwo files)
+ 2) Change one of the source file content and rebuild
+ 3) Replace the new .dwo file with the original one to create a DWO ID mismatch
+ 4) Verifies that a DWO error is reported in statistics
+ 5) Restores the original source file
+ """
+ da = {
+ "CXX_SOURCES": "dwo_error_main.cpp dwo_error_foo.cpp",
+ "EXE": self.getBuildArtifact("a.out"),
+ }
+ # -gsplit-dwarf creates separate .dwo files,
+ # Expected output: dwo_error_main.dwo (contains main) and dwo_error_foo.dwo (contains foo struct/function)
+ self.build(dictionary=da, debug_info="dwo")
+ self.addTearDownCleanup(dictionary=da)
+ exe = self.getBuildArtifact("a.out")
+
+ expected_dwo_files = [
+ self.getBuildArtifact("dwo_error_main.dwo"),
+ self.getBuildArtifact("dwo_error_foo.dwo"),
+ ]
+
+ # Verify expected files exist
+ for dwo_file in expected_dwo_files:
+ self.assertTrue(
+ os.path.exists(dwo_file),
+ f"Expected .dwo file does not exist: {dwo_file}",
+ )
+
+ # Find and make a backup of the original .dwo file
+ original_dwo_file = self.getBuildArtifact("dwo_error_foo.dwo")
+ original_dwo_backup = original_dwo_file + ".bak"
+ shutil.copy2(original_dwo_file, original_dwo_backup)
+
+ target = self.createTestTarget(file_path=exe)
+ initial_stats = self.get_stats()
+ self.assertIn("totalDwoErrorCount", initial_stats)
+ self.assertEqual(initial_stats["totalDwoErrorCount"], 0)
+ self.dbg.DeleteTarget(target)
+
+ # Get the original file size before modification
+ source_file_path = self.getSourcePath("dwo_error_foo.cpp")
+ original_size = os.path.getsize(source_file_path)
+
+ try:
+ # Modify the source code and rebuild
+ with open(source_file_path, "a") as f:
+ f.write("\n void additional_foo(){}\n")
+
+ # Rebuild and replace the new .dwo file with the original one
+ self.build(dictionary=da, debug_info="dwo")
+ shutil.copy2(original_dwo_backup, original_dwo_file)
+
+ # Create a new target and get stats
+ target = self.createTestTarget(file_path=exe)
+ debug_stats = self.get_stats()
+
+ # Check that DWO load error statistics are reported
+ self.assertIn("totalDwoErrorCount", debug_stats)
+ self.assertEqual(debug_stats["totalDwoErrorCount"], 1)
+
+ # Since there's only one module, module stats should have the same count as total count
+ self.assertIn("dwoErrorCount", debug_stats["modules"][0])
+ self.assertEqual(debug_stats["modules"][0]["dwoErrorCount"], 1)
+
+ finally:
+ # Remove the appended content
+ with open(source_file_path, "a") as f:
+ f.truncate(original_size)
+
+ # Restore the original .dwo file
+ if os.path.exists(original_dwo_backup):
+ os.unlink(original_dwo_backup)
+
@skipUnlessDarwin
@no_debug_info_test
def test_dsym_binary_has_symfile_in_stats(self):
diff --git a/lldb/test/API/commands/statistics/basic/dwo_error_foo.cpp b/lldb/test/API/commands/statistics/basic/dwo_error_foo.cpp
new file mode 100644
index 0000000000000..41618bdcee958
--- /dev/null
+++ b/lldb/test/API/commands/statistics/basic/dwo_error_foo.cpp
@@ -0,0 +1,10 @@
+struct foo {
+ int x;
+ bool y;
+};
+
+void dwo_error_foo() {
+ foo f;
+ f.x = 1;
+ f.y = true;
+}
diff --git a/lldb/test/API/commands/statistics/basic/dwo_error_main.cpp b/lldb/test/API/commands/statistics/basic/dwo_error_main.cpp
new file mode 100644
index 0000000000000..4f09bd74e1fd6
--- /dev/null
+++ b/lldb/test/API/commands/statistics/basic/dwo_error_main.cpp
@@ -0,0 +1,5 @@
+void dwo_error_foo();
+int main() {
+ dwo_error_foo();
+ return 0;
+}
|
This relands changes in #155023 for adding a count of dwo errors and combine all the dwo related stats into one struct.
The previous PR was reverted in #156777 as the newly added unit test
test_dwo_id_mismatch_error_stats
sometimes fails due to inappropriate use ofglob.glob
.This change modified the tests created in the former PR to collect and modify the dwo files by there names instead of using index after
glob.glob
. This will avoid the possible failure in these tests if the order of dwo files changes.Original PR: https://github.com/llvm/llvm-project/pull/155023
Testing
Ran unit tests