Modify dwarf verification JSON to include detailed counts by sub-category #128018
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
|
@llvm/pr-subscribers-debuginfo Author: None (youngd007) ChangesDetails: Test: Full diff: https://github.com/llvm/llvm-project/pull/128018.diff 4 Files Affected:
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index 7b51bb63cd15b..6e79619e156cb 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -30,9 +30,15 @@ class DWARFDebugAbbrev;
class DataExtractor;
struct DWARFSection;
+struct AggregationData {
+ unsigned OverallCount;
+ std::map<std::string, unsigned> DetailedCounts;
+ AggregationData() = default;
+};
+
class OutputCategoryAggregator {
private:
- std::map<std::string, unsigned> Aggregation;
+ std::map<std::string, AggregationData> Aggregation;
bool IncludeDetail;
public:
@@ -40,8 +46,13 @@ class OutputCategoryAggregator {
: IncludeDetail(includeDetail) {}
void ShowDetail(bool showDetail) { IncludeDetail = showDetail; }
size_t GetNumCategories() const { return Aggregation.size(); }
- void Report(StringRef s, std::function<void()> detailCallback);
+ void Report(StringRef category, std::function<void()> detailCallback);
+ void Report(StringRef category, StringRef sub_category,
+ std::function<void()> detailCallback);
void EnumerateResults(std::function<void(StringRef, unsigned)> handleCounts);
+ void EnumerateDetailedResultsFor(
+ StringRef category,
+ std::function<void(StringRef, unsigned)> handleCounts);
};
/// A class that verifies DWARF debug information given a DWARF Context.
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 107e79cc5a05a..211d736206563 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -1941,12 +1941,14 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness(
if (none_of(NI.equal_range(Name), [&](const DWARFDebugNames::Entry &E) {
return E.getDIEUnitOffset() == DieUnitOffset;
})) {
- ErrorCategory.Report("Name Index DIE entry missing name", [&]() {
- error() << formatv(
- "Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with "
- "name {3} missing.\n",
- NI.getUnitOffset(), Die.getOffset(), Die.getTag(), Name);
- });
+ ErrorCategory.Report(
+ "Name Index DIE entry missing name",
+ llvm::dwarf::TagString(Die.getTag()), [&]() {
+ error() << formatv(
+ "Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with "
+ "name {3} missing.\n",
+ NI.getUnitOffset(), Die.getOffset(), Die.getTag(), Name);
+ });
++NumErrors;
}
}
@@ -2168,15 +2170,35 @@ bool DWARFVerifier::verifyDebugStrOffsets(
void OutputCategoryAggregator::Report(
StringRef s, std::function<void(void)> detailCallback) {
- Aggregation[std::string(s)]++;
+ this->Report(s, "", detailCallback);
+}
+
+void OutputCategoryAggregator::Report(
+ StringRef category, StringRef sub_category,
+ std::function<void(void)> detailCallback) {
+ std::string category_str = std::string(category);
+ AggregationData *Agg = &Aggregation[category_str];
+ Agg->OverallCount++;
+ if (!sub_category.empty()) {
+ Agg->DetailedCounts[std::string(sub_category)]++;
+ }
if (IncludeDetail)
detailCallback();
}
void OutputCategoryAggregator::EnumerateResults(
std::function<void(StringRef, unsigned)> handleCounts) {
- for (auto &&[name, count] : Aggregation) {
- handleCounts(name, count);
+ for (auto &&[name, aggData] : Aggregation) {
+ handleCounts(name, aggData.OverallCount);
+ }
+}
+void OutputCategoryAggregator::EnumerateDetailedResultsFor(
+ StringRef category, std::function<void(StringRef, unsigned)> handleCounts) {
+ auto Agg = Aggregation.find(std::string(category));
+ if (Agg != Aggregation.end()) {
+ for (auto &&[name, count] : Agg->second.DetailedCounts) {
+ handleCounts(name, count);
+ }
}
}
@@ -2203,6 +2225,12 @@ void DWARFVerifier::summarize() {
ErrorCategory.EnumerateResults([&](StringRef Category, unsigned Count) {
llvm::json::Object Val;
Val.try_emplace("count", Count);
+ llvm::json::Object Details;
+ ErrorCategory.EnumerateDetailedResultsFor(
+ Category, [&](StringRef SubCategory, unsigned SubCount) {
+ Details.try_emplace(SubCategory, SubCount);
+ });
+ Val.try_emplace("details", std::move(Details));
Categories.try_emplace(Category, std::move(Val));
ErrorCount += Count;
});
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-completeness-json-output.s b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-completeness-json-output.s
index c4b7ffe1d2d8e..9c9658308c2f9 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-completeness-json-output.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-completeness-json-output.s
@@ -1,7 +1,7 @@
# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj -o - | not llvm-dwarfdump -verify --verify-json=%t.json -
# RUN: FileCheck %s --input-file %t.json
-# CHECK: {"error-categories":{"Name Index DIE entry missing name":{"count":10}},"error-count":10}
+# CHECK: {"error-categories":{"Name Index DIE entry missing name":{"count":10,"details":{"DW_TAG_inlined_subroutine":1,"DW_TAG_label":1,"DW_TAG_namespace":2,"DW_TAG_subprogram":2,"DW_TAG_variable":4}}},"error-count":10}
# CHECK-NOT: error: Name Index @ 0x0: Entry for DIE @ {{.*}} (DW_TAG_variable) with name var_block_addr missing.
.section .debug_loc,"",@progbits
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-cu-lists-json-output.s b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-cu-lists-json-output.s
index dfbfe04943a83..872fff801576c 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-cu-lists-json-output.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-cu-lists-json-output.s
@@ -2,7 +2,7 @@
# RUN: not llvm-dwarfdump -verify -verify-json=%t.json -
# RUN: FileCheck %s --input-file %t.json
-# CHECK: {"error-categories":{"Duplicate Name Index":{"count":1},"Name Index doesn't index any CU":{"count":1},"Name Index references non-existing CU":{"count":1}},"error-count":3}
+# CHECK: {"error-categories":{"Duplicate Name Index":{"count":1,"details":{}},"Name Index doesn't index any CU":{"count":1,"details":{}},"Name Index references non-existing CU":{"count":1,"details":{}}},"error-count":3}
# CHECK-NOT : error: Name Index @ 0x58 references a CU @ 0x0, but this CU is already indexed by Name Index @ 0x28
# CHECK-NOT: warning: CU @ 0x13 not covered by any Name Index
|
|
This is a redo of #125062 that was reverted in Icohedron@3c1b8aa |
|
The original was failing on |
mbucko
requested changes
Feb 25, 2025
| struct AggregationData { | ||
| unsigned OverallCount; | ||
| std::map<std::string, unsigned> DetailedCounts; | ||
| AggregationData() = default; |
There was a problem hiding this comment.
I can double check, but the original PR would not build or run without the constructor defined.
| if (IncludeDetail) | ||
| detailCallback(); | ||
| } | ||
|
|
||
| void OutputCategoryAggregator::EnumerateResults( | ||
| std::function<void(StringRef, unsigned)> handleCounts) { | ||
| for (auto &&[name, count] : Aggregation) { | ||
| handleCounts(name, count); | ||
| for (auto &&[name, aggData] : Aggregation) { |
There was a problem hiding this comment.
I remember you mentioning that before original PR and when I tried, it did not build.
There was a problem hiding this comment.
Is there any benefit of && here?
| StringRef category, std::function<void(StringRef, unsigned)> handleCounts) { | ||
| auto Agg = Aggregation.find(std::string(category)); | ||
| if (Agg != Aggregation.end()) { | ||
| for (auto &&[name, count] : Agg->second.DetailedCounts) { |
| } | ||
| void OutputCategoryAggregator::EnumerateDetailedResultsFor( | ||
| StringRef category, std::function<void(StringRef, unsigned)> handleCounts) { | ||
| auto Agg = Aggregation.find(std::string(category)); |
There was a problem hiding this comment.
const on which part? All three (two params and the Agg?)?
ayermolo
reviewed
Feb 25, 2025
| StringRef category, StringRef sub_category, | ||
| std::function<void(void)> detailCallback) { | ||
| std::string category_str = std::string(category); | ||
| AggregationData *Agg = &Aggregation[category_str]; |
There was a problem hiding this comment.
Why is it a pointer?
Can you just do:
AggregationData &Agg = Aggregation[category_str];
…gory Details: To help make better use of dwarfdump verification for identifying and fixing issues with debug information, the JSON will now emit details (sub-categories) where relevant. First modification concerns missing tags as those were recently missing for BOLT debug names. Test: test files for JSON output were previously added, so modify here to expect the new JSON keys. One test has sub-categories and another is empty. ninja check-llvm-tools-llvm-dwarfdump Also build the tool and run with a local executable to verify. ninja llvm-dwarfdump
mbucko
reviewed
Feb 27, 2025
clayborg
approved these changes
Feb 28, 2025
ayermolo
approved these changes
Feb 28, 2025
mbucko
approved these changes
Mar 4, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/8741 Here is the relevant piece of the build log for the reference |
jph-13
pushed a commit
to jph-13/llvm-project
that referenced
this pull request
Mar 21, 2025
…gory (llvm#128018) To help make better use of dwarfdump verification for identifying and fixing issues with debug information, the JSON will now emit details (sub-categories) where relevant. First modification concerns missing tags as those were recently missing for BOLT debug names. Test: test files for JSON output were previously added, so modify here to expect the new JSON keys. One test has sub-categories and another is empty. ninja check-llvm-tools-llvm-dwarfdump Also build the tool and run with a local executable to verify. ninja llvm-dwarfdump
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.
Details:
To help make better use of dwarfdump verification for identifying and fixing issues with debug information, the JSON will now emit details (sub-categories) where relevant. First modification concerns missing tags as those were recently missing for BOLT debug names.
Test:
test files for JSON output were previously added, so modify here to expect the new JSON keys. One test has sub-categories and another is empty.
ninja check-llvm-tools-llvm-dwarfdump
Also build the tool and run with a local executable to verify.
ninja llvm-dwarfdump