llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Ziyi Wang (zw3917) <details> <summary>Changes</summary> ## Summary A new `totalDwoLoadErrorCount` counter to available statisctics when calling `statistics dump`. 1. A new function, `CountDwoLoadErrors` is created, and returns the number of DWO file loading errors. An override is implemented for `SymbolFileDWARF` that loops through each compile unit, and uses `GetDwoError` to get its dwo loading status. It the status is as `Fail`, which means there is error happened during the loading process, we increment the counter. 2. In Statistics, we sum up the total number of DWO file loading errors. This is done by getting `current_dwo_errors` for each sym_file and adding up the results for each module, then adding to the total count of errors among all modules. ## Expected Behavior - When binaries are compiled with split-dwarf and separate DWO files, `totalDwoLoadErrorCount` would be the number of dwo files with error occurs during the loading process, 0 if no error occurs during a loading process. - When not using split-dwarf, we expect `totalDwoLoadErrorCount` to be 0 since there no DWO file loading errors would be caused. ## Testing ### Manual Testing We created some files to simulate the possible DWO errors manually and observed the results generated by statistics dump. For example, if we delete one of the DWO files generated after compiling, we would get: ``` (lldb) statistics dump { ... "totalLoadedDwoFileCount": 1, } ``` We also checked the time cost of `statistics dump` w/o the modification to make sure no significant time cost increase imported. ### Unit test Added two unit tests that build with new "dwo_error_foo.cpp" and "dwo_error_main.cpp" files. For tests with flags -gsplit-dwarf, this generates 2 DWO files. In one of the tests, we delete both DWO files and check the result to see if it reflects the number of DWO files with errors correctly. In another test we update one of the files but loading the outdated .dwo file of it, expecting it increments the error count by 1. To run the test: ``` $ bin/lldb-dotest -p TestStats.py ~/local/llvm-project/lldb/test/API/commands/statistics/basic/ -G "dwo" ---------------------------------------------------------------------- Ran 27 tests in 7.439s OK (skipped=21) ``` --- Full diff: https://github.com/llvm/llvm-project/pull/155023.diff 8 Files Affected: - (modified) lldb/include/lldb/Symbol/SymbolFile.h (+4) - (modified) lldb/include/lldb/Target/Statistics.h (+1) - (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+18) - (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h (+3) - (modified) lldb/source/Target/Statistics.cpp (+6) - (modified) lldb/test/API/commands/statistics/basic/TestStats.py (+115) - (added) lldb/test/API/commands/statistics/basic/dwo_error_foo.cpp (+10) - (added) lldb/test/API/commands/statistics/basic/dwo_error_main.cpp (+5) ``````````diff diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h index bbc615d9fdc38..7f590fc2e7e64 100644 --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/lldb/include/lldb/Symbol/SymbolFile.h @@ -496,6 +496,10 @@ class SymbolFile : public PluginInterface { /// symbol file doesn't support DWO files, both counts will be 0. virtual std::pair<uint32_t, uint32_t> GetDwoFileCounts() { return {0, 0}; } + /// Calculates the count of dwo load error, return the number of dwo file with + /// errors, 0 by default. + virtual uint32_t CountDwoLoadErrors() { return 0; } + virtual lldb::TypeSP MakeType(lldb::user_id_t uid, ConstString name, std::optional<uint64_t> byte_size, SymbolContextScope *context, diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index 55dff8861a9ab..6dd09cac890b8 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -155,6 +155,7 @@ struct ModuleStats { bool debug_info_had_incomplete_types = false; uint32_t dwo_file_count = 0; uint32_t loaded_dwo_file_count = 0; + uint32_t dwo_load_error_count = 0; }; struct ConstStringStats { diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index b15e0c15fedb8..5c95200abd1bd 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -4530,3 +4530,21 @@ std::pair<uint32_t, uint32_t> SymbolFileDWARF::GetDwoFileCounts() { return {loaded_dwo_count, total_dwo_count}; } + +uint32_t SymbolFileDWARF::CountDwoLoadErrors() { + uint32_t dwo_load_error_count = 0; + + DWARFDebugInfo &info = DebugInfo(); + const size_t num_cus = info.GetNumUnits(); + for (size_t cu_idx = 0; cu_idx < num_cus; cu_idx++) { + DWARFUnit *dwarf_cu = info.GetUnitAtIndex(cu_idx); + if (dwarf_cu == nullptr) + continue; + + // Check if this unit has dwo error (False by default). + const Status &dwo_error = dwarf_cu->GetDwoError(); + if (dwo_error.Fail()) + dwo_load_error_count++; + } + return dwo_load_error_count; +} diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index d7db8a3c0869f..349ea9ef8224b 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -288,6 +288,9 @@ class SymbolFileDWARF : public SymbolFileCommon { // CUs and total DWO CUs. For non-split-dwarf files, this reports 0 for both. std::pair<uint32_t, uint32_t> GetDwoFileCounts() override; + /// Count the number of dwo load errors happened. + uint32_t CountDwoLoadErrors() override; + DWARFContext &GetDWARFContext() { return m_context; } const std::shared_ptr<SymbolFileDWARFDwo> &GetDwpSymbolFile(); diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index 909f335687b21..0f9eb5a785163 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -75,6 +75,7 @@ json::Value ModuleStats::ToJSON() const { 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("dwoLoadErrorCount", dwo_load_error_count); if (!symbol_locator_time.map.empty()) { json::Object obj; @@ -326,6 +327,7 @@ llvm::json::Value DebuggerStats::ReportStatistics( uint32_t symtab_symbol_count = 0; uint32_t total_loaded_dwo_file_count = 0; uint32_t total_dwo_file_count = 0; + uint32_t total_dwo_load_error_count = 0; for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) { Module *module = target != nullptr ? target->GetImages().GetModuleAtIndex(image_idx).get() @@ -361,6 +363,9 @@ llvm::json::Value DebuggerStats::ReportStatistics( sym_file->GetDwoFileCounts(); total_dwo_file_count += module_stat.dwo_file_count; total_loaded_dwo_file_count += module_stat.loaded_dwo_file_count; + uint32_t current_dwo_errors = sym_file->CountDwoLoadErrors(); + module_stat.dwo_load_error_count += current_dwo_errors; + total_dwo_load_error_count += current_dwo_errors; module_stat.debug_info_index_loaded_from_cache = sym_file->GetDebugInfoIndexWasLoadedFromCache(); if (module_stat.debug_info_index_loaded_from_cache) @@ -437,6 +442,7 @@ llvm::json::Value DebuggerStats::ReportStatistics( {"totalSymbolTableSymbolCount", symtab_symbol_count}, {"totalLoadedDwoFileCount", total_loaded_dwo_file_count}, {"totalDwoFileCount", total_dwo_file_count}, + {"totalDwoLoadErrorCount", total_dwo_load_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..7a2dc213ce6fd 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", + "totalDwoLoadErrorCount", ] 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", + "totalDwoLoadErrorCount", ] 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", + "totalDwoLoadErrorCount", ] self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None) @@ -385,6 +390,7 @@ def test_modules(self): "totalDebugInfoByteSize", "totalDwoFileCount", "totalLoadedDwoFileCount", + "totalDwoLoadErrorCount", ] 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", + "dwoLoadErrorCount", "triple", "uuid", ] @@ -497,6 +504,7 @@ def test_breakpoints(self): "totalDebugInfoByteSize", "totalDwoFileCount", "totalLoadedDwoFileCount", + "totalDwoLoadErrorCount", ] 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 load 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("totalDwoLoadErrorCount", debug_stats) + self.assertEqual(debug_stats["totalDwoLoadErrorCount"], 2) + + # Since there's only one module, module stats should have the same count as total count + self.assertIn("dwoLoadErrorCount", debug_stats["modules"][0]) + self.assertEqual(debug_stats["modules"][0]["dwoLoadErrorCount"], 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 load 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("totalDwoLoadErrorCount", initial_stats) + self.assertEqual(initial_stats["totalDwoLoadErrorCount"], 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("totalDwoLoadErrorCount", debug_stats) + self.assertEqual(debug_stats["totalDwoLoadErrorCount"], 1) + + # Since there's only one module, module stats should have the same count as total count + self.assertIn("dwoLoadErrorCount", debug_stats["modules"][0]) + self.assertEqual(debug_stats["modules"][0]["dwoLoadErrorCount"], 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; +} `````````` </details> https://github.com/llvm/llvm-project/pull/155023 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits