Skip to content

Conversation

zw3917
Copy link
Contributor

@zw3917 zw3917 commented Sep 4, 2025

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 of glob.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

$ ./bin/llvm-lit /data/users/ziyiww/llvm-project/lldb/test/API/commands/statistics/basic/TestStats.py
./bin/llvm-lit /data/users/ziyiww/llvm-project/lldb/test/API/commands/statistics/basic/TestStats.py -v
-- Testing: 1 tests, 1 workers --
PASS: lldb-api :: commands/statistics/basic/TestStats.py (1 of 1)

Testing Time: 388.52s

Total Discovered Tests: 1
  Passed: 1 (100.00%)

$ bin/lldb-dotest -p TestStats.py /data/users/ziyiww/llvm-project/lldb/test/API/commands/statistics/basic/
----------------------------------------------------------------------
Ran 27 tests in 386.302s

OK (skipped=3)

@zw3917 zw3917 marked this pull request as ready for review September 4, 2025 23:23
@zw3917 zw3917 requested a review from JDevlieghere as a code owner September 4, 2025 23:23
@llvmbot llvmbot added the lldb label Sep 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 4, 2025

@llvm/pr-subscribers-lldb

Author: Ziyi Wang (zw3917)

Changes

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 of glob.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

$ ./bin/llvm-lit /data/users/ziyiww/llvm-project/lldb/test/API/commands/statistics/basic/TestStats.py
./bin/llvm-lit /data/users/ziyiww/llvm-project/lldb/test/API/commands/statistics/basic/TestStats.py -v
-- Testing: 1 tests, 1 workers --
PASS: lldb-api :: commands/statistics/basic/TestStats.py (1 of 1)

Testing Time: 388.52s

Total Discovered Tests: 1
  Passed: 1 (100.00%)

$ bin/lldb-dotest -p TestStats.py /data/users/ziyiww/llvm-project/lldb/test/API/commands/statistics/basic/
----------------------------------------------------------------------
Ran 27 tests in 386.302s

OK (skipped=3)

Full diff: https://github.com/llvm/llvm-project/pull/156980.diff

8 Files Affected:

  • (modified) lldb/include/lldb/Symbol/SymbolFile.h (+8-5)
  • (modified) lldb/include/lldb/Target/Statistics.h (+14-2)
  • (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+10-6)
  • (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h (+5-4)
  • (modified) lldb/source/Target/Statistics.cpp (+10-10)
  • (modified) lldb/test/API/commands/statistics/basic/TestStats.py (+135)
  • (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 --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;
+}

@zw3917
Copy link
Contributor Author

zw3917 commented Sep 4, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants