[dwarfdump] Verify DW_AT_type.

[JDevlieghere]

Summary

This extends the verifier to catch three new errors:

1. Missing DW_AT_type attributes for DW_TAG_formal_parameter, DW_TAG_variable and DW_TAG_array_type.
2. Invalid references for DW_AT_type attribute values. Technically we already detect invalid references, but now we can be more specific.
3. Valid references for DW_AT_type pointing to a non-type tag.

[aprantl]

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

192 ↗

(On Diff #165907)

Have you tested this? I have a vague recollection that older versions of clang emitted (void) as no type. You may need to add an exception for DWARF2 or something.

Found it: rdar://problem/13291085 (For everyone else: it's a bugreport about exactly this)

195 ↗

(On Diff #165907)

It might be nice to define an operator<<(const Die&) for this presumably(?) common case.

[probinson]

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

192 ↗

(On Diff #165907)

@aprantl I am unclear what it means to have a variable/parameter with type void, or an array whose elements are type void? (pointer to void, sure, but then there will be a DW_AT_type.)
Omitting the type is how you describe a function with void return type, but offhand I can't think of any other situation where that should occur.
or do you mean there are older buggy compilers out there and we should tolerate their bugs?

[aprantl]

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

192 ↗

(On Diff #165907)

The bugreport is about a clang producing the type for (void *) as

DW_TAG_pointer_type
NULL

I had thought that newer clangs are emitting this as

DW_TAG_pointer_type
  DW_AT_TYPE [1]

[1] DW_TAG_unspecified_type
        DW_AT_name "void"

but, no, this is still how it is represented, and frankly, changing it doesn't seem to have any benefits.

Since, as you point out, you can't have an array[void] or a variable of type void, my concern doesn't really apply to this patch.

[JDevlieghere]

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

195 ↗

(On Diff #165907)

I've added a helper function in r342526. The operator isn't an option here because we have to pass the DIDumpOptions, something I apparently forgot here. Another benefit of the new helper :-)

[JDevlieghere]

• Use the new dump method that passes the correct DI dump options.
• Update test to expect verbose output.

[aprantl]

LGTM with the outstanding DIDumpOptions fix. Sorry for holding things up :-)

[xbolva00]

DWARFVerifier.cpp:477:5: warning: this statement may fall through [-Wimplicit-fallthrough=]

}
^

note: here

case DW_AT_type: {
^~~~

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

bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData,
   return Success;
 }
 
 unsigned DWARFVerifier::verifyUnitContents(DWARFUnit &Unit) {
   unsigned 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;
+
+    bool HasTypeAttr = false;
     for (auto AttrValue : Die.attributes()) {
       NumUnitErrors += verifyDebugInfoAttribute(Die, AttrValue);
       NumUnitErrors += verifyDebugInfoForm(Die, AttrValue);
+      HasTypeAttr |= (AttrValue.Attr == DW_AT_type);
+    }
+
+    if (!HasTypeAttr && (Die.getTag() == DW_TAG_formal_parameter ||
+                         Die.getTag() == DW_TAG_variable ||
+                         Die.getTag() == DW_TAG_array_type)) {
+      error() << "DIE with tag " << TagString(Die.getTag())
+              << " is missing type attribute:\n";
+      dump(Die) << '\n';
+      NumUnitErrors++;
     }
   }
 
   DWARFDie Die = Unit.getUnitDIE(/* ExtractUnitDIEOnly = */ false);
   if (!Die) {
     error() << "Compilation unit without DIE.\n";
     NumUnitErrors++;
     return NumUnitErrors;
if (auto ReferencedDie = Die.getAttributeValueAsReferencedDie(Attr)) {
         break;
       ReportError("DIE with tag " + TagString(DieTag) + " has " +
                   AttributeString(Attr) +
                   " that points to DIE with "
                   "incompatible tag " +
                   TagString(RefTag));
     }
   }
+  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()));
+      break;
+    }
+  }
   default:
     break;
   }
   return NumErrors;
 }
 
 unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
                                             DWARFAttribute &AttrValue) {

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 -v -verify - \
 # RUN: | FileCheck %s
 
 # 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:]]}}
+# CHECK-NEXT: error: DIE has DW_AT_type with incompatible tag DW_TAG_null{{[[:space:]]}}
+# CHECK-NEXT: 0x0000002b: DW_TAG_subprogram [2] *
+# CHECK-NEXT: DW_AT_low_pc [DW_FORM_addr]       (0x0000000000000000)
+# CHECK-NEXT: DW_AT_high_pc [DW_FORM_data4]     (0x00000016)
+# CHECK-NEXT: DW_AT_frame_base [DW_FORM_exprloc]        (DW_OP_reg6)
+# CHECK-NEXT: DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000061] = "main")
+# CHECK-NEXT: DW_AT_decl_file [DW_FORM_data1]   (0x01)
+# CHECK-NEXT: DW_AT_decl_line [DW_FORM_data1]   (1)
+# CHECK-NEXT: DW_AT_prototyped [DW_FORM_flag_present]   (true)
+# CHECK-NEXT: DW_AT_type [DW_FORM_ref4] (cu + 0x0052 => {0x00000052} "")
+# CHECK-NEXT: DW_AT_external [DW_FORM_flag_present]     (true){{[[:space:]]}}
+# CHECK-NEXT: error: DIE with tag DW_TAG_variable is missing type attribute:{{[[:space:]]}}
+# CHECK-NEXT: 0x00000044: DW_TAG_variable [3]
+# CHECK-NEXT: DW_AT_location [DW_FORM_exprloc]  (DW_OP_fbreg -8)
+# CHECK-NEXT: DW_AT_name [DW_FORM_strp] ( .debug_str[0x0000006a] = "a")
+# CHECK-NEXT: DW_AT_decl_file [DW_FORM_data1]   (0x01)
+# CHECK-NEXT: DW_AT_decl_line [DW_FORM_data1]   (2)
+# CHECK-NEXT: DW_AT_use_location [DW_FORM_ref4] (cu + 0x0053 => {0x00000053}){{[[:space:]]}}
 # CHECK-NEXT: error: Compilation unit root DIE is not a unit DIE: DW_TAG_null.
 # CHECK-NEXT: error: Compilation unit type (DW_UT_compile) and root DIE (DW_TAG_null) do not match.
 # CHECK-NEXT: error: Units[2] - start offset: 0x00000068
 # CHECK-NEXT: note: The length for this unit is too large for the .debug_info provided.
 # CHECK-NEXT: note: The unit type encoding is not valid.
 
 
   .section  __TEXT,__text,regular,pure_instructions
Lsection_abbrev:
   .byte 2                       ## DW_AT_location
   .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
   .byte 11                      ## DW_FORM_data1
-  .byte 73                      ## DW_AT_type
+  .byte 74                      ## DW_AT_type
   .byte 19                      ## DW_FORM_ref4
   .byte 0                       ## EOM(1)
   .byte 0                       ## EOM(2)
   .byte 4                       ## Abbreviation Code
   .byte 36                      ## DW_TAG_base_type
   .byte 0                       ## DW_CHILDREN_no
   .byte 3                       ## DW_AT_name
   .byte 14                      ## DW_FORM_strp

 Lset3 = Lfunc_end0-Lfunc_begin0         ## DW_AT_high_pc
   .long Lset3
   .byte 1                       ## DW_AT_frame_base
   .byte 86
   .long 97                      ## DW_AT_name
   .byte 1                       ## DW_AT_decl_file
   .byte 1                       ## DW_AT_decl_line
                                         ## DW_AT_prototyped
-  .long 83                      ## DW_AT_type
+  .long 82                      ## DW_AT_type
                                         ## DW_AT_external
   .byte 3                       ## Abbrev [3] 0x44:0xe DW_TAG_variable
   .byte 2                       ## DW_AT_location
   .byte 145
   .byte 120
   .long 106                     ## DW_AT_name
   .byte 1                       ## DW_AT_decl_file
   .byte 2                       ## DW_AT_decl_line