[dwarfdump] Verify that unit type matches root DIE

[JDevlieghere]

Summary

This patch adds two new verifications:

• Check that the root DIE of a CU is actually a valid unit DIE. (based on its tag)
• For DWARF5 which contains a unit type int he CU header, check that this matches the type of the unit DIE.

[aprantl]

include/llvm/DebugInfo/DWARF/DWARFUnit.h

315

Is that correct? I thought imported units are an orthogonal concept to split units. (But I may be wrong here)

lib/DebugInfo/DWARF/DWARFVerifier.cpp

187

Perhaps: Compilation unit header without any DIEs?

[JDevlieghere]

• Feedback Adrian

[JDevlieghere]

include/llvm/DebugInfo/DWARF/DWARFUnit.h

315

Nope, this is wrong. I made this assumption before checking the standard which in its example uses a regular compile_unit. I must haveuploaded the patch before fixing this.

lib/DebugInfo/DWARF/DWARFVerifier.cpp

187

I'd be glad to change this if you think that's better. I felt like this is implicit/obvious and prevents the confusion that we might still be verifying the header. But again, I have no strong opinion about this.

[aprantl]

include/llvm/DebugInfo/DWARF/DWARFUnit.h

315

this seems right now, thanks.

[aprantl]

include/llvm/DebugInfo/DWARF/DWARFUnit.h

296

Should we move these two into Dwarf.h as isUnitTag ?

[JDevlieghere]

include/llvm/DebugInfo/DWARF/DWARFUnit.h

296

Sure, sounds reasonable!

[dblaikie]

llvm/trunk/include/llvm/BinaryFormat/Dwarf.h

348 ↗

(On Diff #118096)

This is probably incorrect - imported_units seem to be used to import a unit into another unit, they aren't themselves a valid top-level unit. (See Figure E.6 in DWARF5 for an example of their use)

[JDevlieghere]

llvm/trunk/include/llvm/BinaryFormat/Dwarf.h

348 ↗

(On Diff #118096)

Thanks David! I sent an e-mail to the dwarf standard mailing list and the first reply confirms you are indeed correct. I've committed a fix that removes this.

include/llvm/DebugInfo/DWARF/DWARFUnit.h

public:
   static bool isValidUnitType(uint8_t UnitType) {
     return UnitType == dwarf::DW_UT_compile || UnitType == dwarf::DW_UT_type ||
            UnitType == dwarf::DW_UT_partial ||
            UnitType == dwarf::DW_UT_skeleton ||
            UnitType == dwarf::DW_UT_split_compile ||
            UnitType == dwarf::DW_UT_split_type;
   }
 
+  static bool isValidUnitTag(dwarf::Tag Tag) {
+    return Tag == dwarf::DW_TAG_compile_unit ||
+           Tag == dwarf::DW_TAG_type_unit ||
+           Tag == dwarf::DW_TAG_partial_unit ||
+           Tag == dwarf::DW_TAG_skeleton_unit ||
+           Tag == dwarf::DW_TAG_imported_unit;
+  }
+
+  static bool isMatchingUnitTypeAndTag(uint8_t UnitType, dwarf::Tag Tag) {
+    switch (UnitType) {
+    case dwarf::DW_UT_compile:
+      return Tag == dwarf::DW_TAG_compile_unit;
+    case dwarf::DW_UT_type:
+      return Tag == dwarf::DW_TAG_type_unit;
+    case dwarf::DW_UT_partial:
+      return Tag == dwarf::DW_TAG_partial_unit;
+    case dwarf::DW_UT_skeleton:
+      return Tag == dwarf::DW_TAG_skeleton_unit;
+    case dwarf::DW_UT_split_compile:
+    case dwarf::DW_UT_split_type:
+      return Tag == dwarf::DW_TAG_imported_unit;
+    }
+    return false;
+  }
+
   /// \brief Return the number of bytes for the header of a unit of
   /// UnitType type.
   ///
   /// This function must be called with a valid unit type which in
   /// DWARF5 is defined as one of the following six types.
   static uint32_t getDWARF5HeaderSize(uint8_t UnitType) {
     switch (UnitType) {
     case dwarf::DW_UT_compile:

include/llvm/DebugInfo/DWARF/DWARFVerifier.h

private:
   /// \param isUnitDWARF64 A reference to a flag that shows whether the unit is
   /// in 64-bit format.
   ///
   /// \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);
+  /// Verifies the header of a unit in the .debug_info section.
+  ///
+  /// This function currently verifies:
+  ///  - The debug info attributes.
+  ///  - The debug info form=s.
+  ///  - The presence of a root DIE.
+  ///  - That the root DIE is a unit DIE.
+  ///  - If a unit type is provided, that the unit DIE matches the unit type.
+  ///  - The DIE ranges.
+  ///
+  /// \param Unit      The DWARF Unit to verifiy.
+  /// \param UnitType  An optional unit type which will be used to verify the
+  ///                  type of the unit DIE.
+  ///
+  /// \returns true if the content is verified successfully, false otherwise.
+  bool verifyUnitContents(DWARFUnit Unit, uint8_t UnitType = 0);
 
   /// Verify that all Die ranges are valid.
   ///
   /// This function currently checks for:
   /// - cases in which lowPC >= highPC
   ///
   /// \returns Number of errors that occured during verification.
   unsigned verifyDieRanges(const DWARFDie &Die, DieRangeInfo &ParentRI);

lib/DebugInfo/DWARF/DWARFVerifier.cpp

if (!ValidAbbrevOffset)
             "not valid.\n";
     if (!ValidAddrSize)
       note() << "The address size is unsupported.\n";
   }
   *Offset = OffsetStart + Length + 4;
   return Success;
 }
 
-bool DWARFVerifier::verifyUnitContents(DWARFUnit Unit) {
+bool DWARFVerifier::verifyUnitContents(DWARFUnit Unit, uint8_t UnitType) {
   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;
     for (auto AttrValue : Die.attributes()) {
       NumUnitErrors += verifyDebugInfoAttribute(Die, AttrValue);
       NumUnitErrors += verifyDebugInfoForm(Die, AttrValue);
     }
   }
 
-  if (DWARFDie Die = Unit.getUnitDIE(/* ExtractUnitDIEOnly = */ false)) {
-    DieRangeInfo RI;
-    NumUnitErrors += verifyDieRanges(Die, RI);
-  } else {
-    error() << "Compilation unit without unit DIE.\n";
+  DWARFDie Die = Unit.getUnitDIE(/* ExtractUnitDIEOnly = */ false);
+  if (!Die) {
+    error() << "Compilation unit without DIE.\n";
+    NumUnitErrors++;
+    return NumUnitErrors == 0;
+  }
+
+  if (!DWARFUnit::isValidUnitTag(Die.getTag())) {
+    error() << "Compilation unit root DIE is not a unit DIE: "
+            << dwarf::TagString(Die.getTag()) << ".\n";
+    NumUnitErrors++;
+  }
+
+  if (UnitType != 0 &&
+      !DWARFUnit::isMatchingUnitTypeAndTag(UnitType, Die.getTag())) {
+    error() << "Compilation unit type (" << dwarf::UnitTypeString(UnitType)
+            << ") and root DIE (" << dwarf::TagString(Die.getTag())
+            << ") do not match.\n";
     NumUnitErrors++;
   }
 
+  DieRangeInfo RI;
+  NumUnitErrors += verifyDieRanges(Die, RI);
+
   return NumUnitErrors == 0;
 }
 
 unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) {
   unsigned NumErrors = 0;
   if (Abbrev) {
     const DWARFAbbreviationDeclarationSet *AbbrDecls =
         Abbrev->getAbbreviationDeclarationSet(0);
if (!verifyUnitHeader(DebugInfoData, &Offset, UnitIdx, UnitType,
             DObj.getStringOffsetSection(), &DObj.getAppleObjCSection(),
             DObj.getLineSection(), DCtx.isLittleEndian(), false, CUSection,
             nullptr));
         break;
       }
       default: { llvm_unreachable("Invalid UnitType."); }
       }
       Unit->extract(DebugInfoData, &OffsetStart);
-      if (!verifyUnitContents(*Unit))
+      if (!verifyUnitContents(*Unit, UnitType))
         ++NumDebugInfoErrors;
     }
     hasDIE = DebugInfoData.isValidOffset(Offset);
     ++UnitIdx;
   }
   if (UnitIdx == 0 && !hasDIE) {
     warn() << ".debug_info is empty.\n";
     isHeaderChainValid = true;

test/DebugInfo/dwarfdump-header.test

 RUN: llvm-dwarfdump -v %p/Inputs/dwarfdump-header.elf-x86-64 | FileCheck %s
+RUN: llvm-dwarfdump -v --verify %p/Inputs/dwarfdump-header.elf-x86-64
 
 The input file is hand-coded assembler to generate all the units,
 so we're willing to make exact checks for offsets and such.
 
 CHECK-LABEL: .debug_info contents:
 
 The v4 CU header.
 

test/tools/llvm-dwarfdump/X86/empty-CU.s

 # RUN: llvm-mc %s -filetype obj -triple x86_64-apple-darwin -o - \
 # RUN: | not llvm-dwarfdump --verify --debug-info - \
 # RUN: | FileCheck %s
-# CHECK: error: Compilation unit without unit DIE.
+# CHECK: error: Compilation unit without DIE.
 
         .section        __DWARF,__debug_info,regular,debug
 .long 8  # CU length
 .short 3 # Version
 .long 0  # Abbrev offset
 .byte 4  # AddrSize
 .byte 1  # Abbrev 1
 .long 7  # Unit lengthh...

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: 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
   .globl  _main                   ## -- Begin function main
   .p2align  4, 0x90

test/tools/llvm-dwarfdump/X86/verify_unit_header_chain.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: Units[1] - start offset: 0x0000000d
 # CHECK-NEXT: note: The unit type encoding is not valid.
 # CHECK-NEXT: note: The address size is unsupported.
 # CHECK-NEXT: error: Units[2] - start offset: 0x00000026
 # CHECK-NEXT: note: The 16 bit unit header version is not valid.
 # CHECK-NEXT: note: The offset into the .debug_abbrev section is not valid.
+# 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[4] - start offset: 0x00000041
 # CHECK-NEXT: note: The length for this unit is too large for the .debug_info provided.
 
   .section  __TEXT,__text,regular,pure_instructions
   .file 1 "basic.c"
   .comm _i,4,2                  ## @i
   .comm _j,4,2                  ## @j
   .section  __DWARF,__debug_str,regular,debug