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

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


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

>From 35f5ee6274f25f6fe3cbfaf7d60a518773642769 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 84a6d8e06acb29cc5fa4e1e1719957dfe2bc4da6 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 580d8b6db05ced72bf6b6238780b46ff750a7bd1 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 724e1faf32ce4eaaa8fe4612114487f17bdf2b42 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 7967c79d9d549c38dff960cae8d1e3d4ea36c4ab 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 43f3dc055bd5e8748cf0c1fa6a9398fdbd06fd15 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 efb2d596a84661c49363aaa7fd3989b5399200b8 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 ff9a88a27161d2fcf6ee9ca63f0f5af6fc916c94 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 9fe63e3d6843cc89dd51d4eab1c32d344a84d781 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 9d7f00c76b98f94ecf56091afc3fb3362491ef98 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 1b726a35a3b4c5a813af0847035c331399968c70 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