[DWARF] Generalized verification of .debug_abbrev to be applicable to both .debug_abbrev and .debug_abbrev.dwo sections.

[dblaikie]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

135 ↗

(On Diff #107582)

General feedback: probably want to consistently use lowercase 'warning' and 'error' (or better yet/as well, maybe refactor error/warning handling to use a single function for printing warnings and errors).

Also there are two spaces between ".debug_abbrev" and "and" - should only be one.

Is there an existing warning about there being no debug_info section? And about debug_info not having a valid debug_abbrev? Then maybe this warning about missing debug_abbrev isn't necessary?

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

1 ↗

(On Diff #107582)

This seems like a really big/long test - what's in basic.c? Does it need to be that complicated? (could it be simpler/shorter/etc?)

6–7 ↗

(On Diff #107582)

Maybe worth mentioning the DIE tag? Or dumping the whole abbrev after/with the diagnostic?

[sgravani]

Addressed reviewer's comments.

lib/DebugInfo/DWARF/DWARFVerifier.cpp

135 ↗

(On Diff #107582)

Partially fixed. I removed the warning from .debug_abbrev. Currently I'm printing a warning for .debug_info section as well, but I plan
to remove it with another patch. I also removed the extra space character.

I agree with having a function generating the errors/warnings. I could do this in another patch.
I could also add a verbose mode in another patch and output errors only when verbose is on.

[dblaikie]

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

7 ↗

(On Diff #107603)

Should check that the DIE gets dumped too

[probinson]

Thanks!

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

 namespace llvm {
 class raw_ostream;
 struct DWARFAttribute;
 class DWARFContext;
 class DWARFDie;
 class DWARFUnit;
 class DWARFAcceleratorTable;
 class DWARFDataExtractor;
+class DWARFDebugAbbrev;
 
 /// A class that verifies DWARF debug information given a DWARF Context.
 class DWARFVerifier {
   raw_ostream &OS;
   DWARFContext &DCtx;
   /// A map that tracks all references (converted absolute references) so we
   /// can verify each reference points to a valid DIE and not an offset that
   /// lies between to valid DIEs.
   std::map<uint64_t, std::set<uint32_t>> ReferenceToDIEOffsets;
   uint32_t NumDebugLineErrors = 0;
   uint32_t NumAppleNamesErrors = 0;
 
+  /// Verifies the abbreviations section.
+  ///
+  /// This function currently checks that:
+  /// --No abbreviation declaration has more than one attributes with the same
+  /// name.
+  ///
+  /// \param Abbrev Pointer to the abbreviations section we are verifying
+  /// Abbrev can be a pointer to either .debug_abbrev or debug_abbrev.dwo.
+  ///
+  /// \returns The number of errors that occured during verification.
+  unsigned verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev);
+
   /// Verifies the header of a unit in the .debug_info section.
   ///
   /// This function currently checks for:
   /// - Unit is in 32-bit DWARF format. The function can be modified to
   /// support 64-bit format.
   /// - The DWARF version is valid
   /// - The unit type is valid (if unit is in version >=5)
   /// - The unit doesn't extend beyond .debug_info section
class DWARFVerifier {
   /// This function currently checks for:
   /// - addresses within a sequence that decrease in value
   /// - invalid file indexes
   void verifyDebugLineRows();
 
 public:
   DWARFVerifier(raw_ostream &S, DWARFContext &D)
       : OS(S), DCtx(D) {}
-  /// Verify the information in the .debug_abbrev section.
+  /// Verify the information in any of the following sections, if available:
+  /// .debug_abbrev, debug_abbrev.dwo
   ///
-  /// Currently, we check that no Abbreviation Declaration has more than one
-  /// attributes with the same name.
+  /// Any errors are reported to the stream that was this object was
+  /// constructed with.
   ///
-  /// \returns true if the .debug_abbrev verifies successfully, false otherwise.
+  /// \returns true if .debug_abbrev and .debug_abbrev.dwo verify successfully,
+  /// false otherwise.
   bool handleDebugAbbrev();
+
   /// Verify the information in the .debug_info section.
   ///
   /// Any errors are reported to the stream that was this object was
   /// constructed with.
   ///
   /// \returns true if the .debug_info verifies successfully, false otherwise.
   bool handleDebugInfo();
 

llvm/trunk/lib/DebugInfo/DWARF/DWARFVerifier.cpp

for (unsigned I = 0; I < NumDies; ++I) {
     for (auto AttrValue : Die.attributes()) {
       NumUnitErrors += verifyDebugInfoAttribute(Die, AttrValue);
       NumUnitErrors += verifyDebugInfoForm(Die, AttrValue);
     }
   }
   return NumUnitErrors == 0;
 }
 
-bool DWARFVerifier::handleDebugAbbrev() {
-  OS << "Verifying .debug_abbrev...\n";
-
-  const DWARFObject &DObj = DCtx.getDWARFObj();
-  if (DObj.getAbbrevSection().empty()) {
-    OS << "Warning: .debug_abbrev is empty.\n";
-    return true;
-  }
-
+unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) {
   unsigned NumErrors = 0;
-  const DWARFDebugAbbrev *Abbrev = DCtx.getDebugAbbrev();
   if (Abbrev) {
     const DWARFAbbreviationDeclarationSet *AbbrDecls =
         Abbrev->getAbbreviationDeclarationSet(0);
     for (auto AbbrDecl : *AbbrDecls) {
       SmallDenseSet<uint16_t> AttributeSet;
       for (auto Attribute : AbbrDecl.attributes()) {
         auto Result = AttributeSet.insert(Attribute.Attr);
         if (!Result.second) {
-          OS << format("Error: Abbreviation declaration with code %d ",
-                       AbbrDecl.getCode());
-          OS << "contains multiple " << AttributeString(Attribute.Attr)
-             << " attributes.\n";
+          OS << "Error: Abbreviation declaration contains multiple "
+             << AttributeString(Attribute.Attr) << " attributes.\n";
+          AbbrDecl.dump(OS);
           ++NumErrors;
         }
       }
     }
   }
+  return NumErrors;
+}
+
+bool DWARFVerifier::handleDebugAbbrev() {
+  OS << "Verifying .debug_abbrev...\n";
+
+  const DWARFObject &DObj = DCtx.getDWARFObj();
+  bool noDebugAbbrev = DObj.getAbbrevSection().empty();
+  bool noDebugAbbrevDWO = DObj.getAbbrevDWOSection().empty();
+
+  if (noDebugAbbrev && noDebugAbbrevDWO) {
+    return true;
+  }
+
+  unsigned NumErrors = 0;
+  if (!noDebugAbbrev)
+    NumErrors += verifyAbbrevSection(DCtx.getDebugAbbrev());
+
+  if (!noDebugAbbrevDWO)
+    NumErrors += verifyAbbrevSection(DCtx.getDebugAbbrevDWO());
   return NumErrors == 0;
 }
 
 bool DWARFVerifier::handleDebugInfo() {
   OS << "Verifying .debug_info Unit Header Chain...\n";
 
   const DWARFObject &DObj = DCtx.getDWARFObj();
   DWARFDataExtractor DebugInfoData(DObj, DObj.getInfoSection(),

llvm/trunk/test/tools/llvm-dwarfdump/X86/verify_debug_abbrev.s

+# RUN: llvm-mc %s -filetype obj -triple x86_64-unknown-linux-gnu -o - \
+# RUN: | not llvm-dwarfdump -verify - \
+# RUN: | FileCheck %s
+
+# CHECK: Verifying .debug_abbrev...
+# CHECK-NEXT: Error: Abbreviation declaration contains multiple DW_AT_stmt_list attributes.
+# CHECK-NEXT:[1] DW_TAG_compile_unit  DW_CHILDREN_no
+# CHECK-NEXT: DW_AT_stmt_list DW_FORM_sec_offset
+# CHECK-NEXT: DW_AT_GNU_dwo_name  DW_FORM_strp
+# CHECK-NEXT: DW_AT_stmt_list DW_FORM_strp{{[[:space:]]}}
+# CHECK-NEXT: Error: Abbreviation declaration contains multiple DW_AT_producer attributes.
+# CHECK-NEXT:[1] DW_TAG_compile_unit  DW_CHILDREN_yes
+# CHECK-NEXT: DW_AT_GNU_dwo_name  DW_FORM_GNU_str_index
+# CHECK-NEXT: DW_AT_producer  DW_FORM_GNU_str_index
+# CHECK-NEXT: DW_AT_producer  DW_FORM_data2
+
+
+  .section  .debug_abbrev,"",@progbits
+  .byte  1                       # Abbreviation Code
+  .byte  17                      # DW_TAG_compile_unit
+  .byte  0                       # DW_CHILDREN_no
+  .byte  16                      # DW_AT_stmt_list
+  .byte  23                      # DW_FORM_sec_offset
+  .ascii  "\260B"                # DW_AT_GNU_dwo_name
+  .byte  14                      # DW_FORM_strp
+  .byte  16                      # DW_AT_stmt_list -- Error: Abbreviation declaration contains multiple DW_AT_stmt_list attributes.
+  .byte  14                      # DW_FORM_strp
+  .byte  0                       # EOM(1)
+  .byte  0                       # EOM(2)
+  .byte  0                       # EOM(3)
+  .section  .debug_abbrev.dwo,"",@progbits
+  .byte  1                       # Abbreviation Code
+  .byte  17                      # DW_TAG_compile_unit
+  .byte  1                       # DW_CHILDREN_yes
+  .ascii  "\260B"                # DW_AT_GNU_dwo_name
+  .ascii  "\202>"                # DW_FORM_GNU_str_index
+  .byte  37                      # DW_AT_producer
+  .ascii  "\202>"                # DW_FORM_GNU_str_index
+  .byte  37                      # DW_AT_producer -- Error: Abbreviation declaration contains multiple DW_AT_producer attributes.
+  .byte  5                       # DW_FORM_data1
+  .byte  0                       # EOM(1)
+  .byte  0                       # EOM(2)
+  .byte  0                       # EOM(3)
+  
 No newline at end of file

llvm/trunk/test/tools/llvm-dwarfdump/X86/verify_debug_info.s

 # RUN: llvm-mc %s -filetype obj -triple x86_64-apple-darwin -o - \
 # RUN: | not llvm-dwarfdump -verify - \
 # RUN: | FileCheck %s
 
-# CHECK: Verifying .debug_abbrev...
-# CHECK-NEXT: Error: Abbreviation declaration with code 2 contains multiple DW_AT_low_pc attributes.
-# CHECK-NEXT: Verifying .debug_info Unit Header Chain...
-# CHECK-NEXT: error: DIE has invalid DW_AT_stmt_list encoding:{{[[:space:]]}}
+# CHECK: error: DIE has invalid DW_AT_stmt_list encoding:{{[[:space:]]}}
 # CHECK-NEXT: 0x0000000c: DW_TAG_compile_unit [1] *
 # CHECK-NEXT: DW_AT_producer [DW_FORM_strp] ( .debug_str[0x00000000] = "clang version 5.0.0 (trunk 308185) (llvm/trunk 308186)")
 # CHECK-NEXT: DW_AT_language [DW_FORM_data2]  (DW_LANG_C99)
 # CHECK-NEXT: DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000037] = "basic.c")
 # CHECK-NEXT: DW_AT_stmt_list [DW_FORM_strx4] ( indexed (00000000) string = )
 # CHECK-NEXT: DW_AT_comp_dir [DW_FORM_strp] ( .debug_str[0x0000003f] = "/Users/sgravani/Development/tests")
 # CHECK-NEXT: DW_AT_low_pc [DW_FORM_addr] (0x0000000000000000)
 # CHECK-NEXT: DW_AT_high_pc [DW_FORM_data4] (0x00000016){{[[:space:]]}}
Lsection_abbrev:
   .byte 6                       ## DW_FORM_data4
   .byte 0                       ## EOM(1)
   .byte 0                       ## EOM(2)
   .byte 2                       ## Abbreviation Code
   .byte 46                      ## DW_TAG_subprogram
   .byte 1                       ## DW_CHILDREN_yes
   .byte 17                      ## DW_AT_low_pc
   .byte 1                       ## DW_FORM_addr
-  .byte  17                      ## DW_AT_low_pc -- Error: Die at offset 0x0000002b contains multiple DW_AT_low_pc attributes.
+  .byte 18                      ## DW_AT_high_pc
   .byte 6                       ## DW_FORM_data4
   .byte 64                      ## DW_AT_frame_base
   .byte 24                      ## DW_FORM_exprloc
   .byte 3                       ## DW_AT_name
   .byte 14                      ## DW_FORM_strp
   .byte 58                      ## DW_AT_decl_file
   .byte 11                      ## DW_FORM_data1
   .byte 59                      ## DW_AT_decl_line