[DWARF] Added verification check for die ranges. If highPC is an address, then it should be greater or equal to lowPC for each range.

[aprantl]

Seems reasonable, thanks!

lib/DebugInfo/DWARF/DWARFVerifier.cpp

148

extra whitespace

[dblaikie]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

218

Should this be >=? Or is it intentional that empty ranges are allowed to account for things like comdat functions being omitted by the linker & resolving the addresses to zero? (I forget exactly how those work)

[aprantl]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

218

I assume that this is just copying darwin-dwarfdump behavior, but it also looks like DWARF allows a single DW_AT_low_pc to be attached to a DW_TAG_label, which would then show up as an entity of size 0. (cf. 2.17.1 Single Address)

[aprantl]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

218

... and DW_TAG_label is commonly generated from assembler sources.

[sgravani]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

218

Yes, it should be >= since the case that Adrian describes won't have a highPC attribute to start with.
Thanks!

include/llvm/DebugInfo/DWARF/DWARFVerifier.h

class DWARFVerifier {
   ///
   /// \returns true if the header is verified successfully, false otherwise.
   bool verifyUnitHeader(const DWARFDataExtractor DebugInfoData,
                         uint32_t *Offset, unsigned UnitIndex, uint8_t &UnitType,
                         bool &isUnitDWARF64);
 
 
   bool verifyUnitContents(DWARFUnit Unit);
+
+  /// Verify that all Die ranges are valid.
+  ///
+  /// This function currently checks for:
+  /// - cases in which highPC < lowPC
+  ///
+  /// \returns Number of errors that occured during verification.
+  unsigned verifyDieRanges(const DWARFDie &Die);
+
   /// Verifies the attribute's DWARF attribute and its value.
   ///
   /// This function currently checks for:
   /// - DW_AT_ranges values is a valid .debug_ranges offset
   /// - DW_AT_stmt_list is a valid .debug_line offset
   ///
   /// \param Die          The DWARF DIE that owns the attribute value
   /// \param AttrValue    The DWARF attribute value to check

lib/DebugInfo/DWARF/DWARFVerifier.cpp

 
 bool DWARFVerifier::verifyUnitContents(DWARFUnit Unit) {
   uint32_t NumUnitErrors = 0;
   unsigned NumDies = Unit.getNumDIEs();
   for (unsigned I = 0; I < NumDies; ++I) {
     auto Die = Unit.getDIEAtIndex(I);
     if (Die.getTag() == DW_TAG_null)
       continue;
+    NumUnitErrors += verifyDieRanges(Die);
     for (auto AttrValue : Die.attributes()) {
       NumUnitErrors += verifyDebugInfoAttribute(Die, AttrValue);
       NumUnitErrors += verifyDebugInfoForm(Die, AttrValue);
     }
   }
   return NumUnitErrors == 0;
 }
 
bool DWARFVerifier::handleDebugAbbrev() {
   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(),
                                    DCtx.isLittleEndian(), 0);
   uint32_t NumDebugInfoErrors = 0;
   uint32_t OffsetStart = 0, Offset = 0, UnitIdx = 0;
bool DWARFVerifier::handleDebugInfo() {
   if (UnitIdx == 0 && !hasDIE) {
     OS << "Warning: .debug_info is empty.\n";
     isHeaderChainValid = true;
   }
   NumDebugInfoErrors += verifyDebugInfoReferences();
   return (isHeaderChainValid && NumDebugInfoErrors == 0);
 }
 
+unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die) {
+  unsigned NumErrors = 0;
+  for (auto Range : Die.getAddressRanges()) {
+    if (Range.LowPC > Range.HighPC) {
+      ++NumErrors;
+      OS << format("error: Invalid address range [0x%08x - 0x%08x].\n",
+                   Range.LowPC, Range.HighPC);
+    }
+  }
+  return NumErrors;
+}
+
 unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die,
                                                  DWARFAttribute &AttrValue) {
   const DWARFObject &DObj = DCtx.getDWARFObj();
   unsigned NumErrors = 0;
   const auto Attr = AttrValue.Attr;
   switch (Attr) {
   case DW_AT_ranges:
     // Make sure the offset in the DW_AT_ranges attribute is valid.

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

+# RUN: llvm-mc %s -filetype obj -triple x86_64-apple-darwin -o - \
+# RUN: | not llvm-dwarfdump -verify - \
+# RUN: | FileCheck %s
+
+# CHECK: Verifying .debug_info Unit Header Chain...
+# CHECK-NEXT: error: Invalid address range [0x00000007 - 0x00000006].
+
+  .section  __TEXT,__text,regular,pure_instructions
+  .macosx_version_min 10, 12
+  .globl  _foo                    ## -- Begin function foo
+  .p2align  4, 0x90
+_foo:                                   ## @foo
+Lfunc_begin0:
+  .file 1 "basic.c"
+  .loc  1 1 0                   ## basic.c:1:0
+  .cfi_startproc
+## BB#0:                                ## %entry
+  pushq %rbp
+Lcfi0:
+  .cfi_def_cfa_offset 16
+Lcfi1:
+  .cfi_offset %rbp, -16
+  movq  %rsp, %rbp
+Lcfi2:
+  .cfi_def_cfa_register %rbp
+Ltmp0:
+  .loc  1 1 17 prologue_end     ## basic.c:1:17
+  popq  %rbp
+  retq
+Ltmp1:
+Lfunc_end0:
+  .cfi_endproc
+                                        ## -- End function
+  .section  __DWARF,__debug_str,regular,debug
+Linfo_string:
+  .asciz  "clang version 6.0.0 (trunk 308773) (llvm/trunk 308774)" ## string offset=0
+  .asciz  "basic.c"               ## string offset=55
+  .asciz  "/Users/sgravani/Development/tests" ## string offset=63
+  .asciz  "foo"                   ## string offset=97
+  .section  __DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+  .byte 1                       ## Abbreviation Code
+  .byte 17                      ## DW_TAG_compile_unit
+  .byte 1                       ## DW_CHILDREN_yes
+  .byte 37                      ## DW_AT_producer
+  .byte 14                      ## DW_FORM_strp
+  .byte 19                      ## DW_AT_language
+  .byte 5                       ## DW_FORM_data2
+  .byte 3                       ## DW_AT_name
+  .byte 14                      ## DW_FORM_strp
+  .byte 16                      ## DW_AT_stmt_list
+  .byte 6                       ## DW_FORM_data4
+  .byte 27                      ## DW_AT_comp_dir
+  .byte 14                      ## DW_FORM_strp
+  .byte 17                      ## DW_AT_low_pc
+  .byte 1                       ## DW_FORM_addr
+  .byte 18                      ## DW_AT_high_pc
+  .byte 1                       ## DW_FORM_addr
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .section  __DWARF,__debug_info,regular,debug
+Lsection_info:
+Lcu_begin0:
+  .long 43                      ## Length of Unit
+  .short  3                       ## DWARF version number
+Lset0 = Lsection_abbrev-Lsection_abbrev ## Offset Into Abbrev. Section
+  .long Lset0
+  .byte 8                       ## Address Size (in bytes)
+  .byte 1                       ## Abbrev [1] 0xb:0x3f DW_TAG_compile_unit
+  .long 0                       ## DW_AT_producer
+  .short  12                      ## DW_AT_language
+  .long 55                      ## DW_AT_name
+Lset1 = Lline_table_start0-Lsection_line ## DW_AT_stmt_list
+  .long Lset1
+  .long 63                      ## DW_AT_comp_dir
+  .quad Lfunc_end0 + 1          ## DW_AT_low_pc -- error: Invalid address range [0x00000007 - 0x00000006].
+  .quad Lfunc_end0              ## DW_AT_high_pc
+  .byte 0                       ## End Of Children Mark
+.subsections_via_symbols
+  .section  __DWARF,__debug_line,regular,debug
+Lsection_line:
+Lline_table_start0: