First chunk of PR feedback addressed

Author: Kevin Frei

llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h

@@ -40,6 +40,7 @@ class OutputCategoryAggregator {
       : IncludeDetail(includeDetail) {}
   void EnableDetail() { IncludeDetail = true; }
   void DisableDetail() { IncludeDetail = false; }
+  size_t GetNumCategories() const { return Aggregation.size(); }
   void Report(StringRef s, std::function<void()> detailCallback);
   void EnumerateResults(std::function<void(StringRef, unsigned)> handleCounts);
 };
@@ -365,7 +366,7 @@ class DWARFVerifier {
       void (DWARFObject::*)(function_ref<void(const DWARFSection &)>) const);
 
   /// Emits any aggregate information collected, depending on the dump options
-  void summarize(bool Success);
+  void summarize();
 };
 
 static inline bool operator<(const DWARFVerifier::DieRangeInfo &LHS,

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.summarize(Success);
+  verifier.summarize();
   return Success;
 }
 

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

@@ -167,24 +167,61 @@ bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData,
   if (!ValidLength || !ValidVersion || !ValidAddrSize || !ValidAbbrevOffset ||
       !ValidType) {
     Success = false;
-    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) {
-        note() << "The 16 bit unit header version is not valid.\n";
-      }
-      if (!ValidType) {
-        note() << "The unit type encoding is not valid.\n";
-      }
-      if (!ValidAbbrevOffset)
-        note() << "The offset into the .debug_abbrev section is "
-                  "not valid.\n";
-      if (!ValidAddrSize)
-        note() << "The address size is unsupported.\n";
-    });
+    bool headerShown = false;
+    if (!ValidLength)
+      ErrorCategory.Report(
+          "Unit Header Length: Unit too large for .debug_info provided", [&]() {
+            if (!headerShown) {
+              error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n",
+                                UnitIndex, OffsetStart);
+              headerShown = true;
+            }
+            note() << "The length for this unit is too "
+                      "large for the .debug_info provided.\n";
+          });
+    if (!ValidVersion)
+      ErrorCategory.Report(
+          "Unit Header Length: 16 bit unit header version is not valid", [&]() {
+            if (!headerShown) {
+              error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n",
+                                UnitIndex, OffsetStart);
+              headerShown = true;
+            }
+            note() << "The 16 bit unit header version is not valid.\n";
+          });
+    if (!ValidType)
+      ErrorCategory.Report(
+          "Unit Header Length: Unit type encoding is not valid", [&]() {
+            if (!headerShown) {
+              error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n",
+                                UnitIndex, OffsetStart);
+              headerShown = true;
+            }
+            note() << "The unit type encoding is not valid.\n";
+          });
+    if (!ValidAbbrevOffset)
+      ErrorCategory.Report(
+          "Unit Header Length: Offset into the .debug_abbrev section is not "
+          "valid",
+          [&]() {
+            if (!headerShown) {
+              error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n",
+                                UnitIndex, OffsetStart);
+              headerShown = true;
+            }
+            note() << "The offset into the .debug_abbrev section is "
+                      "not valid.\n";
+          });
+    if (!ValidAddrSize)
+      ErrorCategory.Report(
+          "Unit Header Length: Address size is unsupported", [&]() {
+            if (!headerShown) {
+              error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n",
+                                UnitIndex, OffsetStart);
+              headerShown = true;
+            }
+            note() << "The address size is unsupported.\n";
+          });
   }
   *Offset = OffsetStart + Length + (isUnitDWARF64 ? 12 : 4);
   return Success;
@@ -1999,12 +2036,11 @@ void OutputCategoryAggregator::EnumerateResults(
   }
 }
 
-void DWARFVerifier::summarize(bool Success) {
-  if (!Success && DumpOpts.ShowAggregateErrors) {
-    error() << "Aggregated error category counts:\n";
+void DWARFVerifier::summarize() {
+  if (ErrorCategory.GetNumCategories() && DumpOpts.ShowAggregateErrors) {
+    error() << "Aggregated error counts:\n";
     ErrorCategory.EnumerateResults([&](StringRef s, unsigned count) {
-      error() << "Error category '" << s << "' occurred " << count
-              << " time(s).\n";
+      error() << s << " occurred " << count << " time(s).\n";
     });
   }
 }

llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml

@@ -51,10 +51,10 @@
 # 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).
+# CHECK-NEXT: error: Aggregated error counts:
+# CHECK-NEXT: error: Invalid encoding in DW_AT_decl_file occurred 4 time(s).
+# CHECK-NEXT: error: Invalid file index in DW_AT_call_line occurred 1 time(s).
+# CHECK-NEXT: error: Invalid file index in DW_AT_decl_line occurred 1 time(s).
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64

llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s

@@ -8,12 +8,12 @@
 # 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
+# CHECK: error: Aggregated error counts:
+# CHECK: error: Compilation unit root DIE is not a unit DIE occurred 1 time(s).
+# CHECK: error: File index in DW_AT_decl_file reference CU with no line table occurred 1 time(s).
+# CHECK: error: Invalid DW_AT_location occurred 1 time(s).
+# CHECK: error: Mismatched unit type occurred 1 time(s).
+# CHECK: Errors detected.
 	.section	.debug_info.dwo,"e",@progbits
 	.long	.Ldebug_info_dwo_end1-.Ldebug_info_dwo_start1 # Length of Unit
 .Ldebug_info_dwo_start1: