[lldb] Add count for errors of DWO files in statistics and combine DWO file count functions #155023
+202
−51
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-lldb Author: Ziyi Wang (zw3917) ChangesSummaryA new
Expected Behavior
TestingManual TestingWe created some files to simulate the possible DWO errors manually and observed the results generated by statistics dump. We also checked the time cost of Unit testAdded 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. Full diff: https://github.com/llvm/llvm-project/pull/155023.diff 8 Files Affected:
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;
+}
|
clayborg
requested changes
Aug 22, 2025
Comment on lines
499
to
502
| /// Calculates the count of dwo load error, return the number of dwo file with | ||
| /// errors, 0 by default. | ||
| virtual uint32_t CountDwoLoadErrors() { return 0; } | ||
|
|
There was a problem hiding this comment.
Might be best to modify the GetDwoFileCounts function to return a simple struct and rename it:
struct DWOStats {
uint32_t loaded_dwo_file_count = 0;
uint32_t dwo_file_count = 0;
uint32_t dwo_error_count = 0;
};
virtual DWOStats GetDwoStats() { return {}; }
This will replace both GetDwoFileCounts() and CountDwoLoadErrors()
There was a problem hiding this comment.
Thanks for pointing this out! Just updated the implementation to use the new structure.
clayborg
requested changes
Aug 28, 2025
Comment on lines
156
to
158
| uint32_t dwo_file_count = 0; | ||
| uint32_t loaded_dwo_file_count = 0; | ||
| uint32_t dwo_error_count = 0; |
There was a problem hiding this comment.
We can change these three lines to:
DWOStats dwo_stats;
clayborg
requested changes
Aug 29, 2025
There was a problem hiding this comment.
One more round of changes just removing any auto clang-format changes that are not part of this PR. When committing PRs to upstream, we want to make sure any noise that isn't related to the PR is not part of the PR itself like whitespace and re-indentation formatting fixes that are not on the lines that you have modified. You might need to disable the auto clang format stuff and then update this PR. This should be good to go after those non-related changes are removed.
Comment on lines
-499
to
-506
| virtual lldb::TypeSP | ||
| MakeType(lldb::user_id_t uid, ConstString name, | ||
| std::optional<uint64_t> byte_size, SymbolContextScope *context, | ||
| lldb::user_id_t encoding_uid, | ||
| Type::EncodingDataType encoding_uid_type, const Declaration &decl, | ||
| const CompilerType &compiler_qual_type, | ||
| Type::ResolveState compiler_type_resolve_state, | ||
| uint32_t opaque_payload = 0) = 0; |
There was a problem hiding this comment.
I would be a good idea to turn off auto clang-format for this file and revert the changed lines that aren't a part of this commit. All of the changes to MakeType below should be reverted. The only way to do this is to turn off the auto clang format feature.
Comment on lines
-600
to
+604
| m_debug_info_had_variable_errors = true; | ||
| m_debug_info_had_variable_errors = true; |
There was a problem hiding this comment.
Ditto (revert auto clang-format changes for stuff that isn't associated with this PR).
Comment on lines
-615
to
+623
| lldb::TypeSP type_sp (new Type( | ||
| uid, this, name, byte_size, context, encoding_uid, | ||
| encoding_uid_type, decl, compiler_qual_type, | ||
| compiler_type_resolve_state, opaque_payload)); | ||
| m_type_list.Insert(type_sp); | ||
| return type_sp; | ||
| lldb::TypeSP type_sp(new Type( | ||
| uid, this, name, byte_size, context, encoding_uid, encoding_uid_type, | ||
| decl, compiler_qual_type, compiler_type_resolve_state, opaque_payload)); | ||
| m_type_list.Insert(type_sp); | ||
| return type_sp; |
There was a problem hiding this comment.
Ditto (revert auto clang-format changes for stuff that isn't associated with this PR).
Comment on lines
-624
to
+632
| // Make sure the real symbol file matches when copying types. | ||
| if (GetBackingSymbolFile() != other_type->GetSymbolFile()) | ||
| // Make sure the real symbol file matches when copying types. | ||
| if (GetBackingSymbolFile() != other_type->GetSymbolFile()) | ||
| return lldb::TypeSP(); | ||
| lldb::TypeSP type_sp(new Type(*other_type)); | ||
| m_type_list.Insert(type_sp); | ||
| return type_sp; | ||
| lldb::TypeSP type_sp(new Type(*other_type)); | ||
| m_type_list.Insert(type_sp); | ||
| return type_sp; |
There was a problem hiding this comment.
Ditto (revert auto clang-format changes for stuff that isn't associated with this PR).
| //#define ENABLE_DEBUG_PRINTF // COMMENT OUT THIS LINE PRIOR TO CHECKIN | ||
| // #define ENABLE_DEBUG_PRINTF // COMMENT OUT THIS LINE PRIOR TO CHECKIN |
There was a problem hiding this comment.
Ditto (revert auto clang-format changes for stuff that isn't associated with this PR).
| // We have a SymbolFileDWARFDebugMap, so let it find the right file | ||
| // We have a SymbolFileDWARFDebugMap, so let it find the right file |
There was a problem hiding this comment.
Ditto (revert auto clang-format changes for stuff that isn't associated with this PR).
Comment on lines
-1728
to
1730
| return DebugInfo().GetUnitAtIndex(*die_ref.file_index()) | ||
| return DebugInfo() | ||
| .GetUnitAtIndex(*die_ref.file_index()) | ||
| ->GetDwoSymbolFile(); // DWO case |
There was a problem hiding this comment.
Ditto (revert auto clang-format changes for stuff that isn't associated with this PR).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
A new
totalDwoErrorCountcounter is available in statistics when callingstatistics dumpto track the number of DWO errors.Additionally, this PR refactors the DWO file statistics by consolidating the existing functionality for counting loaded and total DWO files together with the number of DWO errors into a single function that returns a new DWOStats struct.
DWOStatsis created to hold the number of loaded DWO files, the total number of DWO files and the number of DWO errors.GetDwoFileCountsfunction for loaded and total DWO file counts with a singleGetDwoStats()function returning the structDWOStats. An override is implemented forSymbolFileDWARFthat computes the new DWO error count alongside existing counts in one pass. If the status of a DWO CU isFail, which means there is error happened during the loading process, we increment the DWO error counter.Note: The newly created function
GetDwoStatswill only be called when we try to get statistics. Other codepaths will not be affected.DWOStatsfor each symbol file and adding up the results for each module, then adding to the total count among all modules.Expected Behavior
When binaries are compiled with split-dwarf and separate DWO files,
totalDwoLoadErrorCountwould 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
totalDwoLoadErrorCountto be 0 since there no DWO file loading errors would be caused.totalLoadedDwoFileCountandtotalDwoFileCountshould be correctly calculated after refactoring and updating.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:
We also checked the time cost of
statistics dumpw/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: