[llvm-dwarfdump] Verify compatible TAG for attributes.

[JDevlieghere]

Summary

Adds a verification that DW_AT_specification and DW_AT_abstract origin
reference a DIE with a compatible tag.

[dblaikie]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

423 ↗

(On Diff #118340)

Can you use DWARFDie::getAttributeValueAsReferencedDie here, perhaps?

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

23–24 ↗

(On Diff #118340)

Maybe this should have some more words about which one was expected & which one was actually found? (& maybe dumping whose DIEs or some reference to the DIE so the user knows where to look in the DWARF to find the DIEs in question?)

[JDevlieghere]

• Better error message

[JDevlieghere]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

423 ↗

(On Diff #118340)

Unfortunately not, because this makes use of getUnitForOffset which in turn uses a list of units that isn't build when doing the verification.
Furthermore, this is (probably) slightly more efficient as we already have the attribute value and therefore do not need to do any indirections via the abbrev declaration.

[aprantl]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

422 ↗

(On Diff #118410)

tags

[aprantl]

LGTM with outstanding nitpicks addressed and assuming David is happy with the patch.

lib/DebugInfo/DWARF/DWARFVerifier.cpp

434 ↗

(On Diff #118410)

<< "" ?

[aprantl]

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

23 ↗

(On Diff #118410)

can you use CHECK-NEXT here to make the test even stricter?

[JDevlieghere]

• Feedback Adrian

[JDevlieghere]

• Use SmallSet<DWARFDie, 3> (running dsymutil over clang showed that there were never more than 3 elements in the set for valid DWARF).

[JDevlieghere]

Wrong diff got updated, back to previous version.

[JDevlieghere]

Rebase & address Dave's comment (via the mailing list).

[JDevlieghere]

Use separate vectors for compile and type units.

[JDevlieghere]

Rebase

[dblaikie]

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

115–119 ↗

(On Diff #165769)

This probably needs to ensure the newly added unit is added such that the vector remains in sorted order (so getUnitForOffset, etc, can rely on that sorted order for offset searches).

It could assert this (if the caller(s) you've added ensure this is true) or do a search/insert to keep things sorted.

Or, if you know you won't be using getUnitForOffset, etc, I guess it doesn't need to. Or if you have a phase where you're building the vector then a separate phase where you're using getUnitForOffset - you could add them all, then sort, then query - that's a bit more efficient that maintaining sorted order when you don't need it.

[JDevlieghere]

Make sure addUnit keeps the DWARFUnitVector sorted.

[JDevlieghere]

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

115–119 ↗

(On Diff #165769)

I've updated the patch to keep thing sorted, it seems like the least bug-prone as we keep the invariant. In the verifier we also build and use the vector at the same time, so sorting at the end is tricky (and would require more state if we want to enforce this, making it a less localized approach than simply sorting).

[dblaikie]

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

281–282 ↗

(On Diff #166072)

auto UnitUP = llvm::make_unique<DWARFTypeUnit>(...)

perhaps?

(I'd probably even roll it into the next line:

Unit = TypeUnitVector.addUnit(llvm::make_unique<DWARFTypeUnit>(...));

(similarly below for CompileUnit)

461–464 ↗

(On Diff #166072)

Should there be a positive case for tags are the same? (things like a member subprogram - DieTag and RefTag would both be subprogram?) Or a function defined in a namespace (I think GCC puts the declaration in a namespace, the definition at global scope - I forget what Clang does tehre - maybe it doesn't produce a declaration so there's nothing to check there?)

Have you run this check over a large codebase to see that it doesn't produce false positives?

[JDevlieghere]

• Add check for equal tags to cover case where both are subprograms, both are formal params.
• Ran verifier over clang, no errors.
• Ran verifier over GCC generated binary I had laying around, also no errors.

[JDevlieghere]

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

461–464 ↗

(On Diff #166072)

We didn't account for the case where the tags were the same, as you suggested. I ran the verifier on a ToT clang and on a gcc-generated binary (some xml lib I had around) and didn't see any false positives.

[dblaikie]

Sounds good - thanks!

[kito-cheng]

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

463 ↗

(On Diff #166408)

Hi Jonas:

Compiler will complain about fallthrough here:
lib/DebugInfo/DWARF/DWARFVerifier.cpp:477:5: warning: this statement may fall through [-Wimplicit-fallthrough=]

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

 //===- DWARFDie.cpp -------------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/DWARF/DWARFDie.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugRangeList.h"
 #include "llvm/DebugInfo/DWARF/DWARFExpression.h"
 #include "llvm/DebugInfo/DWARF/DWARFFormValue.h"
 #include "llvm/DebugInfo/DWARF/DWARFUnit.h"
for (auto Attr : Attrs) {
         return Value;
     }
   }
   return None;
 }
 
 Optional<DWARFFormValue>
 DWARFDie::findRecursively(ArrayRef<dwarf::Attribute> Attrs) const {
-  if (!isValid())
-    return None;
-  if (auto Value = find(Attrs))
-    return Value;
-  if (auto Die = getAttributeValueAsReferencedDie(DW_AT_abstract_origin)) {
-    if (auto Value = Die.findRecursively(Attrs))
-      return Value;
-  }
-  if (auto Die = getAttributeValueAsReferencedDie(DW_AT_specification)) {
-    if (auto Value = Die.findRecursively(Attrs))
+  std::vector<DWARFDie> Worklist;
+  SmallSet<DWARFDie, 3> Seen;
+  Worklist.push_back(*this);
+
+  while (!Worklist.empty()) {
+    DWARFDie Die = Worklist.back();
+    Worklist.pop_back();
+
+    if (!Die.isValid())
+      continue;
+
+    if (Seen.count(Die))
+      continue;
+
+    Seen.insert(Die);
+
+    if (auto Value = Die.find(Attrs))
       return Value;
+
+    if (auto D = Die.getAttributeValueAsReferencedDie(DW_AT_abstract_origin))
+      Worklist.push_back(D);
+
+    if (auto D = Die.getAttributeValueAsReferencedDie(DW_AT_specification))
+      Worklist.push_back(D);
   }
+
   return None;
 }
 
 DWARFDie
 DWARFDie::getAttributeValueAsReferencedDie(dwarf::Attribute Attr) const {
   if (auto SpecRef = toReference(find(Attr))) {
     if (auto SpecUnit = U->getUnitSection().getUnitForOffset(*SpecRef))
       return SpecUnit->getDIEForOffset(*SpecRef);

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

+# This test ensures that dwarfdump doesn't crash for malformed DWARF where a
+# DIE references itself.
+#
+# Source:
+#   void f();
+#   __attribute__((always_inline)) void g() {
+#     f();
+#   }
+#   void h() {
+#     g();
+#   };
+#
+# Compile with:
+#   clang inlined.c -S -g -o inlined.s
+#
+# RUN: llvm-mc %s -filetype obj -triple x86_64-apple-darwin -o - \
+# RUN:   | llvm-dwarfdump -debug-info - \
+# RUN:   | FileCheck %s
+
+# CHECK: 0x0000005a:     DW_TAG_inlined_subroutine
+# CHECK-NEXT: DW_AT_abstract_origin (0x0000005a)
+
+  .section  __TEXT,__text,regular,pure_instructions
+  .macosx_version_min 10, 13
+  .globl  _g                      ## -- Begin function g
+  .p2align  4, 0x90
+_g:                                     ## @g
+Lfunc_begin0:
+  .file 1 "inlined.c"
+  .loc  1 2 0                   ## inlined.c:2:0
+  .cfi_startproc
+## %bb.0:                               ## %entry
+  pushq %rbp
+  .cfi_def_cfa_offset 16
+  .cfi_offset %rbp, -16
+  movq  %rsp, %rbp
+  .cfi_def_cfa_register %rbp
+Ltmp0:
+  .loc  1 3 3 prologue_end      ## inlined.c:3:3
+  movb  $0, %al
+  callq _f
+  .loc  1 4 1                   ## inlined.c:4:1
+  popq  %rbp
+  retq
+Ltmp1:
+Lfunc_end0:
+  .cfi_endproc
+                                        ## -- End function
+  .globl  _h                      ## -- Begin function h
+  .p2align  4, 0x90
+_h:                                     ## @h
+Lfunc_begin1:
+  .loc  1 5 0                   ## inlined.c:5:0
+  .cfi_startproc
+## %bb.0:                               ## %entry
+  pushq %rbp
+  .cfi_def_cfa_offset 16
+  .cfi_offset %rbp, -16
+  movq  %rsp, %rbp
+  .cfi_def_cfa_register %rbp
+Ltmp2:
+  .loc  1 3 3 prologue_end      ## inlined.c:3:3
+  movb  $0, %al
+  callq _f
+Ltmp3:
+  .loc  1 7 1                   ## inlined.c:7:1
+  popq  %rbp
+  retq
+Ltmp4:
+Lfunc_end1:
+  .cfi_endproc
+                                        ## -- End function
+  .section  __DWARF,__debug_str,regular,debug
+Linfo_string:
+  .asciz  "clang version 7.0.0 "  ## string offset=0
+  .asciz  "inlined.c"             ## string offset=21
+  .asciz  "/private/tmp"          ## string offset=31
+  .asciz  "g"                     ## string offset=44
+  .asciz  "h"                     ## string offset=46
+  .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 23                      ## DW_FORM_sec_offset
+  .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 6                       ## DW_FORM_data4
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .byte 2                       ## Abbreviation Code
+  .byte 46                      ## DW_TAG_subprogram
+  .byte 0                       ## DW_CHILDREN_no
+  .byte 17                      ## DW_AT_low_pc
+  .byte 1                       ## DW_FORM_addr
+  .byte 18                      ## DW_AT_high_pc
+  .byte 6                       ## DW_FORM_data4
+  .byte 64                      ## DW_AT_frame_base
+  .byte 24                      ## DW_FORM_exprloc
+  .byte 49                      ## DW_AT_abstract_origin
+  .byte 19                      ## DW_FORM_ref4
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .byte 3                       ## Abbreviation Code
+  .byte 46                      ## DW_TAG_subprogram
+  .byte 0                       ## DW_CHILDREN_no
+  .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 63                      ## DW_AT_external
+  .byte 25                      ## DW_FORM_flag_present
+  .byte 32                      ## DW_AT_inline
+  .byte 11                      ## DW_FORM_data1
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .byte 4                       ## Abbreviation Code
+  .byte 46                      ## DW_TAG_subprogram
+  .byte 1                       ## DW_CHILDREN_yes
+  .byte 17                      ## DW_AT_low_pc
+  .byte 1                       ## DW_FORM_addr
+  .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
+  .byte 11                      ## DW_FORM_data1
+  .byte 63                      ## DW_AT_external
+  .byte 25                      ## DW_FORM_flag_present
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .byte 5                       ## Abbreviation Code
+  .byte 29                      ## DW_TAG_inlined_subroutine
+  .byte 0                       ## DW_CHILDREN_no
+  .byte 49                      ## DW_AT_abstract_origin
+  .byte 19                      ## DW_FORM_ref4
+  .byte 17                      ## DW_AT_low_pc
+  .byte 1                       ## DW_FORM_addr
+  .byte 18                      ## DW_AT_high_pc
+  .byte 6                       ## DW_FORM_data4
+  .byte 88                      ## DW_AT_call_file
+  .byte 11                      ## DW_FORM_data1
+  .byte 89                      ## DW_AT_call_line
+  .byte 11                      ## DW_FORM_data1
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .byte 0                       ## EOM(3)
+  .section  __DWARF,__debug_info,regular,debug
+Lsection_info:
+Lcu_begin0:
+  .long 107                     ## Length of Unit
+  .short  4                       ## 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:0x64 DW_TAG_compile_unit
+  .long 0                       ## DW_AT_producer
+  .short  12                      ## DW_AT_language
+  .long 21                      ## DW_AT_name
+Lset1 = Lline_table_start0-Lsection_line ## DW_AT_stmt_list
+  .long Lset1
+  .long 31                      ## DW_AT_comp_dir
+  .quad Lfunc_begin0            ## DW_AT_low_pc
+Lset2 = Lfunc_end1-Lfunc_begin0         ## DW_AT_high_pc
+  .long Lset2
+  .byte 2                       ## Abbrev [2] 0x2a:0x13 DW_TAG_subprogram
+  .quad Lfunc_begin0            ## DW_AT_low_pc
+Lset3 = Lfunc_end0-Lfunc_begin0         ## DW_AT_high_pc
+  .long Lset3
+  .byte 1                       ## DW_AT_frame_base
+  .byte 86
+  .long 61                      ## DW_AT_abstract_origin
+  .byte 3                       ## Abbrev [3] 0x3d:0x8 DW_TAG_subprogram
+  .long 44                      ## DW_AT_name
+  .byte 1                       ## DW_AT_decl_file
+  .byte 2                       ## DW_AT_decl_line
+                                        ## DW_AT_external
+  .byte 1                       ## DW_AT_inline
+  .byte 4                       ## Abbrev [4] 0x45:0x29 DW_TAG_subprogram
+  .quad Lfunc_begin1            ## DW_AT_low_pc
+Lset4 = Lfunc_end1-Lfunc_begin1         ## DW_AT_high_pc
+  .long Lset4
+  .byte 1                       ## DW_AT_frame_base
+  .byte 86
+  .long 46                      ## DW_AT_name
+  .byte 1                       ## DW_AT_decl_file
+  .byte 5                       ## DW_AT_decl_line
+                                        ## DW_AT_external
+  .byte 5                       ## Abbrev [5] 0x5a:0x13 DW_TAG_inlined_subroutine
+  .long 90                      ## DW_AT_abstract_origin <- We modified the value so the DIE references itself.
+  .quad Ltmp2                   ## DW_AT_low_pc
+Lset5 = Ltmp3-Ltmp2                     ## DW_AT_high_pc
+  .long Lset5
+  .byte 1                       ## DW_AT_call_file
+  .byte 6                       ## DW_AT_call_line
+  .byte 0                       ## End Of Children Mark
+  .byte 0                       ## End Of Children Mark
+  .section  __DWARF,__debug_ranges,regular,debug
+Ldebug_range:
+  .section  __DWARF,__debug_macinfo,regular,debug
+Ldebug_macinfo:
+Lcu_macro_begin0:
+  .byte 0                       ## End Of Macro List Mark
+  .section  __DWARF,__apple_names,regular,debug
+Lnames_begin:
+  .long 1212240712              ## Header Magic
+  .short  1                       ## Header Version
+  .short  0                       ## Header Hash Function
+  .long 2                       ## Header Bucket Count
+  .long 2                       ## Header Hash Count
+  .long 12                      ## Header Data Length
+  .long 0                       ## HeaderData Die Offset Base
+  .long 1                       ## HeaderData Atom Count
+  .short  1                       ## DW_ATOM_die_offset
+  .short  6                       ## DW_FORM_data4
+  .long 0                       ## Bucket 0
+  .long 1                       ## Bucket 1
+  .long 177676                  ## Hash in Bucket 0
+  .long 177677                  ## Hash in Bucket 1
+  .long LNames0-Lnames_begin    ## Offset in Bucket 0
+  .long LNames1-Lnames_begin    ## Offset in Bucket 1
+LNames0:
+  .long 44                      ## g
+  .long 2                       ## Num DIEs
+  .long 42
+  .long 90
+  .long 0
+LNames1:
+  .long 46                      ## h
+  .long 1                       ## Num DIEs
+  .long 69
+  .long 0
+  .section  __DWARF,__apple_objc,regular,debug
+Lobjc_begin:
+  .long 1212240712              ## Header Magic
+  .short  1                       ## Header Version
+  .short  0                       ## Header Hash Function
+  .long 1                       ## Header Bucket Count
+  .long 0                       ## Header Hash Count
+  .long 12                      ## Header Data Length
+  .long 0                       ## HeaderData Die Offset Base
+  .long 1                       ## HeaderData Atom Count
+  .short  1                       ## DW_ATOM_die_offset
+  .short  6                       ## DW_FORM_data4
+  .long -1                      ## Bucket 0
+  .section  __DWARF,__apple_namespac,regular,debug
+Lnamespac_begin:
+  .long 1212240712              ## Header Magic
+  .short  1                       ## Header Version
+  .short  0                       ## Header Hash Function
+  .long 1                       ## Header Bucket Count
+  .long 0                       ## Header Hash Count
+  .long 12                      ## Header Data Length
+  .long 0                       ## HeaderData Die Offset Base
+  .long 1                       ## HeaderData Atom Count
+  .short  1                       ## DW_ATOM_die_offset
+  .short  6                       ## DW_FORM_data4
+  .long -1                      ## Bucket 0
+  .section  __DWARF,__apple_types,regular,debug
+Ltypes_begin:
+  .long 1212240712              ## Header Magic
+  .short  1                       ## Header Version
+  .short  0                       ## Header Hash Function
+  .long 1                       ## Header Bucket Count
+  .long 0                       ## Header Hash Count
+  .long 20                      ## Header Data Length
+  .long 0                       ## HeaderData Die Offset Base
+  .long 3                       ## HeaderData Atom Count
+  .short  1                       ## DW_ATOM_die_offset
+  .short  6                       ## DW_FORM_data4
+  .short  3                       ## DW_ATOM_die_tag
+  .short  5                       ## DW_FORM_data2
+  .short  4                       ## DW_ATOM_type_flags
+  .short  11                      ## DW_FORM_data1
+  .long -1                      ## Bucket 0
+
+.subsections_via_symbols
+  .section  __DWARF,__debug_line,regular,debug
+Lsection_line:
+Lline_table_start0: