From b8d2944b83f2aa4b6c4a2ea0265640347668cf4a Mon Sep 17 00:00:00 2001 From: Ziyi Wang Date: Thu, 4 Sep 2025 14:53:38 -0700 Subject: [PATCH 1/5] Reland[lldb] Add count for errors of DWO files in statistics and combine DWO file count functions --- lldb/include/lldb/Symbol/SymbolFile.h | 13 +- lldb/include/lldb/Target/Statistics.h | 16 ++- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 16 ++- .../SymbolFile/DWARF/SymbolFileDWARF.h | 9 +- lldb/source/Target/Statistics.cpp | 20 +-- .../commands/statistics/basic/TestStats.py | 115 ++++++++++++++++++ .../statistics/basic/dwo_error_foo.cpp | 10 ++ .../statistics/basic/dwo_error_main.cpp | 5 + 8 files changed, 177 insertions(+), 27 deletions(-) create mode 100644 lldb/test/API/commands/statistics/basic/dwo_error_foo.cpp create mode 100644 lldb/test/API/commands/statistics/basic/dwo_error_main.cpp 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 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 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 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 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..670c675cbd648 100644 --- a/lldb/test/API/commands/statistics/basic/TestStats.py +++ b/lldb/test/API/commands/statistics/basic/TestStats.py @@ -1,6 +1,8 @@ +import glob import json import os import re +import shutil import lldb from lldbsuite.test.decorators import * @@ -179,6 +181,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 +294,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 +335,7 @@ def test_memory(self): "totalDebugInfoByteSize", "totalDwoFileCount", "totalLoadedDwoFileCount", + "totalDwoErrorCount", ] self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None) @@ -385,6 +390,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 +413,7 @@ def test_modules(self): "symbolTableSavedToCache", "dwoFileCount", "loadedDwoFileCount", + "dwoErrorCount", "triple", "uuid", ] @@ -497,6 +504,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 +663,113 @@ 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") + + # Remove the two .dwo files to trigger a DWO load error + dwo_files = glob.glob(self.getBuildArtifact("*.dwo")) + for dwo_file in 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 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") + + # Find and make a backup of the original .dwo file + dwo_files = glob.glob(self.getBuildArtifact("*.dwo")) + + original_dwo_file = dwo_files[1] + 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 run to a breakpoint to force DWO file loading + 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; +} From 9de5fe3b5a598ba1f587f48ca58b32b833d837c2 Mon Sep 17 00:00:00 2001 From: Ziyi Wang Date: Thu, 4 Sep 2025 14:56:48 -0700 Subject: [PATCH 2/5] Fix the potential inconsisitency issue in test_dwo_id_mismatch_error_stats --- .../commands/statistics/basic/TestStats.py | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py index 670c675cbd648..c38a40ce8e560 100644 --- a/lldb/test/API/commands/statistics/basic/TestStats.py +++ b/lldb/test/API/commands/statistics/basic/TestStats.py @@ -1,4 +1,3 @@ -import glob import json import os import re @@ -682,9 +681,20 @@ def test_dwo_missing_error_stats(self): self.addTearDownCleanup(dictionary=da) exe = self.getBuildArtifact("a.out") - # Remove the two .dwo files to trigger a DWO load error - dwo_files = glob.glob(self.getBuildArtifact("*.dwo")) - for dwo_file in dwo_files: + 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) @@ -699,7 +709,7 @@ def test_dwo_missing_error_stats(self): self.assertEqual(debug_stats["modules"][0]["dwoErrorCount"], 2) # Restore the original .dwo file - for dwo_file in dwo_files: + for dwo_file in expected_dwo_files: os.rename(dwo_file + ".bak", dwo_file) @add_test_categories(["dwo"]) @@ -723,10 +733,20 @@ def test_dwo_id_mismatch_error_stats(self): self.addTearDownCleanup(dictionary=da) exe = self.getBuildArtifact("a.out") - # Find and make a backup of the original .dwo file - dwo_files = glob.glob(self.getBuildArtifact("*.dwo")) + 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}", + ) - original_dwo_file = dwo_files[1] + # 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) @@ -749,7 +769,7 @@ def test_dwo_id_mismatch_error_stats(self): self.build(dictionary=da, debug_info="dwo") shutil.copy2(original_dwo_backup, original_dwo_file) - # Create a new target and run to a breakpoint to force DWO file loading + # Create a new target and get stats target = self.createTestTarget(file_path=exe) debug_stats = self.get_stats() From f369db39e2c6a240a9d31c445ebc2ca309cdeb89 Mon Sep 17 00:00:00 2001 From: Ziyi Wang Date: Mon, 8 Sep 2025 17:08:09 -0700 Subject: [PATCH 3/5] Modify dwo error tests --- lldb/include/lldb/Target/Statistics.h | 16 ++-- lldb/source/Target/Statistics.cpp | 4 +- .../commands/statistics/basic/TestStats.py | 83 ++++++------------- 3 files changed, 37 insertions(+), 66 deletions(-) diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index c288917edbc6a..b4dc6325f9497 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -110,7 +110,7 @@ class StatisticsMap { llvm::StringMap map; }; -/// A class to count success/fail statistics. +/// A class to count success/fail statistics.ß struct StatsSuccessFail { StatsSuccessFail(llvm::StringRef n) : name(n.str()) {} @@ -129,10 +129,16 @@ struct DWOStats { 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}; + DWOStats& operator+=(const DWOStats &rhs) { + loaded_dwo_file_count += rhs.loaded_dwo_file_count; + dwo_file_count += rhs.dwo_file_count; + dwo_error_count += rhs.dwo_error_count; + return *this; + } + + friend DWOStats operator+(DWOStats lhs, const DWOStats &rhs) { + lhs += rhs; + return lhs; } }; diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index 3c16df5440f25..8ad8d507268e2 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -358,8 +358,8 @@ llvm::json::Value DebuggerStats::ReportStatistics( module_stat.symfile_modules.push_back((intptr_t)symbol_module.get()); } 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.dwo_stats += current_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) diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py index c38a40ce8e560..c8527abf3c84e 100644 --- a/lldb/test/API/commands/statistics/basic/TestStats.py +++ b/lldb/test/API/commands/statistics/basic/TestStats.py @@ -668,8 +668,8 @@ 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 + 2) Delete one of the two .dwo files + 3) Verify that 1 DWO error is reported in statistics """ da = { "CXX_SOURCES": "dwo_error_main.cpp dwo_error_foo.cpp", @@ -678,7 +678,6 @@ def test_dwo_missing_error_stats(self): # -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 = [ @@ -693,24 +692,20 @@ def test_dwo_missing_error_stats(self): 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") + # Remove one of .dwo files to trigger DWO load error + dwo_main_file = self.getBuildArtifact("dwo_error_main.dwo") + os.remove(dwo_main_file) 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) + 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"], 2) - - # Restore the original .dwo file - for dwo_file in expected_dwo_files: - os.rename(dwo_file + ".bak", dwo_file) + self.assertEqual(debug_stats["modules"][0]["dwoErrorCount"], 1) @add_test_categories(["dwo"]) def test_dwo_id_mismatch_error_stats(self): @@ -718,10 +713,8 @@ 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 + 2) Replace one of the .dwo files with a mismatched one to cause a DWO ID mismatch error + 3) Verifies that a DWO error is reported in statistics """ da = { "CXX_SOURCES": "dwo_error_main.cpp dwo_error_foo.cpp", @@ -730,7 +723,6 @@ def test_dwo_id_mismatch_error_stats(self): # -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 = [ @@ -745,50 +737,23 @@ def test_dwo_id_mismatch_error_stats(self): 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) + # Replace one of the original .dwo file content with another one to trigger DWO ID mismatch error + dwo_foo_file = self.getBuildArtifact("dwo_error_foo.dwo") + dwo_main_file = self.getBuildArtifact("dwo_error_main.dwo") + + shutil.copy(dwo_main_file, dwo_foo_file) + # Create a new target and get stats 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) + 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) @skipUnlessDarwin @no_debug_info_test From de455165ca39a55e9f0bd7cf1e5da36186dcc25e Mon Sep 17 00:00:00 2001 From: Ziyi Wang Date: Mon, 8 Sep 2025 17:08:09 -0700 Subject: [PATCH 4/5] Modify dwo error tests --- lldb/include/lldb/Target/Statistics.h | 14 +++- lldb/source/Target/Statistics.cpp | 4 +- .../commands/statistics/basic/TestStats.py | 83 ++++++------------- 3 files changed, 36 insertions(+), 65 deletions(-) diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index c288917edbc6a..22b2a17c851eb 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -129,10 +129,16 @@ struct DWOStats { 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}; + DWOStats& operator+=(const DWOStats &rhs) { + loaded_dwo_file_count += rhs.loaded_dwo_file_count; + dwo_file_count += rhs.dwo_file_count; + dwo_error_count += rhs.dwo_error_count; + return *this; + } + + friend DWOStats operator+(DWOStats lhs, const DWOStats &rhs) { + lhs += rhs; + return lhs; } }; diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index 3c16df5440f25..8ad8d507268e2 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -358,8 +358,8 @@ llvm::json::Value DebuggerStats::ReportStatistics( module_stat.symfile_modules.push_back((intptr_t)symbol_module.get()); } 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.dwo_stats += current_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) diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py index c38a40ce8e560..c8527abf3c84e 100644 --- a/lldb/test/API/commands/statistics/basic/TestStats.py +++ b/lldb/test/API/commands/statistics/basic/TestStats.py @@ -668,8 +668,8 @@ 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 + 2) Delete one of the two .dwo files + 3) Verify that 1 DWO error is reported in statistics """ da = { "CXX_SOURCES": "dwo_error_main.cpp dwo_error_foo.cpp", @@ -678,7 +678,6 @@ def test_dwo_missing_error_stats(self): # -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 = [ @@ -693,24 +692,20 @@ def test_dwo_missing_error_stats(self): 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") + # Remove one of .dwo files to trigger DWO load error + dwo_main_file = self.getBuildArtifact("dwo_error_main.dwo") + os.remove(dwo_main_file) 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) + 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"], 2) - - # Restore the original .dwo file - for dwo_file in expected_dwo_files: - os.rename(dwo_file + ".bak", dwo_file) + self.assertEqual(debug_stats["modules"][0]["dwoErrorCount"], 1) @add_test_categories(["dwo"]) def test_dwo_id_mismatch_error_stats(self): @@ -718,10 +713,8 @@ 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 + 2) Replace one of the .dwo files with a mismatched one to cause a DWO ID mismatch error + 3) Verifies that a DWO error is reported in statistics """ da = { "CXX_SOURCES": "dwo_error_main.cpp dwo_error_foo.cpp", @@ -730,7 +723,6 @@ def test_dwo_id_mismatch_error_stats(self): # -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 = [ @@ -745,50 +737,23 @@ def test_dwo_id_mismatch_error_stats(self): 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) + # Replace one of the original .dwo file content with another one to trigger DWO ID mismatch error + dwo_foo_file = self.getBuildArtifact("dwo_error_foo.dwo") + dwo_main_file = self.getBuildArtifact("dwo_error_main.dwo") + + shutil.copy(dwo_main_file, dwo_foo_file) + # Create a new target and get stats 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) + 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) @skipUnlessDarwin @no_debug_info_test From bdfead97b71f4511a05135dfffd4d20353e0952a Mon Sep 17 00:00:00 2001 From: Ziyi Wang Date: Mon, 8 Sep 2025 17:30:17 -0700 Subject: [PATCH 5/5] Fix format --- lldb/include/lldb/Target/Statistics.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index b4dc6325f9497..d6983bb0b9d24 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -110,7 +110,7 @@ class StatisticsMap { llvm::StringMap map; }; -/// A class to count success/fail statistics.ß +/// A class to count success/fail statistics. struct StatsSuccessFail { StatsSuccessFail(llvm::StringRef n) : name(n.str()) {} @@ -129,7 +129,7 @@ struct DWOStats { uint32_t dwo_file_count = 0; uint32_t dwo_error_count = 0; - DWOStats& operator+=(const DWOStats &rhs) { + DWOStats &operator+=(const DWOStats &rhs) { loaded_dwo_file_count += rhs.loaded_dwo_file_count; dwo_file_count += rhs.dwo_file_count; dwo_error_count += rhs.dwo_error_count;