[llvm] Aggregate errors from llvm-dwarfutil --verify (PR #79648)

Kevin Frei via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 26 13:13:31 PST 2024


https://github.com/kevinfrei created https://github.com/llvm/llvm-project/pull/79648

The amount and format of output from `llvm-dwarfutil --verify` makes it quite difficult to know if a change to a tool that produces or modifies DWARF is causing new problems, or is fixing existing problems. This diff adds a categorized summary of issues found by the DWARF verifier, on by default, at the bottom of the error output. 

The change includes two new options: `--only-aggregate-errors` and `--no-aggregate-errors` both of which default to 'off'. The aggregated error count is, by default, displayed at the bottom of the verification output (this is a change to previous output from the tool). If you want the previous output, use `--no-aggregate-errors`. If you *only* want the aggregated information, use `--only-aggregate-errors`.

I changed a handful of tests that were failing due to new output, adding the flag to use the old behavior for all but a couple. For those two I added the new aggregated output to the expected output of the test.

The `OutputCategoryAggregator` is a pretty simple little class that @clayborg suggested to allow code to only be run to dump detail if it's enabled, while still collating counts of the category. Knowing that the lambda passed in is only conditionally executed is pretty important (handling errors has to be done *outside* the lambda). I'm happy to move this somewhere else (and change/improve it) to be more broadly useful if folks would like.

>From 39c4331198a6a3368c6e8c9c364c729791efdfb6 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Wed, 24 Jan 2024 15:23:09 -0800
Subject: [PATCH 01/11] Not formatted, mostly to keep the current model easy to
 see

---
 llvm/include/llvm/DebugInfo/DIContext.h       |   1 +
 .../llvm/DebugInfo/DWARF/DWARFVerifier.h      |  20 +++
 llvm/lib/DebugInfo/DWARF/DWARFContext.cpp     |   1 +
 llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp    | 151 +++++++++++++-----
 4 files changed, 129 insertions(+), 44 deletions(-)

diff --git a/llvm/include/llvm/DebugInfo/DIContext.h b/llvm/include/llvm/DebugInfo/DIContext.h
index 78ac34e5f0d26c..298628b551fb68 100644
--- a/llvm/include/llvm/DebugInfo/DIContext.h
+++ b/llvm/include/llvm/DebugInfo/DIContext.h
@@ -205,6 +205,7 @@ struct DIDumpOptions {
   bool DisplayRawContents = false;
   bool IsEH = false;
   bool DumpNonSkeleton = false;
+  bool DumpAggregateErrors = true;
   std::function<llvm::StringRef(uint64_t DwarfRegNum, bool IsEH)>
       GetNameForDWARFReg;
 
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index e56d3781e824f3..b9a68217aebfce 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -77,8 +77,23 @@ class DWARFVerifier {
   };
 
 private:
+  class ErrorAggregator {
+  private:
+    DWARFVerifier &Verifier;
+    std::map<std::string, int> UniqueErrors;
+    static std::string Clean(const char *s);
+
+  public:
+    ErrorAggregator(DWARFVerifier &v) : Verifier(v) {}
+    raw_ostream &operator<<(const char *s);
+    void Collect(const std::string &s);
+    void Dump();
+  };
+  friend class ErrorAggregator;
+
   raw_ostream &OS;
   DWARFContext &DCtx;
+  ErrorAggregator ErrAggregation;
   DIDumpOptions DumpOpts;
   uint32_t NumDebugLineErrors = 0;
   // Used to relax some checks that do not currently work portably
@@ -86,6 +101,8 @@ class DWARFVerifier {
   bool IsMachOObject;
   using ReferenceMap = std::map<uint64_t, std::set<uint64_t>>;
 
+  raw_ostream &aggregate(const char *);
+  ErrorAggregator &aggregate();
   raw_ostream &error() const;
   raw_ostream &warn() const;
   raw_ostream &note() const;
@@ -348,6 +365,9 @@ class DWARFVerifier {
   bool verifyDebugStrOffsets(
       StringRef SectionName, const DWARFSection &Section, StringRef StrData,
       void (DWARFObject::*)(function_ref<void(const DWARFSection &)>) const);
+
+  /// Emits any aggregate information collection, depending on the dump options
+  void finish();
 };
 
 static inline bool operator<(const DWARFVerifier::DieRangeInfo &LHS,
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index 792df53d304aa7..aa294444b46803 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -1408,6 +1408,7 @@ bool DWARFContext::verify(raw_ostream &OS, DIDumpOptions DumpOpts) {
   if (DumpOpts.DumpType & DIDT_DebugStrOffsets)
     Success &= verifier.handleDebugStrOffsets();
   Success &= verifier.handleAccelTables();
+  verifier.finish();
   return Success;
 }
 
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index c4c14f5e2c9d36..e43f01fad3ce02 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -240,20 +240,20 @@ unsigned DWARFVerifier::verifyUnitContents(DWARFUnit &Unit,
 
   DWARFDie Die = Unit.getUnitDIE(/* ExtractUnitDIEOnly = */ false);
   if (!Die) {
-    error() << "Compilation unit without DIE.\n";
+    aggregate() << "Compilation unit without DIE.\n";
     NumUnitErrors++;
     return NumUnitErrors;
   }
 
   if (!dwarf::isUnitType(Die.getTag())) {
-    error() << "Compilation unit root DIE is not a unit DIE: "
+    aggregate() << "Compilation unit root DIE is not a unit DIE: "
             << dwarf::TagString(Die.getTag()) << ".\n";
     NumUnitErrors++;
   }
 
   uint8_t UnitType = Unit.getUnitType();
   if (!DWARFUnit::isMatchingUnitTypeAndTag(UnitType, Die.getTag())) {
-    error() << "Compilation unit type (" << dwarf::UnitTypeString(UnitType)
+    aggregate("Mismatched unit type") << "Compilation unit type (" << dwarf::UnitTypeString(UnitType)
             << ") and root DIE (" << dwarf::TagString(Die.getTag())
             << ") do not match.\n";
     NumUnitErrors++;
@@ -263,7 +263,7 @@ unsigned DWARFVerifier::verifyUnitContents(DWARFUnit &Unit,
   //  3.1.2 Skeleton Compilation Unit Entries:
   //  "A skeleton compilation unit has no children."
   if (Die.getTag() == dwarf::DW_TAG_skeleton_unit && Die.hasChildren()) {
-    error() << "Skeleton compilation unit has children.\n";
+    aggregate() << "Skeleton compilation unit has children.\n";
     NumUnitErrors++;
   }
 
@@ -280,14 +280,14 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) {
   DWARFDie Curr = Die.getParent();
   for (; Curr.isValid() && !Curr.isSubprogramDIE(); Curr = Die.getParent()) {
     if (Curr.getTag() == DW_TAG_inlined_subroutine) {
-      error() << "Call site entry nested within inlined subroutine:";
+      aggregate() << "Call site entry nested within inlined subroutine:";
       Curr.dump(OS);
       return 1;
     }
   }
 
   if (!Curr.isValid()) {
-    error() << "Call site entry not nested within a valid subprogram:";
+    aggregate() << "Call site entry not nested within a valid subprogram:";
     Die.dump(OS);
     return 1;
   }
@@ -297,7 +297,7 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) {
        DW_AT_call_all_tail_calls, DW_AT_GNU_all_call_sites,
        DW_AT_GNU_all_source_call_sites, DW_AT_GNU_all_tail_call_sites});
   if (!CallAttr) {
-    error() << "Subprogram with call site entry has no DW_AT_call attribute:";
+    aggregate() << "Subprogram with call site entry has no DW_AT_call attribute:";
     Curr.dump(OS);
     Die.dump(OS, /*indent*/ 1);
     return 1;
@@ -313,7 +313,7 @@ unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) {
   Expected<const DWARFAbbreviationDeclarationSet *> AbbrDeclsOrErr =
       Abbrev->getAbbreviationDeclarationSet(0);
   if (!AbbrDeclsOrErr) {
-    error() << toString(AbbrDeclsOrErr.takeError()) << "\n";
+    aggregate("Abbreviateion Declaration error") << toString(AbbrDeclsOrErr.takeError()) << "\n";
     return 1;
   }
 
@@ -324,7 +324,7 @@ unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) {
     for (auto Attribute : AbbrDecl.attributes()) {
       auto Result = AttributeSet.insert(Attribute.Attr);
       if (!Result.second) {
-        error() << "Abbreviation declaration contains multiple "
+        aggregate("Abbreviation declartion contains multiple attributes") << "Abbreviation declaration contains multiple "
                 << AttributeString(Attribute.Attr) << " attributes.\n";
         AbbrDecl.dump(OS);
         ++NumErrors;
@@ -440,7 +440,7 @@ unsigned DWARFVerifier::verifyIndex(StringRef Name,
       auto &M = *Sections[Col];
       auto I = M.find(SC.getOffset());
       if (I != M.end() && I.start() < (SC.getOffset() + SC.getLength())) {
-        error() << llvm::formatv(
+        aggregate("Overlapping index entries") << llvm::formatv(
             "overlapping index entries for entries {0:x16} "
             "and {1:x16} for column {2}\n",
             *I, Sig, toString(Index.getColumnKinds()[Col]));
@@ -532,7 +532,7 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die,
     for (const auto &Range : Ranges) {
       if (!Range.valid()) {
         ++NumErrors;
-        error() << "Invalid address range " << Range << "\n";
+        aggregate() << "Invalid address range " << Range << "\n";
         DumpDieAfterError = true;
         continue;
       }
@@ -545,7 +545,7 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die,
       // address: 0 or -1.
       if (auto PrevRange = RI.insert(Range)) {
         ++NumErrors;
-        error() << "DIE has overlapping ranges in DW_AT_ranges attribute: "
+        aggregate() << "DIE has overlapping ranges in DW_AT_ranges attribute: "
                 << *PrevRange << " and " << Range << '\n';
         DumpDieAfterError = true;
       }
@@ -558,7 +558,7 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die,
   const auto IntersectingChild = ParentRI.insert(RI);
   if (IntersectingChild != ParentRI.Children.end()) {
     ++NumErrors;
-    error() << "DIEs have overlapping address ranges:";
+    aggregate() << "DIEs have overlapping address ranges:";
     dump(Die);
     dump(IntersectingChild->Die) << '\n';
   }
@@ -569,7 +569,7 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die,
                              ParentRI.Die.getTag() == DW_TAG_subprogram);
   if (ShouldBeContained && !ParentRI.contains(RI)) {
     ++NumErrors;
-    error() << "DIE address ranges are not contained in its parent's ranges:";
+    aggregate() << "DIE address ranges are not contained in its parent's ranges:";
     dump(ParentRI.Die);
     dump(Die, 2) << '\n';
   }
@@ -754,7 +754,7 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
       auto CUOffset = AttrValue.Value.getRawUValue();
       if (CUOffset >= CUSize) {
         ++NumErrors;
-        error() << FormEncodingString(Form) << " CU offset "
+        aggregate("Invalid CU offset") << FormEncodingString(Form) << " CU offset "
                 << format("0x%08" PRIx64, CUOffset)
                 << " is invalid (must be less than CU size of "
                 << format("0x%08" PRIx64, CUSize) << "):\n";
@@ -776,7 +776,7 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
     if (RefVal) {
       if (*RefVal >= DieCU->getInfoSection().Data.size()) {
         ++NumErrors;
-        error() << "DW_FORM_ref_addr offset beyond .debug_info "
+        aggregate() << "DW_FORM_ref_addr offset beyond .debug_info "
                    "bounds:\n";
         dump(Die) << '\n';
       } else {
@@ -821,7 +821,7 @@ unsigned DWARFVerifier::verifyDebugInfoReferences(
     if (GetDIEForOffset(Pair.first))
       continue;
     ++NumErrors;
-    error() << "invalid DIE reference " << format("0x%08" PRIx64, Pair.first)
+    aggregate() << "invalid DIE reference " << format("0x%08" PRIx64, Pair.first)
             << ". Offset is in between DIEs:\n";
     for (auto Offset : Pair.second)
       dump(GetDIEForOffset(Offset)) << '\n';
@@ -845,7 +845,7 @@ void DWARFVerifier::verifyDebugLineStmtOffsets() {
     if (LineTableOffset < DCtx.getDWARFObj().getLineSection().Data.size()) {
       if (!LineTable) {
         ++NumDebugLineErrors;
-        error() << ".debug_line[" << format("0x%08" PRIx64, LineTableOffset)
+        aggregate("Unparseable .debug_line entry") << ".debug_line[" << format("0x%08" PRIx64, LineTableOffset)
                 << "] was not able to be parsed for CU:\n";
         dump(Die) << '\n';
         continue;
@@ -860,7 +860,7 @@ void DWARFVerifier::verifyDebugLineStmtOffsets() {
     auto Iter = StmtListToDie.find(LineTableOffset);
     if (Iter != StmtListToDie.end()) {
       ++NumDebugLineErrors;
-      error() << "two compile unit DIEs, "
+      aggregate("Identical DW_AT_stmt_list section offset") << "two compile unit DIEs, "
               << format("0x%08" PRIx64, Iter->second.getOffset()) << " and "
               << format("0x%08" PRIx64, Die.getOffset())
               << ", have the same DW_AT_stmt_list section offset:\n";
@@ -892,7 +892,7 @@ void DWARFVerifier::verifyDebugLineRows() {
       // Verify directory index.
       if (FileName.DirIdx > MaxDirIndex) {
         ++NumDebugLineErrors;
-        error() << ".debug_line["
+        aggregate("Invalid index in .debug_line->prologue.file_names->dir_idx") << ".debug_line["
                 << format("0x%08" PRIx64,
                           *toSectionOffset(Die.find(DW_AT_stmt_list)))
                 << "].prologue.file_names[" << FileIndex
@@ -928,7 +928,7 @@ void DWARFVerifier::verifyDebugLineRows() {
       // Verify row address.
       if (Row.Address.Address < PrevAddress) {
         ++NumDebugLineErrors;
-        error() << ".debug_line["
+        aggregate("decreasing address between debug_line rows") << ".debug_line["
                 << format("0x%08" PRIx64,
                           *toSectionOffset(Die.find(DW_AT_stmt_list)))
                 << "] row[" << RowIndex
@@ -949,7 +949,7 @@ void DWARFVerifier::verifyDebugLineRows() {
            LineTable->Rows.size() != 1) &&
           !LineTable->hasFileAtIndex(Row.File)) {
         ++NumDebugLineErrors;
-        error() << ".debug_line["
+        aggregate("Invalid file index in debug_line") << ".debug_line["
                 << format("0x%08" PRIx64,
                           *toSectionOffset(Die.find(DW_AT_stmt_list)))
                 << "][" << RowIndex << "] has invalid file index " << Row.File
@@ -971,8 +971,8 @@ void DWARFVerifier::verifyDebugLineRows() {
 
 DWARFVerifier::DWARFVerifier(raw_ostream &S, DWARFContext &D,
                              DIDumpOptions DumpOpts)
-    : OS(S), DCtx(D), DumpOpts(std::move(DumpOpts)), IsObjectFile(false),
-      IsMachOObject(false) {
+    : OS(S), DCtx(D), ErrAggregation(*this), DumpOpts(std::move(DumpOpts)),
+      IsObjectFile(false), IsMachOObject(false) {
   if (const auto *F = DCtx.getDWARFObj().getFile()) {
     IsObjectFile = F->isRelocatableObject();
     IsMachOObject = F->isMachO();
@@ -999,7 +999,7 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
 
   // Verify that the fixed part of the header is not too short.
   if (!AccelSectionData.isValidOffset(AccelTable.getSizeHdr())) {
-    error() << "Section is too small to fit a section header.\n";
+    aggregate() << "Section is too small to fit a section header.\n";
     return 1;
   }
 
@@ -1020,18 +1020,18 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
   for (uint32_t BucketIdx = 0; BucketIdx < NumBuckets; ++BucketIdx) {
     uint32_t HashIdx = AccelSectionData.getU32(&BucketsOffset);
     if (HashIdx >= NumHashes && HashIdx != UINT32_MAX) {
-      error() << format("Bucket[%d] has invalid hash index: %u.\n", BucketIdx,
+      aggregate("Invalid hash index") << format("Bucket[%d] has invalid hash index: %u.\n", BucketIdx,
                         HashIdx);
       ++NumErrors;
     }
   }
   uint32_t NumAtoms = AccelTable.getAtomsDesc().size();
   if (NumAtoms == 0) {
-    error() << "No atoms: failed to read HashData.\n";
+    aggregate() << "No atoms: failed to read HashData.\n";
     return 1;
   }
   if (!AccelTable.validateForms()) {
-    error() << "Unsupported form: failed to read HashData.\n";
+    aggregate() << "Unsupported form: failed to read HashData.\n";
     return 1;
   }
 
@@ -1042,7 +1042,7 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
     uint64_t HashDataOffset = AccelSectionData.getU32(&DataOffset);
     if (!AccelSectionData.isValidOffsetForDataOfSize(HashDataOffset,
                                                      sizeof(uint64_t))) {
-      error() << format("Hash[%d] has invalid HashData offset: "
+      aggregate("Invalid HashData offset") << format("Hash[%d] has invalid HashData offset: "
                         "0x%08" PRIx64 ".\n",
                         HashIdx, HashDataOffset);
       ++NumErrors;
@@ -1068,7 +1068,7 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
           if (!Name)
             Name = "<NULL>";
 
-          error() << format(
+          aggregate("Invalid DIE offset") << format(
               "%s Bucket[%d] Hash[%d] = 0x%08x "
               "Str[%u] = 0x%08" PRIx64 " DIE[%d] = 0x%08" PRIx64 " "
               "is not a valid DIE offset for \"%s\".\n",
@@ -1079,7 +1079,7 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
           continue;
         }
         if ((Tag != dwarf::DW_TAG_null) && (Die.getTag() != Tag)) {
-          error() << "Tag " << dwarf::TagString(Tag)
+          aggregate("Mismatched Tag in accellerator table") << "Tag " << dwarf::TagString(Tag)
                   << " in accelerator table does not match Tag "
                   << dwarf::TagString(Die.getTag()) << " of DIE[" << HashDataIdx
                   << "].\n";
@@ -1106,7 +1106,7 @@ DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) {
   unsigned NumErrors = 0;
   for (const DWARFDebugNames::NameIndex &NI : AccelTable) {
     if (NI.getCUCount() == 0) {
-      error() << formatv("Name Index @ {0:x} does not index any CU\n",
+      aggregate("Name Index doesn't index any CU") << formatv("Name Index @ {0:x} does not index any CU\n",
                          NI.getUnitOffset());
       ++NumErrors;
       continue;
@@ -1116,7 +1116,7 @@ DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) {
       auto Iter = CUMap.find(Offset);
 
       if (Iter == CUMap.end()) {
-        error() << formatv(
+        aggregate("Name Index references non-existing CU") << formatv(
             "Name Index @ {0:x} references a non-existing CU @ {1:x}\n",
             NI.getUnitOffset(), Offset);
         ++NumErrors;
@@ -1124,7 +1124,7 @@ DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) {
       }
 
       if (Iter->second != NotIndexed) {
-        error() << formatv("Name Index @ {0:x} references a CU @ {1:x}, but "
+        aggregate("Duplicate Name Index") << formatv("Name Index @ {0:x} references a CU @ {1:x}, but "
                            "this CU is already indexed by Name Index @ {2:x}\n",
                            NI.getUnitOffset(), Offset, Iter->second);
         continue;
@@ -1167,7 +1167,7 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
   for (uint32_t Bucket = 0, End = NI.getBucketCount(); Bucket < End; ++Bucket) {
     uint32_t Index = NI.getBucketArrayEntry(Bucket);
     if (Index > NI.getNameCount()) {
-      error() << formatv("Bucket {0} of Name Index @ {1:x} contains invalid "
+      aggregate("Name Index Bucket contains invalid value") << formatv("Bucket {0} of Name Index @ {1:x} contains invalid "
                          "value {2}. Valid range is [0, {3}].\n",
                          Bucket, NI.getUnitOffset(), Index, NI.getNameCount());
       ++NumErrors;
@@ -1202,7 +1202,7 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
     // will not match because we have already verified that the name's hash
     // puts it into the previous bucket.)
     if (B.Index > NextUncovered) {
-      error() << formatv("Name Index @ {0:x}: Name table entries [{1}, {2}] "
+      aggregate("Name table entries uncovered by hash table") << formatv("Name Index @ {0:x}: Name table entries [{1}, {2}] "
                          "are not covered by the hash table.\n",
                          NI.getUnitOffset(), NextUncovered, B.Index - 1);
       ++NumErrors;
@@ -1220,7 +1220,7 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
     // bucket as empty.
     uint32_t FirstHash = NI.getHashArrayEntry(Idx);
     if (FirstHash % NI.getBucketCount() != B.Bucket) {
-      error() << formatv(
+      aggregate("Name Index point to mismatched hash value") << formatv(
           "Name Index @ {0:x}: Bucket {1} is not empty but points to a "
           "mismatched hash value {2:x} (belonging to bucket {3}).\n",
           NI.getUnitOffset(), B.Bucket, FirstHash,
@@ -1238,7 +1238,7 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
 
       const char *Str = NI.getNameTableEntry(Idx).getString();
       if (caseFoldingDjbHash(Str) != Hash) {
-        error() << formatv("Name Index @ {0:x}: String ({1}) at index {2} "
+        aggregate("String hash doesn't match Name Index hash") << formatv("Name Index @ {0:x}: String ({1}) at index {2} "
                            "hashes to {3:x}, but "
                            "the Name Index hash is {4:x}\n",
                            NI.getUnitOffset(), Str, Idx,
@@ -1258,7 +1258,7 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(
     DWARFDebugNames::AttributeEncoding AttrEnc) {
   StringRef FormName = dwarf::FormEncodingString(AttrEnc.Form);
   if (FormName.empty()) {
-    error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an "
+    aggregate("Unknown NameIndex Abbreviation") << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an "
                        "unknown form: {3}.\n",
                        NI.getUnitOffset(), Abbr.Code, AttrEnc.Index,
                        AttrEnc.Form);
@@ -1267,7 +1267,7 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(
 
   if (AttrEnc.Index == DW_IDX_type_hash) {
     if (AttrEnc.Form != dwarf::DW_FORM_data8) {
-      error() << formatv(
+      aggregate("Unexpected NameIndex Abbreviation") << formatv(
           "NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_type_hash "
           "uses an unexpected form {2} (should be {3}).\n",
           NI.getUnitOffset(), Abbr.Code, AttrEnc.Form, dwarf::DW_FORM_data8);
@@ -1280,7 +1280,7 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(
     constexpr static auto AllowedForms = {dwarf::Form::DW_FORM_flag_present,
                                           dwarf::Form::DW_FORM_ref4};
     if (!is_contained(AllowedForms, AttrEnc.Form)) {
-      error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_parent "
+      aggregate("Unexpected NameIndex Abbreviation") << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_parent "
                          "uses an unexpected form {2} (should be "
                          "DW_FORM_ref4 or DW_FORM_flag_present).\n",
                          NI.getUnitOffset(), Abbr.Code, AttrEnc.Form);
@@ -1315,7 +1315,7 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(
   }
 
   if (!DWARFFormValue(AttrEnc.Form).isFormClass(Iter->Class)) {
-    error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an "
+    aggregate("Unexpected NameIndex Abbreviation") << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an "
                        "unexpected form {3} (expected form class {4}).\n",
                        NI.getUnitOffset(), Abbr.Code, AttrEnc.Index,
                        AttrEnc.Form, Iter->ClassName);
@@ -1344,7 +1344,7 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) {
     SmallSet<unsigned, 5> Attributes;
     for (const auto &AttrEnc : Abbrev.Attributes) {
       if (!Attributes.insert(AttrEnc.Index).second) {
-        error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x} contains "
+        aggregate("NameIndex Abbreviateion contains multiple attributes") << formatv("NameIndex @ {0:x}: Abbreviation {1:x} contains "
                            "multiple {2} attributes.\n",
                            NI.getUnitOffset(), Abbrev.Code, AttrEnc.Index);
         ++NumErrors;
@@ -1354,7 +1354,7 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) {
     }
 
     if (NI.getCUCount() > 1 && !Attributes.count(dwarf::DW_IDX_compile_unit)) {
-      error() << formatv("NameIndex @ {0:x}: Indexing multiple compile units "
+      aggregate("Abbreviation contains no attribute") << formatv("NameIndex @ {0:x}: Indexing multiple compile units "
                          "and abbreviation {1:x} has no {2} attribute.\n",
                          NI.getUnitOffset(), Abbrev.Code,
                          dwarf::DW_IDX_compile_unit);
@@ -1803,6 +1803,69 @@ bool DWARFVerifier::verifyDebugStrOffsets(
   }
   return Success;
 }
+/*
+    raw_ostream & operator <<(const char * s) {
+        collect(s);
+        out << "From inside the class: " << s;
+        return out;
+    }
+    void collect(const char * s) {
+        counters[s]++;
+    }
+    void dump() {
+        for (auto && [name, count]: counters) {
+            out << "counter: " << name << ", count: " << count << "\n";
+        }
+    }
+
+  ErrorAggregator collectErrors();
+*/
+
+std::string DWARFVerifier::ErrorAggregator::Clean(const char *s) {
+  // Find the position of the first alphabetic character
+  const char *start = s;
+  while (!std::isalpha(*start)) {
+    ++start;
+  }
+
+  // Find the position of the last alphabetic character
+  const char *end = &start[std::strlen(start) - 1];
+  while (end >= start && !std::isalpha(*end)) {
+    --end;
+  }
+
+  // Create a std::string from the trimmed portion of the const char*
+  return std::string(start, end - start + 1);
+}
+
+raw_ostream &DWARFVerifier::ErrorAggregator::operator<<(const char *s) {
+  Collect(Clean(s));
+  return Verifier.error() << s;
+}
+
+void DWARFVerifier::ErrorAggregator::Collect(const std::string &s) {
+  UniqueErrors[s]++;
+}
+
+void DWARFVerifier::ErrorAggregator::Dump() {
+  for (auto &&[name, count] : UniqueErrors) {
+    Verifier.error() << "counter: " << name << ", count: " << count << "\n";
+  }
+}
+
+raw_ostream &DWARFVerifier::aggregate(const char *name) {
+  ErrAggregation.Collect(name);
+  return error();
+}
+
+DWARFVerifier::ErrorAggregator &DWARFVerifier::aggregate() {
+  return ErrAggregation;
+}
+
+void DWARFVerifier::finish() {
+  if (DumpOpts.DumpAggregateErrors)
+    ErrAggregation.Dump();
+}
 
 raw_ostream &DWARFVerifier::error() const { return WithColor::error(OS); }
 

>From cfd8e4ed543f6363c24e96082570a6844c29747d Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Wed, 24 Jan 2024 15:33:04 -0800
Subject: [PATCH 02/11] Added usage comments

---
 llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index b9a68217aebfce..d55709675cbd69 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -101,8 +101,14 @@ class DWARFVerifier {
   bool IsMachOObject;
   using ReferenceMap = std::map<uint64_t, std::set<uint64_t>>;
 
-  raw_ostream &aggregate(const char *);
+  // For errors that have sensible names as the first (or only) string
+  // you can just use aggregate() << "DIE looks stupid: " << details...
+  // and it will get aggregated as "DIE looks stupid".
   ErrorAggregator &aggregate();
+  // For errors that have details before good 'categories', you'll need
+  // to provide the aggregated name, like this:
+  // aggregate("CU index is busted") << "CU index [" << n << "] is busted"
+  raw_ostream &aggregate(const char *);
   raw_ostream &error() const;
   raw_ostream &warn() const;
   raw_ostream &note() const;

>From 0ebf6651e7858162bbf1b9e14ba63ed2792bec9b Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Wed, 24 Jan 2024 16:11:12 -0800
Subject: [PATCH 03/11] Finished first pass over the error()'s (inserted some
 TODO's)

---
 .../llvm/DebugInfo/DWARF/DWARFVerifier.h      | 24 +++--
 llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp    | 93 ++++++++++---------
 2 files changed, 66 insertions(+), 51 deletions(-)

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index d55709675cbd69..ae2aa62e28f921 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -81,12 +81,15 @@ class DWARFVerifier {
   private:
     DWARFVerifier &Verifier;
     std::map<std::string, int> UniqueErrors;
+    raw_ostream *NextLineOverride;
+
     static std::string Clean(const char *s);
 
   public:
-    ErrorAggregator(DWARFVerifier &v) : Verifier(v) {}
+    ErrorAggregator(DWARFVerifier &v) : Verifier(v), NextLineOverride(nullptr) {}
     raw_ostream &operator<<(const char *s);
-    void Collect(const std::string &s);
+    ErrorAggregator &NextLineStreamOverride(raw_ostream &o);
+    raw_ostream &Collect(const std::string &s);
     void Dump();
   };
   friend class ErrorAggregator;
@@ -102,13 +105,20 @@ class DWARFVerifier {
   using ReferenceMap = std::map<uint64_t, std::set<uint64_t>>;
 
   // For errors that have sensible names as the first (or only) string
-  // you can just use aggregate() << "DIE looks stupid: " << details...
-  // and it will get aggregated as "DIE looks stupid".
+  // you can just use aggregate() << "DIE is damaged: " << details...
+  // and it will get aggregated as "DIE is damaged".
   ErrorAggregator &aggregate();
-  // For errors that have details before good 'categories', you'll need
-  // to provide the aggregated name, like this:
-  // aggregate("CU index is busted") << "CU index [" << n << "] is busted"
+  // For errors that have the useful aggregated category in a non-error stream
+  // you can do things like this (aggregated as 'Die size is wrong')
+  // aggregate(note()) << "DIE size is wrong.\n"
+  ErrorAggregator &aggregate(raw_ostream &);
+
+  // For errors that have valuable detail before text that is the appropriate
+  // aggregate category, you can provide the aggregated name for the error like
+  // this:
+  // aggregate("CU index is broken") << "CU index [" << n << "] is broken"
   raw_ostream &aggregate(const char *);
+
   raw_ostream &error() const;
   raw_ostream &warn() const;
   raw_ostream &note() const;
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index e43f01fad3ce02..fcfef5904d55ed 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -170,17 +170,19 @@ bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData,
     error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", UnitIndex,
                       OffsetStart);
     if (!ValidLength)
-      note() << "The length for this unit is too "
+      aggregate(note()) << "The length for this unit is too "
                 "large for the .debug_info provided.\n";
-    if (!ValidVersion)
-      note() << "The 16 bit unit header version is not valid.\n";
-    if (!ValidType)
-      note() << "The unit type encoding is not valid.\n";
+    if (!ValidVersion) {
+      aggregate(note()) << "The 16 bit unit header version is not valid.\n";
+    }
+    if (!ValidType){
+      aggregate(note()) << "The unit type encoding is not valid.\n";
+    }
     if (!ValidAbbrevOffset)
-      note() << "The offset into the .debug_abbrev section is "
+      aggregate(note()) << "The offset into the .debug_abbrev section is "
                 "not valid.\n";
     if (!ValidAddrSize)
-      note() << "The address size is unsupported.\n";
+      aggregate(note()) << "The address size is unsupported.\n";
   }
   *Offset = OffsetStart + Length + (isUnitDWARF64 ? 12 : 4);
   return Success;
@@ -198,7 +200,7 @@ bool DWARFVerifier::verifyName(const DWARFDie &Die) {
   if (OriginalFullName.empty() || OriginalFullName == ReconstructedName)
     return false;
 
-  error() << "Simplified template DW_AT_name could not be reconstituted:\n"
+  aggregate() << "Simplified template DW_AT_name could not be reconstituted:\n"
           << formatv("         original: {0}\n"
                      "    reconstituted: {1}\n",
                      OriginalFullName, ReconstructedName);
@@ -581,6 +583,7 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die,
   return NumErrors;
 }
 
+// Aggregation TODO:
 unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die,
                                                  DWARFAttribute &AttrValue) {
   unsigned NumErrors = 0;
@@ -796,6 +799,7 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
   case DW_FORM_line_strp: {
     if (Error E = AttrValue.Value.getAsCString().takeError()) {
       ++NumErrors;
+      // Aggregation TODO:
       error() << toString(std::move(E)) << ":\n";
       dump(Die) << '\n';
     }
@@ -1005,6 +1009,7 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
 
   // Verify that the section is not too short.
   if (Error E = AccelTable.extract()) {
+    // Aggregation TODO:
     error() << toString(std::move(E)) << '\n';
     return 1;
   }
@@ -1361,7 +1366,7 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) {
       ++NumErrors;
     }
     if (!Attributes.count(dwarf::DW_IDX_die_offset)) {
-      error() << formatv(
+      aggregate("Abbreviate in NameIndex missing attribute") << formatv(
           "NameIndex @ {0:x}: Abbreviation {1:x} has no {2} attribute.\n",
           NI.getUnitOffset(), Abbrev.Code, dwarf::DW_IDX_die_offset);
       ++NumErrors;
@@ -1417,7 +1422,7 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
 
   const char *CStr = NTE.getString();
   if (!CStr) {
-    error() << formatv(
+    aggregate("Unable to get string associated with name") << formatv(
         "Name Index @ {0:x}: Unable to get string associated with name {1}.\n",
         NI.getUnitOffset(), NTE.getIndex());
     return 1;
@@ -1433,7 +1438,7 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
                                 EntryOr = NI.getEntry(&NextEntryID)) {
     uint32_t CUIndex = *EntryOr->getCUIndex();
     if (CUIndex > NI.getCUCount()) {
-      error() << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an "
+      aggregate("Name Index entry contains invalid CU index") << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an "
                          "invalid CU index ({2}).\n",
                          NI.getUnitOffset(), EntryID, CUIndex);
       ++NumErrors;
@@ -1443,21 +1448,21 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
     uint64_t DIEOffset = CUOffset + *EntryOr->getDIEUnitOffset();
     DWARFDie DIE = DCtx.getDIEForOffset(DIEOffset);
     if (!DIE) {
-      error() << formatv("Name Index @ {0:x}: Entry @ {1:x} references a "
+      aggregate("NameIndex references nonexisten DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x} references a "
                          "non-existing DIE @ {2:x}.\n",
                          NI.getUnitOffset(), EntryID, DIEOffset);
       ++NumErrors;
       continue;
     }
     if (DIE.getDwarfUnit()->getOffset() != CUOffset) {
-      error() << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of "
-                         "DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n",
+      aggregate("Name index contains mismatched CU of DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of "
+                        "DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n",
                          NI.getUnitOffset(), EntryID, DIEOffset, CUOffset,
                          DIE.getDwarfUnit()->getOffset());
       ++NumErrors;
     }
     if (DIE.getTag() != EntryOr->tag()) {
-      error() << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Tag of "
+      aggregate("Name Index contains mismatched Tag of DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Tag of "
                          "DIE @ {2:x}: index - {3}; debug_info - {4}.\n",
                          NI.getUnitOffset(), EntryID, DIEOffset, EntryOr->tag(),
                          DIE.getTag());
@@ -1471,7 +1476,7 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
         DIE.getTag() == DW_TAG_inlined_subroutine;
     auto EntryNames = getNames(DIE, IncludeStrippedTemplateNames);
     if (!is_contained(EntryNames, Str)) {
-      error() << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Name "
+      aggregate("Name Index contains mismatched name of DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Name "
                          "of DIE @ {2:x}: index - {3}; debug_info - {4}.\n",
                          NI.getUnitOffset(), EntryID, DIEOffset, Str,
                          make_range(EntryNames.begin(), EntryNames.end()));
@@ -1482,12 +1487,13 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
                   [&](const DWARFDebugNames::SentinelError &) {
                     if (NumEntries > 0)
                       return;
-                    error() << formatv("Name Index @ {0:x}: Name {1} ({2}) is "
+                    aggregate("NameIndex Name is not associated with any entries") << formatv("Name Index @ {0:x}: Name {1} ({2}) is "
                                        "not associated with any entries.\n",
                                        NI.getUnitOffset(), NTE.getIndex(), Str);
                     ++NumErrors;
                   },
                   [&](const ErrorInfoBase &Info) {
+                    // Aggregation TODO:
                     error()
                         << formatv("Name Index @ {0:x}: Name {1} ({2}): {3}\n",
                                    NI.getUnitOffset(), NTE.getIndex(), Str,
@@ -1619,7 +1625,7 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness(
     if (none_of(NI.equal_range(Name), [&](const DWARFDebugNames::Entry &E) {
           return E.getDIEUnitOffset() == DieUnitOffset;
         })) {
-      error() << formatv("Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with "
+      aggregate("Name Index DIE entry missing name") << formatv("Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with "
                          "name {3} missing.\n",
                          NI.getUnitOffset(), Die.getOffset(), Die.getTag(),
                          Name);
@@ -1641,6 +1647,7 @@ unsigned DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection,
   // This verifies that we can read individual name indices and their
   // abbreviation tables.
   if (Error E = AccelTable.extract()) {
+    // Aggregate TODO:
     error() << toString(std::move(E)) << '\n';
     return 1;
   }
@@ -1741,7 +1748,7 @@ bool DWARFVerifier::verifyDebugStrOffsets(
       if (!C)
         break;
       if (C.tell() + Length > DA.getData().size()) {
-        error() << formatv(
+        aggregate("Section contribution length exceeds available space") << formatv(
             "{0}: contribution {1:X}: length exceeds available space "
             "(contribution "
             "offset ({1:X}) + length field space ({2:X}) + length ({3:X}) == "
@@ -1755,7 +1762,7 @@ bool DWARFVerifier::verifyDebugStrOffsets(
       NextUnit = C.tell() + Length;
       uint8_t Version = DA.getU16(C);
       if (C && Version != 5) {
-        error() << formatv("{0}: contribution {1:X}: invalid version {2}\n",
+        aggregate("Invalid Section version") << formatv("{0}: contribution {1:X}: invalid version {2}\n",
                            SectionName, StartOffset, Version);
         Success = false;
         // Can't parse the rest of this contribution, since we don't know the
@@ -1768,7 +1775,7 @@ bool DWARFVerifier::verifyDebugStrOffsets(
     DA.setAddressSize(OffsetByteSize);
     uint64_t Remainder = (Length - 4) % OffsetByteSize;
     if (Remainder != 0) {
-      error() << formatv(
+      aggregate("Invalid section contribution length") << formatv(
           "{0}: contribution {1:X}: invalid length ((length ({2:X}) "
           "- header (0x4)) % offset size {3:X} == {4:X} != 0)\n",
           SectionName, StartOffset, Length, OffsetByteSize, Remainder);
@@ -1789,7 +1796,7 @@ bool DWARFVerifier::verifyDebugStrOffsets(
       }
       if (StrData[StrOff - 1] == '\0')
         continue;
-      error() << formatv("{0}: contribution {1:X}: index {2:X}: invalid string "
+      aggregate("Section contribution contains invalid string offset") << formatv("{0}: contribution {1:X}: index {2:X}: invalid string "
                          "offset *{3:X} == {4:X}, is neither zero nor "
                          "immediately following a null character\n",
                          SectionName, StartOffset, Index, OffOff, StrOff);
@@ -1798,28 +1805,12 @@ bool DWARFVerifier::verifyDebugStrOffsets(
   }
 
   if (Error E = C.takeError()) {
+    // Aggregate TODO:
     error() << SectionName << ": " << toString(std::move(E)) << '\n';
     return false;
   }
   return Success;
 }
-/*
-    raw_ostream & operator <<(const char * s) {
-        collect(s);
-        out << "From inside the class: " << s;
-        return out;
-    }
-    void collect(const char * s) {
-        counters[s]++;
-    }
-    void dump() {
-        for (auto && [name, count]: counters) {
-            out << "counter: " << name << ", count: " << count << "\n";
-        }
-    }
-
-  ErrorAggregator collectErrors();
-*/
 
 std::string DWARFVerifier::ErrorAggregator::Clean(const char *s) {
   // Find the position of the first alphabetic character
@@ -1839,12 +1830,18 @@ std::string DWARFVerifier::ErrorAggregator::Clean(const char *s) {
 }
 
 raw_ostream &DWARFVerifier::ErrorAggregator::operator<<(const char *s) {
-  Collect(Clean(s));
-  return Verifier.error() << s;
+  return Collect(Clean(s)) << s;
 }
 
-void DWARFVerifier::ErrorAggregator::Collect(const std::string &s) {
+raw_ostream &DWARFVerifier::ErrorAggregator::Collect(const std::string &s) {
+  // Register the error in the aggregator
   UniqueErrors[s]++;
+
+  // If we have an output stream override, return it and reset it back to
+  // the default
+  raw_ostream *o = NextLineOverride;
+  NextLineOverride = nullptr;
+  return (o == nullptr) ? Verifier.error() : *o;
 }
 
 void DWARFVerifier::ErrorAggregator::Dump() {
@@ -1854,8 +1851,16 @@ void DWARFVerifier::ErrorAggregator::Dump() {
 }
 
 raw_ostream &DWARFVerifier::aggregate(const char *name) {
-  ErrAggregation.Collect(name);
-  return error();
+  return ErrAggregation.Collect(name);
+}
+
+DWARFVerifier::ErrorAggregator &DWARFVerifier::ErrorAggregator::NextLineStreamOverride(raw_ostream &o) {
+  NextLineOverride = &o;
+  return *this;
+}
+
+DWARFVerifier::ErrorAggregator &DWARFVerifier::aggregate(raw_ostream &o) {
+  return ErrAggregation.NextLineStreamOverride(o);
 }
 
 DWARFVerifier::ErrorAggregator &DWARFVerifier::aggregate() {

>From ee4de82892210e7d209b71fd473fed2c1a53159a Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Wed, 24 Jan 2024 17:28:31 -0800
Subject: [PATCH 04/11] Updated with Greg's suggestion for the reporter

---
 .../llvm/DebugInfo/DWARF/DWARFVerifier.h      |  49 +---
 llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp    | 264 +++++++++---------
 2 files changed, 151 insertions(+), 162 deletions(-)

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index ae2aa62e28f921..c2abc49c086947 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -30,6 +30,20 @@ class DWARFDebugAbbrev;
 class DataExtractor;
 struct DWARFSection;
 
+class OutputCategoryAggregator {
+private:
+  std::map<std::string, int> Aggregator;
+  bool Output;
+
+public:
+  OutputCategoryAggregator(bool DetailedOutput = false)
+      : Output(DetailedOutput) {}
+  void EnableDetail() { Output = true; }
+  void DisableDetail() { Output = false; }
+  void Report(StringRef s, std::function<void()> detailCallback);
+  void DumpAggregation(raw_ostream *o);
+};
+
 /// A class that verifies DWARF debug information given a DWARF Context.
 class DWARFVerifier {
 public:
@@ -76,27 +90,9 @@ class DWARFVerifier {
     bool intersects(const DieRangeInfo &RHS) const;
   };
 
-private:
-  class ErrorAggregator {
-  private:
-    DWARFVerifier &Verifier;
-    std::map<std::string, int> UniqueErrors;
-    raw_ostream *NextLineOverride;
-
-    static std::string Clean(const char *s);
-
-  public:
-    ErrorAggregator(DWARFVerifier &v) : Verifier(v), NextLineOverride(nullptr) {}
-    raw_ostream &operator<<(const char *s);
-    ErrorAggregator &NextLineStreamOverride(raw_ostream &o);
-    raw_ostream &Collect(const std::string &s);
-    void Dump();
-  };
-  friend class ErrorAggregator;
-
   raw_ostream &OS;
   DWARFContext &DCtx;
-  ErrorAggregator ErrAggregation;
+  OutputCategoryAggregator ErrorCategory;
   DIDumpOptions DumpOpts;
   uint32_t NumDebugLineErrors = 0;
   // Used to relax some checks that do not currently work portably
@@ -104,21 +100,6 @@ class DWARFVerifier {
   bool IsMachOObject;
   using ReferenceMap = std::map<uint64_t, std::set<uint64_t>>;
 
-  // For errors that have sensible names as the first (or only) string
-  // you can just use aggregate() << "DIE is damaged: " << details...
-  // and it will get aggregated as "DIE is damaged".
-  ErrorAggregator &aggregate();
-  // For errors that have the useful aggregated category in a non-error stream
-  // you can do things like this (aggregated as 'Die size is wrong')
-  // aggregate(note()) << "DIE size is wrong.\n"
-  ErrorAggregator &aggregate(raw_ostream &);
-
-  // For errors that have valuable detail before text that is the appropriate
-  // aggregate category, you can provide the aggregated name for the error like
-  // this:
-  // aggregate("CU index is broken") << "CU index [" << n << "] is broken"
-  raw_ostream &aggregate(const char *);
-
   raw_ostream &error() const;
   raw_ostream &warn() const;
   raw_ostream &note() const;
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index fcfef5904d55ed..db9b96d18476c8 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -170,19 +170,29 @@ bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData,
     error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", UnitIndex,
                       OffsetStart);
     if (!ValidLength)
-      aggregate(note()) << "The length for this unit is too "
-                "large for the .debug_info provided.\n";
+      ErrorCategory.Report("Invalid Length in Unit Header", [&]() {
+        note() << "The length for this unit is too "
+                  "large for the .debug_info provided.\n";
+      });
     if (!ValidVersion) {
-      aggregate(note()) << "The 16 bit unit header version is not valid.\n";
+      ErrorCategory.Report("Invalid 16 bit unit header", [&]() {
+        note() << "The 16 bit unit header version is not valid.\n";
+      });
     }
-    if (!ValidType){
-      aggregate(note()) << "The unit type encoding is not valid.\n";
+    if (!ValidType) {
+      ErrorCategory.Report("Invalid unit type encoding", [&]() {
+        note() << "The unit type encoding is not valid.\n";
+      });
     }
     if (!ValidAbbrevOffset)
-      aggregate(note()) << "The offset into the .debug_abbrev section is "
-                "not valid.\n";
+      ErrorCategory.Report("Invalid unit type encoding", [&]() {
+        note() << "The offset into the .debug_abbrev section is "
+                  "not valid.\n";
+      });
     if (!ValidAddrSize)
-      aggregate(note()) << "The address size is unsupported.\n";
+      ErrorCategory.Report("Invalid unit type encoding", [&]() {
+        note() << "The address size is unsupported.\n";
+      });
   }
   *Offset = OffsetStart + Length + (isUnitDWARF64 ? 12 : 4);
   return Success;
@@ -200,10 +210,14 @@ bool DWARFVerifier::verifyName(const DWARFDie &Die) {
   if (OriginalFullName.empty() || OriginalFullName == ReconstructedName)
     return false;
 
-  aggregate() << "Simplified template DW_AT_name could not be reconstituted:\n"
-          << formatv("         original: {0}\n"
-                     "    reconstituted: {1}\n",
-                     OriginalFullName, ReconstructedName);
+  ErrorCategory.Report(
+      "Simplified template DW_AT_name could not be reconstituted", [&]() {
+        error()
+            << "Simplified template DW_AT_name could not be reconstituted:\n"
+            << formatv("         original: {0}\n"
+                       "    reconstituted: {1}\n",
+                       OriginalFullName, ReconstructedName);
+      });
   dump(Die) << '\n';
   dump(Die.getDwarfUnit()->getUnitDIE()) << '\n';
   return true;
@@ -242,22 +256,28 @@ unsigned DWARFVerifier::verifyUnitContents(DWARFUnit &Unit,
 
   DWARFDie Die = Unit.getUnitDIE(/* ExtractUnitDIEOnly = */ false);
   if (!Die) {
-    aggregate() << "Compilation unit without DIE.\n";
+    ErrorCategory.Report("Compilation unity missing DIE", [&]() {
+      error() << "Compilation unit without DIE.\n";
+    });
     NumUnitErrors++;
     return NumUnitErrors;
   }
 
   if (!dwarf::isUnitType(Die.getTag())) {
-    aggregate() << "Compilation unit root DIE is not a unit DIE: "
-            << dwarf::TagString(Die.getTag()) << ".\n";
+    ErrorCategory.Report("Compilation unit root DIE is not a unit DIE", [&]() {
+      error() << "Compilation unit root DIE is not a unit DIE: "
+              << dwarf::TagString(Die.getTag()) << ".\n";
+    });
     NumUnitErrors++;
   }
 
   uint8_t UnitType = Unit.getUnitType();
   if (!DWARFUnit::isMatchingUnitTypeAndTag(UnitType, Die.getTag())) {
-    aggregate("Mismatched unit type") << "Compilation unit type (" << dwarf::UnitTypeString(UnitType)
-            << ") and root DIE (" << dwarf::TagString(Die.getTag())
-            << ") do not match.\n";
+    ErrorCategory.Report("Mismatched unit type", [&]() {
+      error() << "Compilation unit type (" << dwarf::UnitTypeString(UnitType)
+              << ") and root DIE (" << dwarf::TagString(Die.getTag())
+              << ") do not match.\n";
+    });
     NumUnitErrors++;
   }
 
@@ -265,7 +285,9 @@ unsigned DWARFVerifier::verifyUnitContents(DWARFUnit &Unit,
   //  3.1.2 Skeleton Compilation Unit Entries:
   //  "A skeleton compilation unit has no children."
   if (Die.getTag() == dwarf::DW_TAG_skeleton_unit && Die.hasChildren()) {
-    aggregate() << "Skeleton compilation unit has children.\n";
+    ErrorCategory.Report("Skeleton CU has children", [&]() {
+      error() << "Skeleton compilation unit has children.\n";
+    });
     NumUnitErrors++;
   }
 
@@ -282,14 +304,20 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) {
   DWARFDie Curr = Die.getParent();
   for (; Curr.isValid() && !Curr.isSubprogramDIE(); Curr = Die.getParent()) {
     if (Curr.getTag() == DW_TAG_inlined_subroutine) {
-      aggregate() << "Call site entry nested within inlined subroutine:";
+      ErrorCategory.Report(
+          "Call site nested entry within inlined subroutine", [&]() {
+            error() << "Call site entry nested within inlined subroutine:";
+          });
       Curr.dump(OS);
       return 1;
     }
   }
 
   if (!Curr.isValid()) {
-    aggregate() << "Call site entry not nested within a valid subprogram:";
+    ErrorCategory.Report(
+        "Call site entry not nexted within valid subprogram", [&]() {
+          error() << "Call site entry not nested within a valid subprogram:";
+        });
     Die.dump(OS);
     return 1;
   }
@@ -299,7 +327,11 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) {
        DW_AT_call_all_tail_calls, DW_AT_GNU_all_call_sites,
        DW_AT_GNU_all_source_call_sites, DW_AT_GNU_all_tail_call_sites});
   if (!CallAttr) {
-    aggregate() << "Subprogram with call site entry has no DW_AT_call attribute:";
+    ErrorCategory.Report(
+        "Subprogram with call site entry has no DW_AT_call attribute", [&]() {
+          error()
+              << "Subprogram with call site entry has no DW_AT_call attribute:";
+        });
     Curr.dump(OS);
     Die.dump(OS, /*indent*/ 1);
     return 1;
@@ -315,7 +347,9 @@ unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) {
   Expected<const DWARFAbbreviationDeclarationSet *> AbbrDeclsOrErr =
       Abbrev->getAbbreviationDeclarationSet(0);
   if (!AbbrDeclsOrErr) {
-    aggregate("Abbreviateion Declaration error") << toString(AbbrDeclsOrErr.takeError()) << "\n";
+    ErrorCategory.Report("Abbreviation Declaration error", [&]() {
+      error() << toString(AbbrDeclsOrErr.takeError()) << "\n";
+    });
     return 1;
   }
 
@@ -326,9 +360,12 @@ unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) {
     for (auto Attribute : AbbrDecl.attributes()) {
       auto Result = AttributeSet.insert(Attribute.Attr);
       if (!Result.second) {
-        aggregate("Abbreviation declartion contains multiple attributes") << "Abbreviation declaration contains multiple "
-                << AttributeString(Attribute.Attr) << " attributes.\n";
-        AbbrDecl.dump(OS);
+        ErrorCategory.Report(
+            "Abbreviation declartion contains multiple attributes", [&]() {
+              error() << "Abbreviation declaration contains multiple "
+                      << AttributeString(Attribute.Attr) << " attributes.\n";
+              AbbrDecl.dump(OS);
+            });
         ++NumErrors;
       }
     }
@@ -442,10 +479,12 @@ unsigned DWARFVerifier::verifyIndex(StringRef Name,
       auto &M = *Sections[Col];
       auto I = M.find(SC.getOffset());
       if (I != M.end() && I.start() < (SC.getOffset() + SC.getLength())) {
-        aggregate("Overlapping index entries") << llvm::formatv(
-            "overlapping index entries for entries {0:x16} "
-            "and {1:x16} for column {2}\n",
-            *I, Sig, toString(Index.getColumnKinds()[Col]));
+        ErrorCategory.Report("Overlapping index entries", [&]() {
+          error() << llvm::formatv(
+              "overlapping index entries for entries {0:x16} "
+              "and {1:x16} for column {2}\n",
+              *I, Sig, toString(Index.getColumnKinds()[Col]));
+        });
         return 1;
       }
       M.insert(SC.getOffset(), SC.getOffset() + SC.getLength() - 1, Sig);
@@ -529,6 +568,9 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die,
   // For now, simply elide the range verification for the CU DIEs if we are
   // processing an object file.
 
+// KBF TODO: Continue here tomorrow:
+ErrorCategory.Report("", [&]() {
+      error
   if (!IsObjectFile || IsMachOObject || Die.getTag() != DW_TAG_compile_unit) {
     bool DumpDieAfterError = false;
     for (const auto &Range : Ranges) {
@@ -1438,9 +1480,10 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
                                 EntryOr = NI.getEntry(&NextEntryID)) {
     uint32_t CUIndex = *EntryOr->getCUIndex();
     if (CUIndex > NI.getCUCount()) {
-      aggregate("Name Index entry contains invalid CU index") << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an "
-                         "invalid CU index ({2}).\n",
-                         NI.getUnitOffset(), EntryID, CUIndex);
+      aggregate("Name Index entry contains invalid CU index")
+          << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an "
+                     "invalid CU index ({2}).\n",
+                     NI.getUnitOffset(), EntryID, CUIndex);
       ++NumErrors;
       continue;
     }
@@ -1448,24 +1491,26 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
     uint64_t DIEOffset = CUOffset + *EntryOr->getDIEUnitOffset();
     DWARFDie DIE = DCtx.getDIEForOffset(DIEOffset);
     if (!DIE) {
-      aggregate("NameIndex references nonexisten DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x} references a "
-                         "non-existing DIE @ {2:x}.\n",
-                         NI.getUnitOffset(), EntryID, DIEOffset);
+      aggregate("NameIndex references nonexisten DIE")
+          << formatv("Name Index @ {0:x}: Entry @ {1:x} references a "
+                     "non-existing DIE @ {2:x}.\n",
+                     NI.getUnitOffset(), EntryID, DIEOffset);
       ++NumErrors;
       continue;
     }
     if (DIE.getDwarfUnit()->getOffset() != CUOffset) {
-      aggregate("Name index contains mismatched CU of DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of "
-                        "DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n",
-                         NI.getUnitOffset(), EntryID, DIEOffset, CUOffset,
-                         DIE.getDwarfUnit()->getOffset());
+      aggregate("Name index contains mismatched CU of DIE")
+          << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of "
+                     "DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n",
+                     NI.getUnitOffset(), EntryID, DIEOffset, CUOffset,
+                     DIE.getDwarfUnit()->getOffset());
       ++NumErrors;
     }
     if (DIE.getTag() != EntryOr->tag()) {
-      aggregate("Name Index contains mismatched Tag of DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Tag of "
-                         "DIE @ {2:x}: index - {3}; debug_info - {4}.\n",
-                         NI.getUnitOffset(), EntryID, DIEOffset, EntryOr->tag(),
-                         DIE.getTag());
+      aggregate("Name Index contains mismatched Tag of DIE") << formatv(
+          "Name Index @ {0:x}: Entry @ {1:x}: mismatched Tag of "
+          "DIE @ {2:x}: index - {3}; debug_info - {4}.\n",
+          NI.getUnitOffset(), EntryID, DIEOffset, EntryOr->tag(), DIE.getTag());
       ++NumErrors;
     }
 
@@ -1476,10 +1521,11 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
         DIE.getTag() == DW_TAG_inlined_subroutine;
     auto EntryNames = getNames(DIE, IncludeStrippedTemplateNames);
     if (!is_contained(EntryNames, Str)) {
-      aggregate("Name Index contains mismatched name of DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Name "
-                         "of DIE @ {2:x}: index - {3}; debug_info - {4}.\n",
-                         NI.getUnitOffset(), EntryID, DIEOffset, Str,
-                         make_range(EntryNames.begin(), EntryNames.end()));
+      aggregate("Name Index contains mismatched name of DIE")
+          << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Name "
+                     "of DIE @ {2:x}: index - {3}; debug_info - {4}.\n",
+                     NI.getUnitOffset(), EntryID, DIEOffset, Str,
+                     make_range(EntryNames.begin(), EntryNames.end()));
       ++NumErrors;
     }
   }
@@ -1487,19 +1533,19 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
                   [&](const DWARFDebugNames::SentinelError &) {
                     if (NumEntries > 0)
                       return;
-                    aggregate("NameIndex Name is not associated with any entries") << formatv("Name Index @ {0:x}: Name {1} ({2}) is "
-                                       "not associated with any entries.\n",
-                                       NI.getUnitOffset(), NTE.getIndex(), Str);
-                    ++NumErrors;
-                  },
-                  [&](const ErrorInfoBase &Info) {
-                    // Aggregation TODO:
-                    error()
-                        << formatv("Name Index @ {0:x}: Name {1} ({2}): {3}\n",
-                                   NI.getUnitOffset(), NTE.getIndex(), Str,
-                                   Info.message());
-                    ++NumErrors;
-                  });
+        aggregate("NameIndex Name is not associated with any entries")
+            << formatv("Name Index @ {0:x}: Name {1} ({2}) is "
+                       "not associated with any entries.\n",
+                       NI.getUnitOffset(), NTE.getIndex(), Str);
+        ++NumErrors;
+      },
+      [&](const ErrorInfoBase &Info) {
+        // Aggregation TODO:
+        error() << formatv("Name Index @ {0:x}: Name {1} ({2}): {3}\n",
+                           NI.getUnitOffset(), NTE.getIndex(), Str,
+                           Info.message());
+        ++NumErrors;
+      });
   return NumErrors;
 }
 
@@ -1625,10 +1671,10 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness(
     if (none_of(NI.equal_range(Name), [&](const DWARFDebugNames::Entry &E) {
           return E.getDIEUnitOffset() == DieUnitOffset;
         })) {
-      aggregate("Name Index DIE entry missing name") << formatv("Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with "
-                         "name {3} missing.\n",
-                         NI.getUnitOffset(), Die.getOffset(), Die.getTag(),
-                         Name);
+      aggregate("Name Index DIE entry missing name")
+          << formatv("Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with "
+                     "name {3} missing.\n",
+                     NI.getUnitOffset(), Die.getOffset(), Die.getTag(), Name);
       ++NumErrors;
     }
   }
@@ -1748,13 +1794,15 @@ bool DWARFVerifier::verifyDebugStrOffsets(
       if (!C)
         break;
       if (C.tell() + Length > DA.getData().size()) {
-        aggregate("Section contribution length exceeds available space") << formatv(
-            "{0}: contribution {1:X}: length exceeds available space "
-            "(contribution "
-            "offset ({1:X}) + length field space ({2:X}) + length ({3:X}) == "
-            "{4:X} > section size {5:X})\n",
-            SectionName, StartOffset, C.tell() - StartOffset, Length,
-            C.tell() + Length, DA.getData().size());
+        aggregate("Section contribution length exceeds available space")
+            << formatv(
+                   "{0}: contribution {1:X}: length exceeds available space "
+                   "(contribution "
+                   "offset ({1:X}) + length field space ({2:X}) + length "
+                   "({3:X}) == "
+                   "{4:X} > section size {5:X})\n",
+                   SectionName, StartOffset, C.tell() - StartOffset, Length,
+                   C.tell() + Length, DA.getData().size());
         Success = false;
         // Nothing more to do - no other contributions to try.
         break;
@@ -1762,8 +1810,9 @@ bool DWARFVerifier::verifyDebugStrOffsets(
       NextUnit = C.tell() + Length;
       uint8_t Version = DA.getU16(C);
       if (C && Version != 5) {
-        aggregate("Invalid Section version") << formatv("{0}: contribution {1:X}: invalid version {2}\n",
-                           SectionName, StartOffset, Version);
+        aggregate("Invalid Section version")
+            << formatv("{0}: contribution {1:X}: invalid version {2}\n",
+                       SectionName, StartOffset, Version);
         Success = false;
         // Can't parse the rest of this contribution, since we don't know the
         // version, but we can pick up with the next contribution.
@@ -1796,10 +1845,11 @@ bool DWARFVerifier::verifyDebugStrOffsets(
       }
       if (StrData[StrOff - 1] == '\0')
         continue;
-      aggregate("Section contribution contains invalid string offset") << formatv("{0}: contribution {1:X}: index {2:X}: invalid string "
-                         "offset *{3:X} == {4:X}, is neither zero nor "
-                         "immediately following a null character\n",
-                         SectionName, StartOffset, Index, OffOff, StrOff);
+      aggregate("Section contribution contains invalid string offset")
+          << formatv("{0}: contribution {1:X}: index {2:X}: invalid string "
+                     "offset *{3:X} == {4:X}, is neither zero nor "
+                     "immediately following a null character\n",
+                     SectionName, StartOffset, Index, OffOff, StrOff);
       Success = false;
     }
   }
@@ -1812,64 +1862,22 @@ bool DWARFVerifier::verifyDebugStrOffsets(
   return Success;
 }
 
-std::string DWARFVerifier::ErrorAggregator::Clean(const char *s) {
-  // Find the position of the first alphabetic character
-  const char *start = s;
-  while (!std::isalpha(*start)) {
-    ++start;
-  }
-
-  // Find the position of the last alphabetic character
-  const char *end = &start[std::strlen(start) - 1];
-  while (end >= start && !std::isalpha(*end)) {
-    --end;
-  }
-
-  // Create a std::string from the trimmed portion of the const char*
-  return std::string(start, end - start + 1);
-}
-
-raw_ostream &DWARFVerifier::ErrorAggregator::operator<<(const char *s) {
-  return Collect(Clean(s)) << s;
-}
-
-raw_ostream &DWARFVerifier::ErrorAggregator::Collect(const std::string &s) {
-  // Register the error in the aggregator
+void OutputCategoryAggregator::Report(
+    StringRef s, std::function<raw_ostream & Out> detailCallback) {
   UniqueErrors[s]++;
-
-  // If we have an output stream override, return it and reset it back to
-  // the default
-  raw_ostream *o = NextLineOverride;
-  NextLineOverride = nullptr;
-  return (o == nullptr) ? Verifier.error() : *o;
+  if (Output)
+    detailedCallback(*Output);
 }
 
-void DWARFVerifier::ErrorAggregator::Dump() {
-  for (auto &&[name, count] : UniqueErrors) {
-    Verifier.error() << "counter: " << name << ", count: " << count << "\n";
+void OutputCategoryAggregator::DumpAggregation(raw_ostream *o) {
+  for (auto &&[name, count] : Aggregator) {
+    *o << name << ": seen " << count << " time(s)\n";
   }
 }
 
-raw_ostream &DWARFVerifier::aggregate(const char *name) {
-  return ErrAggregation.Collect(name);
-}
-
-DWARFVerifier::ErrorAggregator &DWARFVerifier::ErrorAggregator::NextLineStreamOverride(raw_ostream &o) {
-  NextLineOverride = &o;
-  return *this;
-}
-
-DWARFVerifier::ErrorAggregator &DWARFVerifier::aggregate(raw_ostream &o) {
-  return ErrAggregation.NextLineStreamOverride(o);
-}
-
-DWARFVerifier::ErrorAggregator &DWARFVerifier::aggregate() {
-  return ErrAggregation;
-}
-
 void DWARFVerifier::finish() {
   if (DumpOpts.DumpAggregateErrors)
-    ErrAggregation.Dump();
+    Category.Dump(error());
 }
 
 raw_ostream &DWARFVerifier::error() const { return WithColor::error(OS); }

>From fb1f826d125956ed58e93bd079df86f734096df9 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 25 Jan 2024 11:41:14 -0800
Subject: [PATCH 05/11] Cleaned up: Non-verbose output is downright succinct!

---
 .../llvm/DebugInfo/DWARF/DWARFVerifier.h      |  13 +-
 llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp    | 682 +++++++++++-------
 llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp  |   2 +-
 3 files changed, 410 insertions(+), 287 deletions(-)

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index c2abc49c086947..d70175999e8df8 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -32,16 +32,15 @@ struct DWARFSection;
 
 class OutputCategoryAggregator {
 private:
-  std::map<std::string, int> Aggregator;
-  bool Output;
+  std::map<std::string, unsigned> Aggregation;
+  bool CallDetail;
 
 public:
-  OutputCategoryAggregator(bool DetailedOutput = false)
-      : Output(DetailedOutput) {}
-  void EnableDetail() { Output = true; }
-  void DisableDetail() { Output = false; }
+  OutputCategoryAggregator(bool callDetail = false) : CallDetail(callDetail) {}
+  void EnableDetail() { CallDetail = true; }
+  void DisableDetail() { CallDetail = false; }
   void Report(StringRef s, std::function<void()> detailCallback);
-  void DumpAggregation(raw_ostream *o);
+  void HandleAggregate(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 db9b96d18476c8..88c0e4ab2c8104 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -167,32 +167,24 @@ bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData,
   if (!ValidLength || !ValidVersion || !ValidAddrSize || !ValidAbbrevOffset ||
       !ValidType) {
     Success = false;
-    error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", UnitIndex,
-                      OffsetStart);
-    if (!ValidLength)
-      ErrorCategory.Report("Invalid Length in Unit Header", [&]() {
+    ErrorCategory.Report("Invalid Length in Unit Header", [&]() {
+      error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n",
+                        UnitIndex, OffsetStart);
+      if (!ValidLength)
         note() << "The length for this unit is too "
                   "large for the .debug_info provided.\n";
-      });
-    if (!ValidVersion) {
-      ErrorCategory.Report("Invalid 16 bit unit header", [&]() {
+      if (!ValidVersion) {
         note() << "The 16 bit unit header version is not valid.\n";
-      });
-    }
-    if (!ValidType) {
-      ErrorCategory.Report("Invalid unit type encoding", [&]() {
+      }
+      if (!ValidType) {
         note() << "The unit type encoding is not valid.\n";
-      });
-    }
-    if (!ValidAbbrevOffset)
-      ErrorCategory.Report("Invalid unit type encoding", [&]() {
+      }
+      if (!ValidAbbrevOffset)
         note() << "The offset into the .debug_abbrev section is "
                   "not valid.\n";
-      });
-    if (!ValidAddrSize)
-      ErrorCategory.Report("Invalid unit type encoding", [&]() {
+      if (!ValidAddrSize)
         note() << "The address size is unsupported.\n";
-      });
+    });
   }
   *Offset = OffsetStart + Length + (isUnitDWARF64 ? 12 : 4);
   return Success;
@@ -217,9 +209,9 @@ bool DWARFVerifier::verifyName(const DWARFDie &Die) {
             << formatv("         original: {0}\n"
                        "    reconstituted: {1}\n",
                        OriginalFullName, ReconstructedName);
+        dump(Die) << '\n';
+        dump(Die.getDwarfUnit()->getUnitDIE()) << '\n';
       });
-  dump(Die) << '\n';
-  dump(Die.getDwarfUnit()->getUnitDIE()) << '\n';
   return true;
 }
 
@@ -256,7 +248,7 @@ unsigned DWARFVerifier::verifyUnitContents(DWARFUnit &Unit,
 
   DWARFDie Die = Unit.getUnitDIE(/* ExtractUnitDIEOnly = */ false);
   if (!Die) {
-    ErrorCategory.Report("Compilation unity missing DIE", [&]() {
+    ErrorCategory.Report("Compilation unit missing DIE", [&]() {
       error() << "Compilation unit without DIE.\n";
     });
     NumUnitErrors++;
@@ -307,8 +299,8 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) {
       ErrorCategory.Report(
           "Call site nested entry within inlined subroutine", [&]() {
             error() << "Call site entry nested within inlined subroutine:";
+            Curr.dump(OS);
           });
-      Curr.dump(OS);
       return 1;
     }
   }
@@ -317,8 +309,8 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) {
     ErrorCategory.Report(
         "Call site entry not nexted within valid subprogram", [&]() {
           error() << "Call site entry not nested within a valid subprogram:";
+          Die.dump(OS);
         });
-    Die.dump(OS);
     return 1;
   }
 
@@ -331,9 +323,9 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) {
         "Subprogram with call site entry has no DW_AT_call attribute", [&]() {
           error()
               << "Subprogram with call site entry has no DW_AT_call attribute:";
+          Curr.dump(OS);
+          Die.dump(OS, /*indent*/ 1);
         });
-    Curr.dump(OS);
-    Die.dump(OS, /*indent*/ 1);
     return 1;
   }
 
@@ -479,7 +471,10 @@ unsigned DWARFVerifier::verifyIndex(StringRef Name,
       auto &M = *Sections[Col];
       auto I = M.find(SC.getOffset());
       if (I != M.end() && I.start() < (SC.getOffset() + SC.getLength())) {
-        ErrorCategory.Report("Overlapping index entries", [&]() {
+        StringRef Category = InfoColumnKind == DWARFSectionKind::DW_SECT_INFO
+                                 ? "Overlapping CU index entries"
+                                 : "Overlapping TU index entries";
+        ErrorCategory.Report(Category, [&]() {
           error() << llvm::formatv(
               "overlapping index entries for entries {0:x16} "
               "and {1:x16} for column {2}\n",
@@ -568,15 +563,14 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die,
   // For now, simply elide the range verification for the CU DIEs if we are
   // processing an object file.
 
-// KBF TODO: Continue here tomorrow:
-ErrorCategory.Report("", [&]() {
-      error
   if (!IsObjectFile || IsMachOObject || Die.getTag() != DW_TAG_compile_unit) {
     bool DumpDieAfterError = false;
     for (const auto &Range : Ranges) {
       if (!Range.valid()) {
         ++NumErrors;
-        aggregate() << "Invalid address range " << Range << "\n";
+        ErrorCategory.Report("Invalid address range", [&]() {
+          error() << "Invalid address range " << Range << "\n";
+        });
         DumpDieAfterError = true;
         continue;
       }
@@ -589,9 +583,11 @@ ErrorCategory.Report("", [&]() {
       // address: 0 or -1.
       if (auto PrevRange = RI.insert(Range)) {
         ++NumErrors;
-        aggregate() << "DIE has overlapping ranges in DW_AT_ranges attribute: "
-                << *PrevRange << " and " << Range << '\n';
-        DumpDieAfterError = true;
+        ErrorCategory.Report("DIE has overlapping DW_AT_ranges", [&]() {
+          error() << "DIE has overlapping ranges in DW_AT_ranges attribute: "
+                  << *PrevRange << " and " << Range << '\n';
+        });
+        DumpDieAfterError = DumpOpts.Verbose;
       }
     }
     if (DumpDieAfterError)
@@ -602,9 +598,11 @@ ErrorCategory.Report("", [&]() {
   const auto IntersectingChild = ParentRI.insert(RI);
   if (IntersectingChild != ParentRI.Children.end()) {
     ++NumErrors;
-    aggregate() << "DIEs have overlapping address ranges:";
-    dump(Die);
-    dump(IntersectingChild->Die) << '\n';
+    ErrorCategory.Report("DIEs have overlapping address ranges", [&]() {
+      error() << "DIEs have overlapping address ranges:";
+      dump(Die);
+      dump(IntersectingChild->Die) << '\n';
+    });
   }
 
   // Verify that ranges are contained within their parent.
@@ -613,9 +611,13 @@ ErrorCategory.Report("", [&]() {
                              ParentRI.Die.getTag() == DW_TAG_subprogram);
   if (ShouldBeContained && !ParentRI.contains(RI)) {
     ++NumErrors;
-    aggregate() << "DIE address ranges are not contained in its parent's ranges:";
-    dump(ParentRI.Die);
-    dump(Die, 2) << '\n';
+    ErrorCategory.Report(
+        "DIE address ranges are not contained by parent ranges", [&]() {
+          error()
+              << "DIE address ranges are not contained in its parent's ranges:";
+          dump(ParentRI.Die);
+          dump(Die, 2) << '\n';
+        });
   }
 
   // Recursively check children.
@@ -625,14 +627,15 @@ ErrorCategory.Report("", [&]() {
   return NumErrors;
 }
 
-// Aggregation TODO:
 unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die,
                                                  DWARFAttribute &AttrValue) {
   unsigned NumErrors = 0;
-  auto ReportError = [&](const Twine &TitleMsg) {
+  auto ReportError = [&](StringRef category, const Twine &TitleMsg) {
     ++NumErrors;
-    error() << TitleMsg << '\n';
-    dump(Die) << '\n';
+    ErrorCategory.Report(category, [&]() {
+      error() << TitleMsg << '\n';
+      dump(Die) << '\n';
+    });
   };
 
   const DWARFObject &DObj = DCtx.getDWARFObj();
@@ -649,23 +652,27 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die,
       if (U->isDWOUnit() && RangeSection.Data.empty())
         break;
       if (*SectionOffset >= RangeSection.Data.size())
-        ReportError(
-            "DW_AT_ranges offset is beyond " +
-            StringRef(DwarfVersion < 5 ? ".debug_ranges" : ".debug_rnglists") +
-            " bounds: " + llvm::formatv("{0:x8}", *SectionOffset));
+        ReportError("DW_AT_ranges offset out of bounds",
+                    "DW_AT_ranges offset is beyond " +
+                        StringRef(DwarfVersion < 5 ? ".debug_ranges"
+                                                   : ".debug_rnglists") +
+                        " bounds: " + llvm::formatv("{0:x8}", *SectionOffset));
       break;
     }
-    ReportError("DIE has invalid DW_AT_ranges encoding:");
+    ReportError("Invalid DW_AT_ranges encoding",
+                "DIE has invalid DW_AT_ranges encoding:");
     break;
   case DW_AT_stmt_list:
     // Make sure the offset in the DW_AT_stmt_list attribute is valid.
     if (auto SectionOffset = AttrValue.Value.getAsSectionOffset()) {
       if (*SectionOffset >= U->getLineSection().Data.size())
-        ReportError("DW_AT_stmt_list offset is beyond .debug_line bounds: " +
-                    llvm::formatv("{0:x8}", *SectionOffset));
+        ReportError("DW_AT_stmt_list offset out of bounds",
+                    "DW_AT_stmt_list offset is beyond .debug_line bounds: " +
+                        llvm::formatv("{0:x8}", *SectionOffset));
       break;
     }
-    ReportError("DIE has invalid DW_AT_stmt_list encoding:");
+    ReportError("Invalid DW_AT_stmt_list encoding",
+                "DIE has invalid DW_AT_stmt_list encoding:");
     break;
   case DW_AT_location: {
     // FIXME: It might be nice if there's a way to walk location expressions
@@ -689,14 +696,15 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die,
               return Op.isError();
             });
         if (Error || !Expression.verify(U))
-          ReportError("DIE contains invalid DWARF expression:");
+          ReportError("Invalid DWARF expressions",
+                      "DIE contains invalid DWARF expression:");
       }
     } else if (Error Err = handleErrors(
                    Loc.takeError(), [&](std::unique_ptr<ResolverError> E) {
                      return U->isDWOUnit() ? Error::success()
                                            : Error(std::move(E));
                    }))
-      ReportError(toString(std::move(Err)));
+      ReportError("Invalid DW_AT_location", toString(std::move(Err)));
     break;
   }
   case DW_AT_specification:
@@ -713,19 +721,21 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die,
       // This might be reference to a function declaration.
       if (DieTag == DW_TAG_GNU_call_site && RefTag == DW_TAG_subprogram)
         break;
-      ReportError("DIE with tag " + TagString(DieTag) + " has " +
-                  AttributeString(Attr) +
-                  " that points to DIE with "
-                  "incompatible tag " +
-                  TagString(RefTag));
+      ReportError("Incompatible DW_AT_abstract_origin tag reference",
+                  "DIE with tag " + TagString(DieTag) + " has " +
+                      AttributeString(Attr) +
+                      " that points to DIE with "
+                      "incompatible tag " +
+                      TagString(RefTag));
     }
     break;
   }
   case DW_AT_type: {
     DWARFDie TypeDie = Die.getAttributeValueAsReferencedDie(DW_AT_type);
     if (TypeDie && !isType(TypeDie.getTag())) {
-      ReportError("DIE has " + AttributeString(Attr) +
-                  " with incompatible tag " + TagString(TypeDie.getTag()));
+      ReportError("Incompatible DW_AT_type attribute tag",
+                  "DIE has " + AttributeString(Attr) +
+                      " with incompatible tag " + TagString(TypeDie.getTag()));
     }
     break;
   }
@@ -740,35 +750,43 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die,
           bool IsZeroIndexed = LT->Prologue.getVersion() >= 5;
           if (std::optional<uint64_t> LastFileIdx =
                   LT->getLastValidFileIndex()) {
-            ReportError("DIE has " + AttributeString(Attr) +
-                        " with an invalid file index " +
-                        llvm::formatv("{0}", *FileIdx) +
-                        " (valid values are [" + (IsZeroIndexed ? "0-" : "1-") +
-                        llvm::formatv("{0}", *LastFileIdx) + "])");
+            ReportError("Invalid file index in DW_AT_decl_file",
+                        "DIE has " + AttributeString(Attr) +
+                            " with an invalid file index " +
+                            llvm::formatv("{0}", *FileIdx) +
+                            " (valid values are [" +
+                            (IsZeroIndexed ? "0-" : "1-") +
+                            llvm::formatv("{0}", *LastFileIdx) + "])");
           } else {
-            ReportError("DIE has " + AttributeString(Attr) +
-                        " with an invalid file index " +
-                        llvm::formatv("{0}", *FileIdx) +
-                        " (the file table in the prologue is empty)");
+            ReportError("Invalid file index in DW_AT_decl_file",
+                        "DIE has " + AttributeString(Attr) +
+                            " with an invalid file index " +
+                            llvm::formatv("{0}", *FileIdx) +
+                            " (the file table in the prologue is empty)");
           }
         }
       } else {
-        ReportError("DIE has " + AttributeString(Attr) +
-                    " that references a file with index " +
-                    llvm::formatv("{0}", *FileIdx) +
-                    " and the compile unit has no line table");
+        ReportError(
+            "File index in DW_AT_decl_file reference CU with no line table",
+            "DIE has " + AttributeString(Attr) +
+                " that references a file with index " +
+                llvm::formatv("{0}", *FileIdx) +
+                " and the compile unit has no line table");
       }
     } else {
-      ReportError("DIE has " + AttributeString(Attr) +
-                  " with invalid encoding");
+      ReportError("Invalid encoding in DW_AT_decl_file",
+                  "DIE has " + AttributeString(Attr) +
+                      " with invalid encoding");
     }
     break;
   }
   case DW_AT_call_line:
   case DW_AT_decl_line: {
     if (!AttrValue.Value.getAsUnsignedConstant()) {
-      ReportError("DIE has " + AttributeString(Attr) +
-                  " with invalid encoding");
+      ReportError(
+          Attr == DW_AT_call_line ? "Invalid file index in DW_AT_decl_line"
+                                  : "Invalid file index in DW_AT_call_line",
+          "DIE has " + AttributeString(Attr) + " with invalid encoding");
     }
     break;
   }
@@ -799,12 +817,14 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
       auto CUOffset = AttrValue.Value.getRawUValue();
       if (CUOffset >= CUSize) {
         ++NumErrors;
-        aggregate("Invalid CU offset") << FormEncodingString(Form) << " CU offset "
-                << format("0x%08" PRIx64, CUOffset)
-                << " is invalid (must be less than CU size of "
-                << format("0x%08" PRIx64, CUSize) << "):\n";
-        Die.dump(OS, 0, DumpOpts);
-        dump(Die) << '\n';
+        ErrorCategory.Report("Invalid CU offset", [&]() {
+          error() << FormEncodingString(Form) << " CU offset "
+                  << format("0x%08" PRIx64, CUOffset)
+                  << " is invalid (must be less than CU size of "
+                  << format("0x%08" PRIx64, CUSize) << "):\n";
+          Die.dump(OS, 0, DumpOpts);
+          dump(Die) << '\n';
+        });
       } else {
         // Valid reference, but we will verify it points to an actual
         // DIE later.
@@ -821,9 +841,11 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
     if (RefVal) {
       if (*RefVal >= DieCU->getInfoSection().Data.size()) {
         ++NumErrors;
-        aggregate() << "DW_FORM_ref_addr offset beyond .debug_info "
-                   "bounds:\n";
-        dump(Die) << '\n';
+        ErrorCategory.Report("DW_FORM_ref_addr offset out of bounds", [&]() {
+          error() << "DW_FORM_ref_addr offset beyond .debug_info "
+                     "bounds:\n";
+          dump(Die) << '\n';
+        });
       } else {
         // Valid reference, but we will verify it points to an actual
         // DIE later.
@@ -841,9 +863,10 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
   case DW_FORM_line_strp: {
     if (Error E = AttrValue.Value.getAsCString().takeError()) {
       ++NumErrors;
-      // Aggregation TODO:
-      error() << toString(std::move(E)) << ":\n";
-      dump(Die) << '\n';
+      ErrorCategory.Report("Invalid DW_FORM attribute", [&]() {
+        error() << toString(std::move(E)) << ":\n";
+        dump(Die) << '\n';
+      });
     }
     break;
   }
@@ -867,11 +890,13 @@ unsigned DWARFVerifier::verifyDebugInfoReferences(
     if (GetDIEForOffset(Pair.first))
       continue;
     ++NumErrors;
-    aggregate() << "invalid DIE reference " << format("0x%08" PRIx64, Pair.first)
-            << ". Offset is in between DIEs:\n";
-    for (auto Offset : Pair.second)
-      dump(GetDIEForOffset(Offset)) << '\n';
-    OS << "\n";
+    ErrorCategory.Report("Invalid DIE reference", [&]() {
+      error() << "invalid DIE reference " << format("0x%08" PRIx64, Pair.first)
+              << ". Offset is in between DIEs:\n";
+      for (auto Offset : Pair.second)
+        dump(GetDIEForOffset(Offset)) << '\n';
+      OS << "\n";
+    });
   }
   return NumErrors;
 }
@@ -891,9 +916,11 @@ void DWARFVerifier::verifyDebugLineStmtOffsets() {
     if (LineTableOffset < DCtx.getDWARFObj().getLineSection().Data.size()) {
       if (!LineTable) {
         ++NumDebugLineErrors;
-        aggregate("Unparseable .debug_line entry") << ".debug_line[" << format("0x%08" PRIx64, LineTableOffset)
-                << "] was not able to be parsed for CU:\n";
-        dump(Die) << '\n';
+        ErrorCategory.Report("Unparseable .debug_line entry", [&]() {
+          error() << ".debug_line[" << format("0x%08" PRIx64, LineTableOffset)
+                  << "] was not able to be parsed for CU:\n";
+          dump(Die) << '\n';
+        });
         continue;
       }
     } else {
@@ -906,12 +933,14 @@ void DWARFVerifier::verifyDebugLineStmtOffsets() {
     auto Iter = StmtListToDie.find(LineTableOffset);
     if (Iter != StmtListToDie.end()) {
       ++NumDebugLineErrors;
-      aggregate("Identical DW_AT_stmt_list section offset") << "two compile unit DIEs, "
-              << format("0x%08" PRIx64, Iter->second.getOffset()) << " and "
-              << format("0x%08" PRIx64, Die.getOffset())
-              << ", have the same DW_AT_stmt_list section offset:\n";
-      dump(Iter->second);
-      dump(Die) << '\n';
+      ErrorCategory.Report("Identical DW_AT_stmt_list section offset", [&]() {
+        error() << "two compile unit DIEs, "
+                << format("0x%08" PRIx64, Iter->second.getOffset()) << " and "
+                << format("0x%08" PRIx64, Die.getOffset())
+                << ", have the same DW_AT_stmt_list section offset:\n";
+        dump(Iter->second);
+        dump(Die) << '\n';
+      });
       // Already verified this line table before, no need to do it again.
       continue;
     }
@@ -938,12 +967,16 @@ void DWARFVerifier::verifyDebugLineRows() {
       // Verify directory index.
       if (FileName.DirIdx > MaxDirIndex) {
         ++NumDebugLineErrors;
-        aggregate("Invalid index in .debug_line->prologue.file_names->dir_idx") << ".debug_line["
-                << format("0x%08" PRIx64,
-                          *toSectionOffset(Die.find(DW_AT_stmt_list)))
-                << "].prologue.file_names[" << FileIndex
-                << "].dir_idx contains an invalid index: " << FileName.DirIdx
-                << "\n";
+        ErrorCategory.Report(
+            "Invalid index in .debug_line->prologue.file_names->dir_idx",
+            [&]() {
+              error() << ".debug_line["
+                      << format("0x%08" PRIx64,
+                                *toSectionOffset(Die.find(DW_AT_stmt_list)))
+                      << "].prologue.file_names[" << FileIndex
+                      << "].dir_idx contains an invalid index: "
+                      << FileName.DirIdx << "\n";
+            });
       }
 
       // Check file paths for duplicates.
@@ -956,7 +989,7 @@ void DWARFVerifier::verifyDebugLineRows() {
       auto It = FullPathMap.find(FullPath);
       if (It == FullPathMap.end())
         FullPathMap[FullPath] = FileIndex;
-      else if (It->second != FileIndex) {
+      else if (It->second != FileIndex && DumpOpts.Verbose) {
         warn() << ".debug_line["
                << format("0x%08" PRIx64,
                          *toSectionOffset(Die.find(DW_AT_stmt_list)))
@@ -974,17 +1007,20 @@ void DWARFVerifier::verifyDebugLineRows() {
       // Verify row address.
       if (Row.Address.Address < PrevAddress) {
         ++NumDebugLineErrors;
-        aggregate("decreasing address between debug_line rows") << ".debug_line["
-                << format("0x%08" PRIx64,
-                          *toSectionOffset(Die.find(DW_AT_stmt_list)))
-                << "] row[" << RowIndex
-                << "] decreases in address from previous row:\n";
-
-        DWARFDebugLine::Row::dumpTableHeader(OS, 0);
-        if (RowIndex > 0)
-          LineTable->Rows[RowIndex - 1].dump(OS);
-        Row.dump(OS);
-        OS << '\n';
+        ErrorCategory.Report(
+            "decreasing address between debug_line rows", [&]() {
+              error() << ".debug_line["
+                      << format("0x%08" PRIx64,
+                                *toSectionOffset(Die.find(DW_AT_stmt_list)))
+                      << "] row[" << RowIndex
+                      << "] decreases in address from previous row:\n";
+
+              DWARFDebugLine::Row::dumpTableHeader(OS, 0);
+              if (RowIndex > 0)
+                LineTable->Rows[RowIndex - 1].dump(OS);
+              Row.dump(OS);
+              OS << '\n';
+            });
       }
 
       // If the prologue contains no file names and the line table has only one
@@ -995,16 +1031,18 @@ void DWARFVerifier::verifyDebugLineRows() {
            LineTable->Rows.size() != 1) &&
           !LineTable->hasFileAtIndex(Row.File)) {
         ++NumDebugLineErrors;
-        aggregate("Invalid file index in debug_line") << ".debug_line["
-                << format("0x%08" PRIx64,
-                          *toSectionOffset(Die.find(DW_AT_stmt_list)))
-                << "][" << RowIndex << "] has invalid file index " << Row.File
-                << " (valid values are [" << MinFileIndex << ','
-                << LineTable->Prologue.FileNames.size()
-                << (isDWARF5 ? ")" : "]") << "):\n";
-        DWARFDebugLine::Row::dumpTableHeader(OS, 0);
-        Row.dump(OS);
-        OS << '\n';
+        ErrorCategory.Report("Invalid file index in debug_line", [&]() {
+          error() << ".debug_line["
+                  << format("0x%08" PRIx64,
+                            *toSectionOffset(Die.find(DW_AT_stmt_list)))
+                  << "][" << RowIndex << "] has invalid file index " << Row.File
+                  << " (valid values are [" << MinFileIndex << ','
+                  << LineTable->Prologue.FileNames.size()
+                  << (isDWARF5 ? ")" : "]") << "):\n";
+          DWARFDebugLine::Row::dumpTableHeader(OS, 0);
+          Row.dump(OS);
+          OS << '\n';
+        });
       }
       if (Row.EndSequence)
         PrevAddress = 0;
@@ -1017,8 +1055,11 @@ void DWARFVerifier::verifyDebugLineRows() {
 
 DWARFVerifier::DWARFVerifier(raw_ostream &S, DWARFContext &D,
                              DIDumpOptions DumpOpts)
-    : OS(S), DCtx(D), ErrAggregation(*this), DumpOpts(std::move(DumpOpts)),
-      IsObjectFile(false), IsMachOObject(false) {
+    : OS(S), DCtx(D), DumpOpts(std::move(DumpOpts)), IsObjectFile(false),
+      IsMachOObject(false) {
+  if (DumpOpts.Verbose) {
+    ErrorCategory.EnableDetail();
+  }
   if (const auto *F = DCtx.getDWARFObj().getFile()) {
     IsObjectFile = F->isRelocatableObject();
     IsMachOObject = F->isMachO();
@@ -1045,14 +1086,17 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
 
   // Verify that the fixed part of the header is not too short.
   if (!AccelSectionData.isValidOffset(AccelTable.getSizeHdr())) {
-    aggregate() << "Section is too small to fit a section header.\n";
+    ErrorCategory.Report("Section is too small to fit a section header", [&]() {
+      error() << "Section is too small to fit a section header.\n";
+    });
     return 1;
   }
 
   // Verify that the section is not too short.
   if (Error E = AccelTable.extract()) {
-    // Aggregation TODO:
-    error() << toString(std::move(E)) << '\n';
+    std::string Msg = toString(std::move(E));
+    ErrorCategory.Report("Section is too small to fit a section header",
+                         [&]() { error() << Msg << '\n'; });
     return 1;
   }
 
@@ -1067,18 +1111,24 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
   for (uint32_t BucketIdx = 0; BucketIdx < NumBuckets; ++BucketIdx) {
     uint32_t HashIdx = AccelSectionData.getU32(&BucketsOffset);
     if (HashIdx >= NumHashes && HashIdx != UINT32_MAX) {
-      aggregate("Invalid hash index") << format("Bucket[%d] has invalid hash index: %u.\n", BucketIdx,
-                        HashIdx);
+      ErrorCategory.Report("Invalid hash index", [&]() {
+        error() << format("Bucket[%d] has invalid hash index: %u.\n", BucketIdx,
+                          HashIdx);
+      });
       ++NumErrors;
     }
   }
   uint32_t NumAtoms = AccelTable.getAtomsDesc().size();
   if (NumAtoms == 0) {
-    aggregate() << "No atoms: failed to read HashData.\n";
+    ErrorCategory.Report("No atoms", [&]() {
+      error() << "No atoms: failed to read HashData.\n";
+    });
     return 1;
   }
   if (!AccelTable.validateForms()) {
-    aggregate() << "Unsupported form: failed to read HashData.\n";
+    ErrorCategory.Report("Unsupported form", [&]() {
+      error() << "Unsupported form: failed to read HashData.\n";
+    });
     return 1;
   }
 
@@ -1089,9 +1139,11 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
     uint64_t HashDataOffset = AccelSectionData.getU32(&DataOffset);
     if (!AccelSectionData.isValidOffsetForDataOfSize(HashDataOffset,
                                                      sizeof(uint64_t))) {
-      aggregate("Invalid HashData offset") << format("Hash[%d] has invalid HashData offset: "
-                        "0x%08" PRIx64 ".\n",
-                        HashIdx, HashDataOffset);
+      ErrorCategory.Report("Invalid HashData offset", [&]() {
+        error() << format("Hash[%d] has invalid HashData offset: "
+                          "0x%08" PRIx64 ".\n",
+                          HashIdx, HashDataOffset);
+      });
       ++NumErrors;
     }
 
@@ -1115,21 +1167,25 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
           if (!Name)
             Name = "<NULL>";
 
-          aggregate("Invalid DIE offset") << format(
-              "%s Bucket[%d] Hash[%d] = 0x%08x "
-              "Str[%u] = 0x%08" PRIx64 " DIE[%d] = 0x%08" PRIx64 " "
-              "is not a valid DIE offset for \"%s\".\n",
-              SectionName, BucketIdx, HashIdx, Hash, StringCount, StrpOffset,
-              HashDataIdx, Offset, Name);
+          ErrorCategory.Report("Invalid DIE offset", [&]() {
+            error() << format(
+                "%s Bucket[%d] Hash[%d] = 0x%08x "
+                "Str[%u] = 0x%08" PRIx64 " DIE[%d] = 0x%08" PRIx64 " "
+                "is not a valid DIE offset for \"%s\".\n",
+                SectionName, BucketIdx, HashIdx, Hash, StringCount, StrpOffset,
+                HashDataIdx, Offset, Name);
+          });
 
           ++NumErrors;
           continue;
         }
         if ((Tag != dwarf::DW_TAG_null) && (Die.getTag() != Tag)) {
-          aggregate("Mismatched Tag in accellerator table") << "Tag " << dwarf::TagString(Tag)
-                  << " in accelerator table does not match Tag "
-                  << dwarf::TagString(Die.getTag()) << " of DIE[" << HashDataIdx
-                  << "].\n";
+          ErrorCategory.Report("Mismatched Tag in accellerator table", [&]() {
+            error() << "Tag " << dwarf::TagString(Tag)
+                    << " in accelerator table does not match Tag "
+                    << dwarf::TagString(Die.getTag()) << " of DIE["
+                    << HashDataIdx << "].\n";
+          });
           ++NumErrors;
         }
       }
@@ -1153,8 +1209,10 @@ DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) {
   unsigned NumErrors = 0;
   for (const DWARFDebugNames::NameIndex &NI : AccelTable) {
     if (NI.getCUCount() == 0) {
-      aggregate("Name Index doesn't index any CU") << formatv("Name Index @ {0:x} does not index any CU\n",
-                         NI.getUnitOffset());
+      ErrorCategory.Report("Name Index doesn't index any CU", [&]() {
+        error() << formatv("Name Index @ {0:x} does not index any CU\n",
+                           NI.getUnitOffset());
+      });
       ++NumErrors;
       continue;
     }
@@ -1163,17 +1221,22 @@ DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) {
       auto Iter = CUMap.find(Offset);
 
       if (Iter == CUMap.end()) {
-        aggregate("Name Index references non-existing CU") << formatv(
-            "Name Index @ {0:x} references a non-existing CU @ {1:x}\n",
-            NI.getUnitOffset(), Offset);
+        ErrorCategory.Report("Name Index references non-existing CU", [&]() {
+          error() << formatv(
+              "Name Index @ {0:x} references a non-existing CU @ {1:x}\n",
+              NI.getUnitOffset(), Offset);
+        });
         ++NumErrors;
         continue;
       }
 
       if (Iter->second != NotIndexed) {
-        aggregate("Duplicate Name Index") << formatv("Name Index @ {0:x} references a CU @ {1:x}, but "
-                           "this CU is already indexed by Name Index @ {2:x}\n",
-                           NI.getUnitOffset(), Offset, Iter->second);
+        ErrorCategory.Report("Duplicate Name Index", [&]() {
+          error() << formatv(
+              "Name Index @ {0:x} references a CU @ {1:x}, but "
+              "this CU is already indexed by Name Index @ {2:x}\n",
+              NI.getUnitOffset(), Offset, Iter->second);
+        });
         continue;
       }
       Iter->second = NI.getUnitOffset();
@@ -1214,9 +1277,12 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
   for (uint32_t Bucket = 0, End = NI.getBucketCount(); Bucket < End; ++Bucket) {
     uint32_t Index = NI.getBucketArrayEntry(Bucket);
     if (Index > NI.getNameCount()) {
-      aggregate("Name Index Bucket contains invalid value") << formatv("Bucket {0} of Name Index @ {1:x} contains invalid "
-                         "value {2}. Valid range is [0, {3}].\n",
-                         Bucket, NI.getUnitOffset(), Index, NI.getNameCount());
+      ErrorCategory.Report("Name Index Bucket contains invalid value", [&]() {
+        error() << formatv("Bucket {0} of Name Index @ {1:x} contains invalid "
+                           "value {2}. Valid range is [0, {3}].\n",
+                           Bucket, NI.getUnitOffset(), Index,
+                           NI.getNameCount());
+      });
       ++NumErrors;
       continue;
     }
@@ -1249,9 +1315,11 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
     // will not match because we have already verified that the name's hash
     // puts it into the previous bucket.)
     if (B.Index > NextUncovered) {
-      aggregate("Name table entries uncovered by hash table") << formatv("Name Index @ {0:x}: Name table entries [{1}, {2}] "
-                         "are not covered by the hash table.\n",
-                         NI.getUnitOffset(), NextUncovered, B.Index - 1);
+      ErrorCategory.Report("Name table entries uncovered by hash table", [&]() {
+        error() << formatv("Name Index @ {0:x}: Name table entries [{1}, {2}] "
+                           "are not covered by the hash table.\n",
+                           NI.getUnitOffset(), NextUncovered, B.Index - 1);
+      });
       ++NumErrors;
     }
     uint32_t Idx = B.Index;
@@ -1267,11 +1335,13 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
     // bucket as empty.
     uint32_t FirstHash = NI.getHashArrayEntry(Idx);
     if (FirstHash % NI.getBucketCount() != B.Bucket) {
-      aggregate("Name Index point to mismatched hash value") << formatv(
-          "Name Index @ {0:x}: Bucket {1} is not empty but points to a "
-          "mismatched hash value {2:x} (belonging to bucket {3}).\n",
-          NI.getUnitOffset(), B.Bucket, FirstHash,
-          FirstHash % NI.getBucketCount());
+      ErrorCategory.Report("Name Index point to mismatched hash value", [&]() {
+        error() << formatv(
+            "Name Index @ {0:x}: Bucket {1} is not empty but points to a "
+            "mismatched hash value {2:x} (belonging to bucket {3}).\n",
+            NI.getUnitOffset(), B.Bucket, FirstHash,
+            FirstHash % NI.getBucketCount());
+      });
       ++NumErrors;
     }
 
@@ -1285,11 +1355,14 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
 
       const char *Str = NI.getNameTableEntry(Idx).getString();
       if (caseFoldingDjbHash(Str) != Hash) {
-        aggregate("String hash doesn't match Name Index hash") << formatv("Name Index @ {0:x}: String ({1}) at index {2} "
-                           "hashes to {3:x}, but "
-                           "the Name Index hash is {4:x}\n",
-                           NI.getUnitOffset(), Str, Idx,
-                           caseFoldingDjbHash(Str), Hash);
+        ErrorCategory.Report(
+            "String hash doesn't match Name Index hash", [&]() {
+              error() << formatv(
+                  "Name Index @ {0:x}: String ({1}) at index {2} "
+                  "hashes to {3:x}, but "
+                  "the Name Index hash is {4:x}\n",
+                  NI.getUnitOffset(), Str, Idx, caseFoldingDjbHash(Str), Hash);
+            });
         ++NumErrors;
       }
 
@@ -1305,19 +1378,23 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(
     DWARFDebugNames::AttributeEncoding AttrEnc) {
   StringRef FormName = dwarf::FormEncodingString(AttrEnc.Form);
   if (FormName.empty()) {
-    aggregate("Unknown NameIndex Abbreviation") << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an "
-                       "unknown form: {3}.\n",
-                       NI.getUnitOffset(), Abbr.Code, AttrEnc.Index,
-                       AttrEnc.Form);
+    ErrorCategory.Report("Unknown NameIndex Abbreviation", [&]() {
+      error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an "
+                         "unknown form: {3}.\n",
+                         NI.getUnitOffset(), Abbr.Code, AttrEnc.Index,
+                         AttrEnc.Form);
+    });
     return 1;
   }
 
   if (AttrEnc.Index == DW_IDX_type_hash) {
     if (AttrEnc.Form != dwarf::DW_FORM_data8) {
-      aggregate("Unexpected NameIndex Abbreviation") << formatv(
-          "NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_type_hash "
-          "uses an unexpected form {2} (should be {3}).\n",
-          NI.getUnitOffset(), Abbr.Code, AttrEnc.Form, dwarf::DW_FORM_data8);
+      ErrorCategory.Report("Unexpected NameIndex Abbreviation", [&]() {
+        error() << formatv(
+            "NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_type_hash "
+            "uses an unexpected form {2} (should be {3}).\n",
+            NI.getUnitOffset(), Abbr.Code, AttrEnc.Form, dwarf::DW_FORM_data8);
+      });
       return 1;
     }
     return 0;
@@ -1327,10 +1404,13 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(
     constexpr static auto AllowedForms = {dwarf::Form::DW_FORM_flag_present,
                                           dwarf::Form::DW_FORM_ref4};
     if (!is_contained(AllowedForms, AttrEnc.Form)) {
-      aggregate("Unexpected NameIndex Abbreviation") << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_parent "
-                         "uses an unexpected form {2} (should be "
-                         "DW_FORM_ref4 or DW_FORM_flag_present).\n",
-                         NI.getUnitOffset(), Abbr.Code, AttrEnc.Form);
+      ErrorCategory.Report("Unexpected NameIndex Abbreviation", [&]() {
+        error() << formatv(
+            "NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_parent "
+            "uses an unexpected form {2} (should be "
+            "DW_FORM_ref4 or DW_FORM_flag_present).\n",
+            NI.getUnitOffset(), Abbr.Code, AttrEnc.Form);
+      });
       return 1;
     }
     return 0;
@@ -1362,10 +1442,12 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(
   }
 
   if (!DWARFFormValue(AttrEnc.Form).isFormClass(Iter->Class)) {
-    aggregate("Unexpected NameIndex Abbreviation") << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an "
-                       "unexpected form {3} (expected form class {4}).\n",
-                       NI.getUnitOffset(), Abbr.Code, AttrEnc.Index,
-                       AttrEnc.Form, Iter->ClassName);
+    ErrorCategory.Report("Unexpected NameIndex Abbreviation", [&]() {
+      error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an "
+                         "unexpected form {3} (expected form class {4}).\n",
+                         NI.getUnitOffset(), Abbr.Code, AttrEnc.Index,
+                         AttrEnc.Form, Iter->ClassName);
+    });
     return 1;
   }
   return 0;
@@ -1391,9 +1473,13 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) {
     SmallSet<unsigned, 5> Attributes;
     for (const auto &AttrEnc : Abbrev.Attributes) {
       if (!Attributes.insert(AttrEnc.Index).second) {
-        aggregate("NameIndex Abbreviateion contains multiple attributes") << formatv("NameIndex @ {0:x}: Abbreviation {1:x} contains "
-                           "multiple {2} attributes.\n",
-                           NI.getUnitOffset(), Abbrev.Code, AttrEnc.Index);
+        ErrorCategory.Report(
+            "NameIndex Abbreviateion contains multiple attributes", [&]() {
+              error() << formatv(
+                  "NameIndex @ {0:x}: Abbreviation {1:x} contains "
+                  "multiple {2} attributes.\n",
+                  NI.getUnitOffset(), Abbrev.Code, AttrEnc.Index);
+            });
         ++NumErrors;
         continue;
       }
@@ -1401,16 +1487,20 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) {
     }
 
     if (NI.getCUCount() > 1 && !Attributes.count(dwarf::DW_IDX_compile_unit)) {
-      aggregate("Abbreviation contains no attribute") << formatv("NameIndex @ {0:x}: Indexing multiple compile units "
-                         "and abbreviation {1:x} has no {2} attribute.\n",
-                         NI.getUnitOffset(), Abbrev.Code,
-                         dwarf::DW_IDX_compile_unit);
+      ErrorCategory.Report("Abbreviation contains no attribute", [&]() {
+        error() << formatv("NameIndex @ {0:x}: Indexing multiple compile units "
+                           "and abbreviation {1:x} has no {2} attribute.\n",
+                           NI.getUnitOffset(), Abbrev.Code,
+                           dwarf::DW_IDX_compile_unit);
+      });
       ++NumErrors;
     }
     if (!Attributes.count(dwarf::DW_IDX_die_offset)) {
-      aggregate("Abbreviate in NameIndex missing attribute") << formatv(
-          "NameIndex @ {0:x}: Abbreviation {1:x} has no {2} attribute.\n",
-          NI.getUnitOffset(), Abbrev.Code, dwarf::DW_IDX_die_offset);
+      ErrorCategory.Report("Abbreviate in NameIndex missing attribute", [&]() {
+        error() << formatv(
+            "NameIndex @ {0:x}: Abbreviation {1:x} has no {2} attribute.\n",
+            NI.getUnitOffset(), Abbrev.Code, dwarf::DW_IDX_die_offset);
+      });
       ++NumErrors;
     }
   }
@@ -1464,9 +1554,11 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
 
   const char *CStr = NTE.getString();
   if (!CStr) {
-    aggregate("Unable to get string associated with name") << formatv(
-        "Name Index @ {0:x}: Unable to get string associated with name {1}.\n",
-        NI.getUnitOffset(), NTE.getIndex());
+    ErrorCategory.Report("Unable to get string associated with name", [&]() {
+      error() << formatv("Name Index @ {0:x}: Unable to get string associated "
+                         "with name {1}.\n",
+                         NI.getUnitOffset(), NTE.getIndex());
+    });
     return 1;
   }
   StringRef Str(CStr);
@@ -1480,10 +1572,11 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
                                 EntryOr = NI.getEntry(&NextEntryID)) {
     uint32_t CUIndex = *EntryOr->getCUIndex();
     if (CUIndex > NI.getCUCount()) {
-      aggregate("Name Index entry contains invalid CU index")
-          << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an "
-                     "invalid CU index ({2}).\n",
-                     NI.getUnitOffset(), EntryID, CUIndex);
+      ErrorCategory.Report("Name Index entry contains invalid CU index", [&]() {
+        error() << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an "
+                           "invalid CU index ({2}).\n",
+                           NI.getUnitOffset(), EntryID, CUIndex);
+      });
       ++NumErrors;
       continue;
     }
@@ -1491,26 +1584,32 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
     uint64_t DIEOffset = CUOffset + *EntryOr->getDIEUnitOffset();
     DWARFDie DIE = DCtx.getDIEForOffset(DIEOffset);
     if (!DIE) {
-      aggregate("NameIndex references nonexisten DIE")
-          << formatv("Name Index @ {0:x}: Entry @ {1:x} references a "
-                     "non-existing DIE @ {2:x}.\n",
-                     NI.getUnitOffset(), EntryID, DIEOffset);
+      ErrorCategory.Report("NameIndex references nonexisten DIE", [&]() {
+        error() << formatv("Name Index @ {0:x}: Entry @ {1:x} references a "
+                           "non-existing DIE @ {2:x}.\n",
+                           NI.getUnitOffset(), EntryID, DIEOffset);
+      });
       ++NumErrors;
       continue;
     }
     if (DIE.getDwarfUnit()->getOffset() != CUOffset) {
-      aggregate("Name index contains mismatched CU of DIE")
-          << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of "
-                     "DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n",
-                     NI.getUnitOffset(), EntryID, DIEOffset, CUOffset,
-                     DIE.getDwarfUnit()->getOffset());
+      ErrorCategory.Report("Name index contains mismatched CU of DIE", [&]() {
+        error() << formatv(
+            "Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of "
+            "DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n",
+            NI.getUnitOffset(), EntryID, DIEOffset, CUOffset,
+            DIE.getDwarfUnit()->getOffset());
+      });
       ++NumErrors;
     }
     if (DIE.getTag() != EntryOr->tag()) {
-      aggregate("Name Index contains mismatched Tag of DIE") << formatv(
-          "Name Index @ {0:x}: Entry @ {1:x}: mismatched Tag of "
-          "DIE @ {2:x}: index - {3}; debug_info - {4}.\n",
-          NI.getUnitOffset(), EntryID, DIEOffset, EntryOr->tag(), DIE.getTag());
+      ErrorCategory.Report("Name Index contains mismatched Tag of DIE", [&]() {
+        error() << formatv(
+            "Name Index @ {0:x}: Entry @ {1:x}: mismatched Tag of "
+            "DIE @ {2:x}: index - {3}; debug_info - {4}.\n",
+            NI.getUnitOffset(), EntryID, DIEOffset, EntryOr->tag(),
+            DIE.getTag());
+      });
       ++NumErrors;
     }
 
@@ -1521,11 +1620,12 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
         DIE.getTag() == DW_TAG_inlined_subroutine;
     auto EntryNames = getNames(DIE, IncludeStrippedTemplateNames);
     if (!is_contained(EntryNames, Str)) {
-      aggregate("Name Index contains mismatched name of DIE")
-          << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Name "
-                     "of DIE @ {2:x}: index - {3}; debug_info - {4}.\n",
-                     NI.getUnitOffset(), EntryID, DIEOffset, Str,
-                     make_range(EntryNames.begin(), EntryNames.end()));
+      ErrorCategory.Report("Name Index contains mismatched name of DIE", [&]() {
+        error() << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Name "
+                           "of DIE @ {2:x}: index - {3}; debug_info - {4}.\n",
+                           NI.getUnitOffset(), EntryID, DIEOffset, Str,
+                           make_range(EntryNames.begin(), EntryNames.end()));
+      });
       ++NumErrors;
     }
   }
@@ -1533,17 +1633,20 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
                   [&](const DWARFDebugNames::SentinelError &) {
                     if (NumEntries > 0)
                       return;
-        aggregate("NameIndex Name is not associated with any entries")
-            << formatv("Name Index @ {0:x}: Name {1} ({2}) is "
-                       "not associated with any entries.\n",
-                       NI.getUnitOffset(), NTE.getIndex(), Str);
+        ErrorCategory.Report(
+            "NameIndex Name is not associated with any entries", [&]() {
+              error() << formatv("Name Index @ {0:x}: Name {1} ({2}) is "
+                                 "not associated with any entries.\n",
+                                 NI.getUnitOffset(), NTE.getIndex(), Str);
+            });
         ++NumErrors;
       },
       [&](const ErrorInfoBase &Info) {
-        // Aggregation TODO:
-        error() << formatv("Name Index @ {0:x}: Name {1} ({2}): {3}\n",
-                           NI.getUnitOffset(), NTE.getIndex(), Str,
-                           Info.message());
+        ErrorCategory.Report("Uncategorized NameIndex error", [&]() {
+          error() << formatv("Name Index @ {0:x}: Name {1} ({2}): {3}\n",
+                             NI.getUnitOffset(), NTE.getIndex(), Str,
+                             Info.message());
+        });
         ++NumErrors;
       });
   return NumErrors;
@@ -1671,10 +1774,12 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness(
     if (none_of(NI.equal_range(Name), [&](const DWARFDebugNames::Entry &E) {
           return E.getDIEUnitOffset() == DieUnitOffset;
         })) {
-      aggregate("Name Index DIE entry missing name")
-          << 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", [&]() {
+        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;
     }
   }
@@ -1693,8 +1798,9 @@ unsigned DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection,
   // This verifies that we can read individual name indices and their
   // abbreviation tables.
   if (Error E = AccelTable.extract()) {
-    // Aggregate TODO:
-    error() << toString(std::move(E)) << '\n';
+    std::string Msg = toString(std::move(E));
+    ErrorCategory.Report("Accelerator Table Error",
+                         [&]() { error() << Msg << '\n'; });
     return 1;
   }
 
@@ -1794,15 +1900,17 @@ bool DWARFVerifier::verifyDebugStrOffsets(
       if (!C)
         break;
       if (C.tell() + Length > DA.getData().size()) {
-        aggregate("Section contribution length exceeds available space")
-            << formatv(
-                   "{0}: contribution {1:X}: length exceeds available space "
-                   "(contribution "
-                   "offset ({1:X}) + length field space ({2:X}) + length "
-                   "({3:X}) == "
-                   "{4:X} > section size {5:X})\n",
-                   SectionName, StartOffset, C.tell() - StartOffset, Length,
-                   C.tell() + Length, DA.getData().size());
+        ErrorCategory.Report(
+            "Section contribution length exceeds available space", [&]() {
+              error() << formatv(
+                  "{0}: contribution {1:X}: length exceeds available space "
+                  "(contribution "
+                  "offset ({1:X}) + length field space ({2:X}) + length "
+                  "({3:X}) == "
+                  "{4:X} > section size {5:X})\n",
+                  SectionName, StartOffset, C.tell() - StartOffset, Length,
+                  C.tell() + Length, DA.getData().size());
+            });
         Success = false;
         // Nothing more to do - no other contributions to try.
         break;
@@ -1810,9 +1918,10 @@ bool DWARFVerifier::verifyDebugStrOffsets(
       NextUnit = C.tell() + Length;
       uint8_t Version = DA.getU16(C);
       if (C && Version != 5) {
-        aggregate("Invalid Section version")
-            << formatv("{0}: contribution {1:X}: invalid version {2}\n",
-                       SectionName, StartOffset, Version);
+        ErrorCategory.Report("Invalid Section version", [&]() {
+          error() << formatv("{0}: contribution {1:X}: invalid version {2}\n",
+                             SectionName, StartOffset, Version);
+        });
         Success = false;
         // Can't parse the rest of this contribution, since we don't know the
         // version, but we can pick up with the next contribution.
@@ -1824,10 +1933,12 @@ bool DWARFVerifier::verifyDebugStrOffsets(
     DA.setAddressSize(OffsetByteSize);
     uint64_t Remainder = (Length - 4) % OffsetByteSize;
     if (Remainder != 0) {
-      aggregate("Invalid section contribution length") << formatv(
-          "{0}: contribution {1:X}: invalid length ((length ({2:X}) "
-          "- header (0x4)) % offset size {3:X} == {4:X} != 0)\n",
-          SectionName, StartOffset, Length, OffsetByteSize, Remainder);
+      ErrorCategory.Report("Invalid section contribution length", [&]() {
+        error() << formatv(
+            "{0}: contribution {1:X}: invalid length ((length ({2:X}) "
+            "- header (0x4)) % offset size {3:X} == {4:X} != 0)\n",
+            SectionName, StartOffset, Length, OffsetByteSize, Remainder);
+      });
       Success = false;
     }
     for (uint64_t Index = 0; C && C.tell() + OffsetByteSize <= NextUnit; ++Index) {
@@ -1837,47 +1948,60 @@ bool DWARFVerifier::verifyDebugStrOffsets(
       if (StrOff == 0)
         continue;
       if (StrData.size() <= StrOff) {
-        error() << formatv(
-            "{0}: contribution {1:X}: index {2:X}: invalid string "
-            "offset *{3:X} == {4:X}, is beyond the bounds of the string section of length {5:X}\n",
-            SectionName, StartOffset, Index, OffOff, StrOff, StrData.size());
+        ErrorCategory.Report(
+            "String offset out of bounds of string section", [&]() {
+              error() << formatv(
+                  "{0}: contribution {1:X}: index {2:X}: invalid string "
+                  "offset *{3:X} == {4:X}, is beyond the bounds of the string "
+                  "section of length {5:X}\n",
+                  SectionName, StartOffset, Index, OffOff, StrOff,
+                  StrData.size());
+            });
         continue;
       }
       if (StrData[StrOff - 1] == '\0')
         continue;
-      aggregate("Section contribution contains invalid string offset")
-          << formatv("{0}: contribution {1:X}: index {2:X}: invalid string "
-                     "offset *{3:X} == {4:X}, is neither zero nor "
-                     "immediately following a null character\n",
-                     SectionName, StartOffset, Index, OffOff, StrOff);
+      ErrorCategory.Report(
+          "Section contribution contains invalid string offset", [&]() {
+            error() << formatv(
+                "{0}: contribution {1:X}: index {2:X}: invalid string "
+                "offset *{3:X} == {4:X}, is neither zero nor "
+                "immediately following a null character\n",
+                SectionName, StartOffset, Index, OffOff, StrOff);
+          });
       Success = false;
     }
   }
 
   if (Error E = C.takeError()) {
-    // Aggregate TODO:
-    error() << SectionName << ": " << toString(std::move(E)) << '\n';
-    return false;
+    std::string Msg = toString(std::move(E));
+    ErrorCategory.Report("String offset error", [&]() {
+      error() << SectionName << ": " << Msg << '\n';
+      return false;
+    });
   }
   return Success;
 }
 
 void OutputCategoryAggregator::Report(
-    StringRef s, std::function<raw_ostream & Out> detailCallback) {
-  UniqueErrors[s]++;
-  if (Output)
-    detailedCallback(*Output);
+    StringRef s, std::function<void(void)> detailCallback) {
+  Aggregation[std::string(s)]++;
+  if (CallDetail)
+    detailCallback();
 }
 
-void OutputCategoryAggregator::DumpAggregation(raw_ostream *o) {
-  for (auto &&[name, count] : Aggregator) {
-    *o << name << ": seen " << count << " time(s)\n";
+void OutputCategoryAggregator::HandleAggregate(
+    std::function<void(StringRef, unsigned)> handleCounts) {
+  for (auto &&[name, count] : Aggregation) {
+    handleCounts(name, count);
   }
 }
 
 void DWARFVerifier::finish() {
   if (DumpOpts.DumpAggregateErrors)
-    Category.Dump(error());
+    ErrorCategory.HandleAggregate([&](StringRef s, unsigned count) {
+      error() << s << ": " << count << '\n';
+    });
 }
 
 raw_ostream &DWARFVerifier::error() const { return WithColor::error(OS); }
diff --git a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
index 941df4eb184457..09eb659d4241b9 100644
--- a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
+++ b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
@@ -326,7 +326,7 @@ static DIDumpOptions getDumpOpts(DWARFContext &C) {
   DumpOpts.RecoverableErrorHandler = C.getRecoverableErrorHandler();
   // In -verify mode, print DIEs without children in error messages.
   if (Verify) {
-    DumpOpts.Verbose = true;
+    // DumpOpts.Verbose = true;
     return DumpOpts.noImplicitRecursion();
   }
   return DumpOpts;

>From f764c8633bdd4ba2454fbaec18b5a41637c084ea Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 25 Jan 2024 13:30:14 -0800
Subject: [PATCH 06/11] Set up the flags in a reasonable way

---
 llvm/include/llvm/DebugInfo/DIContext.h         |  2 +-
 .../llvm/DebugInfo/DWARF/DWARFVerifier.h        |  2 +-
 llvm/lib/DebugInfo/DWARF/DWARFContext.cpp       |  2 +-
 llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp      |  9 ++++++---
 llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp    | 17 ++++++++++++++++-
 5 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/llvm/include/llvm/DebugInfo/DIContext.h b/llvm/include/llvm/DebugInfo/DIContext.h
index 298628b551fb68..288ddf77bdfda7 100644
--- a/llvm/include/llvm/DebugInfo/DIContext.h
+++ b/llvm/include/llvm/DebugInfo/DIContext.h
@@ -205,7 +205,7 @@ struct DIDumpOptions {
   bool DisplayRawContents = false;
   bool IsEH = false;
   bool DumpNonSkeleton = false;
-  bool DumpAggregateErrors = true;
+  bool ShowAggregateErrors = false;
   std::function<llvm::StringRef(uint64_t DwarfRegNum, bool IsEH)>
       GetNameForDWARFReg;
 
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index d70175999e8df8..728eb869ff01b1 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -363,7 +363,7 @@ class DWARFVerifier {
       void (DWARFObject::*)(function_ref<void(const DWARFSection &)>) const);
 
   /// Emits any aggregate information collection, depending on the dump options
-  void finish();
+  void finish(bool Success);
 };
 
 static inline bool operator<(const DWARFVerifier::DieRangeInfo &LHS,
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index aa294444b46803..13d9103a76ec2a 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -1408,7 +1408,7 @@ bool DWARFContext::verify(raw_ostream &OS, DIDumpOptions DumpOpts) {
   if (DumpOpts.DumpType & DIDT_DebugStrOffsets)
     Success &= verifier.handleDebugStrOffsets();
   Success &= verifier.handleAccelTables();
-  verifier.finish();
+  verifier.finish(Success);
   return Success;
 }
 
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 88c0e4ab2c8104..56dfeef1f13508 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -1997,11 +1997,14 @@ void OutputCategoryAggregator::HandleAggregate(
   }
 }
 
-void DWARFVerifier::finish() {
-  if (DumpOpts.DumpAggregateErrors)
+void DWARFVerifier::finish(bool Success) {
+  if (!Success && DumpOpts.ShowAggregateErrors) {
+    error() << "Aggregated error category counts:\n";
     ErrorCategory.HandleAggregate([&](StringRef s, unsigned count) {
-      error() << s << ": " << count << '\n';
+      error() << "Error category '" << s << "' occurred " << count
+              << " time(s).\n";
     });
+  }
 }
 
 raw_ostream &DWARFVerifier::error() const { return WithColor::error(OS); }
diff --git a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
index 09eb659d4241b9..55d484f9136315 100644
--- a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
+++ b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
@@ -276,6 +276,14 @@ static cl::opt<bool>
                 cat(DwarfDumpCategory));
 static opt<bool> Verify("verify", desc("Verify the DWARF debug info."),
                         cat(DwarfDumpCategory));
+static opt<bool> OnlyAggregateErrors(
+    "only-aggregate-errors",
+    desc("Only display the aggregate errors when verifying."),
+    cat(DwarfDumpCategory));
+static opt<bool> NoAggregateErrors(
+    "no-aggregate-errors",
+    desc("Do not display the aggregate errors when verifying."),
+    cat(DwarfDumpCategory));
 static opt<bool> Quiet("quiet", desc("Use with -verify to not emit to STDOUT."),
                        cat(DwarfDumpCategory));
 static opt<bool> DumpUUID("uuid", desc("Show the UUID for each architecture."),
@@ -326,7 +334,8 @@ static DIDumpOptions getDumpOpts(DWARFContext &C) {
   DumpOpts.RecoverableErrorHandler = C.getRecoverableErrorHandler();
   // In -verify mode, print DIEs without children in error messages.
   if (Verify) {
-    // DumpOpts.Verbose = true;
+    DumpOpts.Verbose = !OnlyAggregateErrors;
+    DumpOpts.ShowAggregateErrors = !NoAggregateErrors;
     return DumpOpts.noImplicitRecursion();
   }
   return DumpOpts;
@@ -812,6 +821,12 @@ int main(int argc, char **argv) {
                           "-verbose is currently not supported";
     return 1;
   }
+  if (Verify && NoAggregateErrors && OnlyAggregateErrors) {
+    WithColor::error() << "incompatible arguments: specifying both "
+                          " -no-aggregate-errors, and -only-aggregate-errors "
+                          "is not supported";
+    return 1;
+  }
 
   std::error_code EC;
   ToolOutputFile OutputFile(OutputFilename, EC, sys::fs::OF_TextWithCRLF);

>From 855adb03310ea36cbf08d3f3e49d0f0f81a42b30 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 25 Jan 2024 14:05:58 -0800
Subject: [PATCH 07/11] A few final touches before tests

---
 .../llvm/DebugInfo/DWARF/DWARFVerifier.h      | 18 ++++++++-------
 llvm/lib/DebugInfo/DWARF/DWARFContext.cpp     |  2 +-
 llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp    | 23 ++++++++++---------
 3 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index 728eb869ff01b1..ad50fa92665ce5 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -33,14 +33,15 @@ struct DWARFSection;
 class OutputCategoryAggregator {
 private:
   std::map<std::string, unsigned> Aggregation;
-  bool CallDetail;
+  bool IncludeDetail;
 
 public:
-  OutputCategoryAggregator(bool callDetail = false) : CallDetail(callDetail) {}
-  void EnableDetail() { CallDetail = true; }
-  void DisableDetail() { CallDetail = false; }
+  OutputCategoryAggregator(bool includeDetail = false)
+      : IncludeDetail(includeDetail) {}
+  void EnableDetail() { IncludeDetail = true; }
+  void DisableDetail() { IncludeDetail = false; }
   void Report(StringRef s, std::function<void()> detailCallback);
-  void HandleAggregate(std::function<void(StringRef, unsigned)> handleCounts);
+  void EnumerateResults(std::function<void(StringRef, unsigned)> handleCounts);
 };
 
 /// A class that verifies DWARF debug information given a DWARF Context.
@@ -89,11 +90,12 @@ class DWARFVerifier {
     bool intersects(const DieRangeInfo &RHS) const;
   };
 
+private:
   raw_ostream &OS;
   DWARFContext &DCtx;
-  OutputCategoryAggregator ErrorCategory;
   DIDumpOptions DumpOpts;
   uint32_t NumDebugLineErrors = 0;
+  OutputCategoryAggregator ErrorCategory;
   // Used to relax some checks that do not currently work portably
   bool IsObjectFile;
   bool IsMachOObject;
@@ -362,8 +364,8 @@ class DWARFVerifier {
       StringRef SectionName, const DWARFSection &Section, StringRef StrData,
       void (DWARFObject::*)(function_ref<void(const DWARFSection &)>) const);
 
-  /// Emits any aggregate information collection, depending on the dump options
-  void finish(bool Success);
+  /// Emits any aggregate information collected, depending on the dump options
+  void summarize(bool Success);
 };
 
 static inline bool operator<(const DWARFVerifier::DieRangeInfo &LHS,
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index 13d9103a76ec2a..32a7fec0860be3 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -1408,7 +1408,7 @@ bool DWARFContext::verify(raw_ostream &OS, DIDumpOptions DumpOpts) {
   if (DumpOpts.DumpType & DIDT_DebugStrOffsets)
     Success &= verifier.handleDebugStrOffsets();
   Success &= verifier.handleAccelTables();
-  verifier.finish(Success);
+  verifier.summarize(Success);
   return Success;
 }
 
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 56dfeef1f13508..01278a83f3c582 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -307,7 +307,7 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) {
 
   if (!Curr.isValid()) {
     ErrorCategory.Report(
-        "Call site entry not nexted within valid subprogram", [&]() {
+        "Call site entry not nested within valid subprogram", [&]() {
           error() << "Call site entry not nested within a valid subprogram:";
           Die.dump(OS);
         });
@@ -339,9 +339,9 @@ unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) {
   Expected<const DWARFAbbreviationDeclarationSet *> AbbrDeclsOrErr =
       Abbrev->getAbbreviationDeclarationSet(0);
   if (!AbbrDeclsOrErr) {
-    ErrorCategory.Report("Abbreviation Declaration error", [&]() {
-      error() << toString(AbbrDeclsOrErr.takeError()) << "\n";
-    });
+    std::string ErrMsg = toString(AbbrDeclsOrErr.takeError());
+    ErrorCategory.Report("Abbreviation Declaration error",
+                         [&]() { error() << ErrMsg << "\n"; });
     return 1;
   }
 
@@ -863,8 +863,9 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
   case DW_FORM_line_strp: {
     if (Error E = AttrValue.Value.getAsCString().takeError()) {
       ++NumErrors;
+      std::string ErrMsg = toString(std::move(E));
       ErrorCategory.Report("Invalid DW_FORM attribute", [&]() {
-        error() << toString(std::move(E)) << ":\n";
+        error() << ErrMsg << ":\n";
         dump(Die) << '\n';
       });
     }
@@ -916,7 +917,7 @@ void DWARFVerifier::verifyDebugLineStmtOffsets() {
     if (LineTableOffset < DCtx.getDWARFObj().getLineSection().Data.size()) {
       if (!LineTable) {
         ++NumDebugLineErrors;
-        ErrorCategory.Report("Unparseable .debug_line entry", [&]() {
+        ErrorCategory.Report("Unparsable .debug_line entry", [&]() {
           error() << ".debug_line[" << format("0x%08" PRIx64, LineTableOffset)
                   << "] was not able to be parsed for CU:\n";
           dump(Die) << '\n';
@@ -1584,7 +1585,7 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
     uint64_t DIEOffset = CUOffset + *EntryOr->getDIEUnitOffset();
     DWARFDie DIE = DCtx.getDIEForOffset(DIEOffset);
     if (!DIE) {
-      ErrorCategory.Report("NameIndex references nonexisten DIE", [&]() {
+      ErrorCategory.Report("NameIndex references nonexistent DIE", [&]() {
         error() << formatv("Name Index @ {0:x}: Entry @ {1:x} references a "
                            "non-existing DIE @ {2:x}.\n",
                            NI.getUnitOffset(), EntryID, DIEOffset);
@@ -1986,21 +1987,21 @@ bool DWARFVerifier::verifyDebugStrOffsets(
 void OutputCategoryAggregator::Report(
     StringRef s, std::function<void(void)> detailCallback) {
   Aggregation[std::string(s)]++;
-  if (CallDetail)
+  if (IncludeDetail)
     detailCallback();
 }
 
-void OutputCategoryAggregator::HandleAggregate(
+void OutputCategoryAggregator::EnumerateResults(
     std::function<void(StringRef, unsigned)> handleCounts) {
   for (auto &&[name, count] : Aggregation) {
     handleCounts(name, count);
   }
 }
 
-void DWARFVerifier::finish(bool Success) {
+void DWARFVerifier::summarize(bool Success) {
   if (!Success && DumpOpts.ShowAggregateErrors) {
     error() << "Aggregated error category counts:\n";
-    ErrorCategory.HandleAggregate([&](StringRef s, unsigned count) {
+    ErrorCategory.EnumerateResults([&](StringRef s, unsigned count) {
       error() << "Error category '" << s << "' occurred " << count
               << " time(s).\n";
     });

>From e570d295dae56013b98c43418f3eab713798ef48 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Fri, 26 Jan 2024 08:31:35 -0800
Subject: [PATCH 08/11] Adding flags to make output the same as before for
 tests

---
 llvm/test/DebugInfo/X86/skeleton-unit-verify.s                | 4 +---
 llvm/test/DebugInfo/dwarfdump-accel.test                      | 2 +-
 .../tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml    | 2 +-
 .../llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml | 2 +-
 llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml  | 2 +-
 .../llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml      | 2 +-
 .../tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml   | 2 +-
 llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s          | 2 +-
 8 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/llvm/test/DebugInfo/X86/skeleton-unit-verify.s b/llvm/test/DebugInfo/X86/skeleton-unit-verify.s
index d9c7436d1c750c..28d287d20b1347 100644
--- a/llvm/test/DebugInfo/X86/skeleton-unit-verify.s
+++ b/llvm/test/DebugInfo/X86/skeleton-unit-verify.s
@@ -1,5 +1,5 @@
 # RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t.o
-# RUN: not llvm-dwarfdump --verify %t.o | FileCheck %s
+# RUN: not llvm-dwarfdump --no-aggregate-errors --verify %t.o | FileCheck %s
 
 # CHECK: Verifying .debug_abbrev...
 # CHECK-NEXT: Verifying .debug_info Unit Header Chain...
@@ -51,5 +51,3 @@
         .byte   2                       # Abbrev [2]
         .byte   0
 .Lcu_end1:
-
-
diff --git a/llvm/test/DebugInfo/dwarfdump-accel.test b/llvm/test/DebugInfo/dwarfdump-accel.test
index 27720b6c9b42a8..cda8e3f599970c 100644
--- a/llvm/test/DebugInfo/dwarfdump-accel.test
+++ b/llvm/test/DebugInfo/dwarfdump-accel.test
@@ -1,5 +1,5 @@
 RUN: llvm-dwarfdump -v %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s
-RUN: not llvm-dwarfdump -verify %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s --check-prefix=VERIFY
+RUN: not llvm-dwarfdump -no-aggregate-errors -verify %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s --check-prefix=VERIFY
 
 Gather some DIE indexes to verify the accelerator table contents.
 CHECK: .debug_info contents
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml
index b86623dd011d42..cadc35a09f06c3 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml
+++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml
@@ -1,4 +1,4 @@
-# RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error:
+# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error:
 
 # CHECK:      error: DIE has DW_AT_decl_file with an invalid file index 2 (valid values are [1-1]){{[[:space:]]}}
 # CHECK-NEXT: 0x0000001e: DW_TAG_subprogram
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml
index 3b56dca1bb090b..655a81aecc0ab7 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml
+++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml
@@ -1,4 +1,4 @@
-# RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error:
+# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error:
 
 # CHECK:      error: DIE has DW_AT_decl_file with an invalid file index 2 (the file table in the prologue is empty){{[[:space:]]}}
 # CHECK-NEXT: 0x0000001e: DW_TAG_subprogram
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml
index af55a3a7d10344..2d7b853871678d 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml
+++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml
@@ -1,4 +1,4 @@
-# RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error:
+# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error:
 
 # CHECK:      error: DIE has DW_AT_decl_file with invalid encoding{{[[:space:]]}}
 # CHECK-NEXT: 0x0000001a: DW_TAG_subprogram
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml
index a40959f4d0ded6..4fc99a04cf762c 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml
+++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml
@@ -39,7 +39,7 @@
 #
 # 0x00000066:   NULL
 
-# RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error:
+# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error:
 
 # CHECK: error: DIE has overlapping ranges in DW_AT_ranges attribute: [0x0000000000000000, 0x0000000000000020) and [0x0000000000000000, 0x0000000000000030)
 
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml
index 5188ac5a6d407f..bb61f431ff2da7 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml
+++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml
@@ -30,7 +30,7 @@
 # 0x00000056:   NULL
 
 
-# RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error:
+# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error:
 
 # CHECK: Verifying -:	file format Mach-O 64-bit x86-64
 # CHECK: Verifying .debug_abbrev...
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s b/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s
index 3941d9f1a7a57f..eb8dbd60538538 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s
@@ -1,5 +1,5 @@
 # RUN: llvm-mc %s -filetype obj -triple x86_64-linux-gnu -o - \
-# RUN: | not llvm-dwarfdump -v -verify - 2>&1 \
+# RUN: | not llvm-dwarfdump --no-aggregate-errors -v -verify - 2>&1 \
 # RUN: | FileCheck %s --implicit-check-not=error --implicit-check-not=warning
 
 # CHECK: Verifying dwo Units...

>From 8335f10f42f9fb6a5bc7e58fa30c1d5d8e0386bc Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Fri, 26 Jan 2024 12:16:50 -0800
Subject: [PATCH 09/11] A couple minor tweaks to keep the previous behavior
 'available'

---
 llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 01278a83f3c582..b3b99bc8babcc6 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -570,8 +570,8 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die,
         ++NumErrors;
         ErrorCategory.Report("Invalid address range", [&]() {
           error() << "Invalid address range " << Range << "\n";
+          DumpDieAfterError = true;
         });
-        DumpDieAfterError = true;
         continue;
       }
 
@@ -586,8 +586,8 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die,
         ErrorCategory.Report("DIE has overlapping DW_AT_ranges", [&]() {
           error() << "DIE has overlapping ranges in DW_AT_ranges attribute: "
                   << *PrevRange << " and " << Range << '\n';
+          DumpDieAfterError = true;
         });
-        DumpDieAfterError = DumpOpts.Verbose;
       }
     }
     if (DumpDieAfterError)
@@ -1058,7 +1058,7 @@ DWARFVerifier::DWARFVerifier(raw_ostream &S, DWARFContext &D,
                              DIDumpOptions DumpOpts)
     : OS(S), DCtx(D), DumpOpts(std::move(DumpOpts)), IsObjectFile(false),
       IsMachOObject(false) {
-  if (DumpOpts.Verbose) {
+  if (DumpOpts.Verbose || !DumpOpts.ShowAggregateErrors) {
     ErrorCategory.EnableDetail();
   }
   if (const auto *F = DCtx.getDWARFObj().getFile()) {

>From 99d93fcbf085a6865bfe995a09f25fb55679c800 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Fri, 26 Jan 2024 12:56:31 -0800
Subject: [PATCH 10/11] Added checking for aggregate counts for a dwarfdump
 test

---
 .../tools/llvm-dwarfdump/X86/verify_file_encoding.yaml    | 8 ++++++--
 llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s      | 7 ++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml
index 2d7b853871678d..6e1d751439d1ee 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml
+++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml
@@ -1,4 +1,4 @@
-# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error:
+# RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error:
 
 # CHECK:      error: DIE has DW_AT_decl_file with invalid encoding{{[[:space:]]}}
 # CHECK-NEXT: 0x0000001a: DW_TAG_subprogram
@@ -50,7 +50,11 @@
 # CHECK-NEXT:               DW_AT_decl_line   [DW_FORM_sdata] (3)
 # CHECK-NEXT:               DW_AT_call_file   [DW_FORM_sdata] (4)
 # CHECK-NEXT:               DW_AT_call_line   [DW_FORM_sdata] (5){{[[:space:]]}}
-
+# CHECK-NEXT: Verifying dwo Units...
+# CHECK-NEXT: error: Aggregated error category counts:
+# CHECK-NEXT: error: Error category 'Invalid encoding in DW_AT_decl_file' occurred 4 time(s).
+# CHECK-NEXT: error: Error category 'Invalid file index in DW_AT_call_line' occurred 1 time(s).
+# CHECK-NEXT: error: Error category 'Invalid file index in DW_AT_decl_line' occurred 1 time(s).
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s b/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s
index eb8dbd60538538..f6eea2d65ccd76 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s
@@ -1,5 +1,5 @@
 # RUN: llvm-mc %s -filetype obj -triple x86_64-linux-gnu -o - \
-# RUN: | not llvm-dwarfdump --no-aggregate-errors -v -verify - 2>&1 \
+# RUN: | not llvm-dwarfdump -v -verify - 2>&1 \
 # RUN: | FileCheck %s --implicit-check-not=error --implicit-check-not=warning
 
 # CHECK: Verifying dwo Units...
@@ -8,6 +8,11 @@
 # CHECK: error: Unsupported DW_AT_location encoding: DW_FORM_data1
 # FIXME: This should read "type unit" or just "unit" to be correct for this case/in general
 # CHECK: error: DIE has DW_AT_decl_file that references a file with index 1 and the compile unit has no line table
+# CHECK: error: Aggregated error category counts:
+# CHECK: error: Error category 'Compilation unit root DIE is not a unit DIE' occurred 1 time(s).
+# CHECK: error: Error category 'File index in DW_AT_decl_file reference CU with no line table' occurred 1 time(s).
+# CHECK: error: Error category 'Invalid DW_AT_location' occurred 1 time(s).
+# CHECK: error: Error category 'Mismatched unit type' occurred 1 time(s).
 # CHECK: Errors detected
 	.section	.debug_info.dwo,"e", at progbits
 	.long	.Ldebug_info_dwo_end1-.Ldebug_info_dwo_start1 # Length of Unit

>From 1d07b219adb6723be569e6b73ae7dc07bc1cd8e8 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Fri, 26 Jan 2024 13:07:20 -0800
Subject: [PATCH 11/11] Added a warning for the new flags

---
 llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
index 55d484f9136315..258f61f599acdf 100644
--- a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
+++ b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
@@ -827,6 +827,9 @@ int main(int argc, char **argv) {
                           "is not supported";
     return 1;
   }
+  if (!Verify && (NoAggregateErrors || OnlyAggregateErrors))
+    WithColor::warning() << "-no-aggregate-errors and -only-aggregate-errors "
+                            "have no effect without -verify";
 
   std::error_code EC;
   ToolOutputFile OutputFile(OutputFilename, EC, sys::fs::OF_TextWithCRLF);



More information about the llvm-commits mailing list