[dwarfdump] Fix spurious verification errors for DW_AT_location attributes

[JDevlieghere]

Summary

Verifying any DWARF file that is optimized and contains at least one tag
with a DW_AT_location with a location list offset as a
DW_AT_form_dataXXX results in dwarfdump spuriously claiming that the
location list is invalid.

[probinson]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

431 ↗

(On Diff #123425)

Looks like this path is missing a couple of opportunities to detect invalid references.

[probinson]

The FORM that's appropriate for DW_AT_location to use as a reference varies with the DWARF version. In v2 and v3, it's FORM_data4 or FORM_data8, in v4 it's FORM_sec_offset. I haven't looked at the verifier much but it seems like using the correct FORM is something that ought to be verifiable.

In v5 things get way more complicated, but that's not anything you have to deal with now.

[aprantl]

Do you happen to have a minimal reproducer that we could check in as an assembler testcase?

[dblaikie]

lib/DebugInfo/DWARF/DWARFVerifier.cpp

413 ↗

(On Diff #123425)

Probably move this down into the "if (Expr)" condition below

if (Optional<ArrayRef<uint8_t>> Expr = AttrValue.Value.getAsBlock()) {
  VerifyLocation(...);

426–435 ↗

(On Diff #123425)

Alternatively/slightly different to all this, could use early breaks:

auto X = ...;
if (!X)
  break;

etc...

429–431 ↗

(On Diff #123425)

Could roll this into else-if:

} else if (auto LocOffset = ... ) {

432–435 ↗

(On Diff #123425)

Could roll these variables into the conditions:

if (auto DebugLoc = DCtx.getDebugLoc())
  if (auto LocList = ...

[JDevlieghere]

This is blocking re-enabling llvm-dwarfdump --verify for Swift.

[JDevlieghere]

• Address review comments
• Add test case

[aprantl]

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

413 ↗

(On Diff #133616)

Why doesn't this just take a StringRef? It looks like all call sites convert StringRefs, too.

429 ↗

(On Diff #133616)

Full sentences in the comments?

431 ↗

(On Diff #133616)

redundant braces

[JDevlieghere]

Fixed Adrian's comments.

I also discovered that I was using this false positive to test the dsymutil -verify feature, so I had to fixing that test as well.

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

413 ↗

(On Diff #133616)

No. The call sites have respectively ArrayRef<uint8t_t> and SmallVector<char, 4> as argument.

[probinson]

I'd have though invalid.s should be a text file from the name, but it shows up as binary here?

[JDevlieghere]

In D40199#1004154, @probinson wrote:

I'd have though invalid.s should be a text file from the name, but it shows up as binary here?

Not sure why Phabricator didn't pick it up. Here's its content: https://reviews.llvm.org/P8063

[JDevlieghere]

• Have VerifyLocation lambda take a StringRef as Adrian suggested.

[aprantl]

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

427 ↗

(On Diff #134721)

-> llvm::toStringRef(ArrayRef<uint8_t> Input)

[JDevlieghere]

Cleanup

[aprantl]

That's a lot cleaner!

[clayborg]

I look forward to this being checked in

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

if (auto SectionOffset = AttrValue.Value.getAsSectionOffset()) {
       if (*SectionOffset >= DObj.getLineSection().Data.size())
         ReportError("DW_AT_stmt_list offset is beyond .debug_line bounds: " +
                     llvm::formatv("{0:x8}", *SectionOffset));
       break;
     }
     ReportError("DIE has invalid DW_AT_stmt_list encoding:");
     break;
   case DW_AT_location: {
-    Optional<ArrayRef<uint8_t>> Expr = AttrValue.Value.getAsBlock();
-    if (!Expr) {
-      ReportError("DIE has invalid DW_AT_location encoding:");
-      break;
-    }
-
+    auto VerifyLocation = [&](StringRef D) {
       DWARFUnit *U = Die.getDwarfUnit();
-    DataExtractor Data(
-        StringRef(reinterpret_cast<const char *>(Expr->data()), Expr->size()),
-        DCtx.isLittleEndian(), 0);
-    DWARFExpression Expression(Data, U->getVersion(), U->getAddressByteSize());
+      DataExtractor Data(D, DCtx.isLittleEndian(), 0);
+      DWARFExpression Expression(Data, U->getVersion(),
+                                 U->getAddressByteSize());
       bool Error = llvm::any_of(Expression, [](DWARFExpression::Operation &Op) {
         return Op.isError();
       });
       if (Error)
         ReportError("DIE contains invalid DWARF expression:");
+    };
+    if (Optional<ArrayRef<uint8_t>> Expr = AttrValue.Value.getAsBlock()) {
+      // Verify inlined location.
+      VerifyLocation(llvm::toStringRef(*Expr));
+    } else if (auto LocOffset = AttrValue.Value.getAsUnsignedConstant()) {
+      // Verify location list.
+      if (auto DebugLoc = DCtx.getDebugLoc())
+        if (auto LocList = DebugLoc->getLocationListAtOffset(*LocOffset))
+          for (const auto &Entry : LocList->Entries)
+            VerifyLocation({Entry.Loc.data(), Entry.Loc.size()});
+    }
     break;
   }
 
   default:
     break;
   }
   return NumErrors;
 }

llvm/trunk/test/tools/dsymutil/Inputs/invalid.s

+# The DW_AT_low_pc was modified to be greater than the DW_AT_high_pc.
+
+  .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 1 0                   ## inlined.c:1: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 2 1 prologue_end      ## inlined.c:2:1
+  popq  %rbp
+  retq
+Ltmp1:
+Lfunc_end0:
+  .cfi_endproc
+                                        ## -- End function
+  .globl  _main                   ## -- Begin function main
+  .p2align  4, 0x90
+_main:                                  ## @main
+Lfunc_begin1:
+  .loc  1 3 0                   ## inlined.c:3: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
+  xorl  %eax, %eax
+Ltmp2:
+  .loc  1 5 1 prologue_end      ## inlined.c:5:1
+  popq  %rbp
+  retq
+Ltmp3:
+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  "main"                  ## string offset=46
+  .asciz  "int"                   ## string offset=51
+  .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 19                      ## DW_AT_low_pc <- LOW PC > HIGH 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 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 3                       ## 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 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 19                      ## DW_FORM_ref4
+  .byte 63                      ## DW_AT_external
+  .byte 25                      ## DW_FORM_flag_present
+  .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
+  .byte 62                      ## DW_AT_encoding
+  .byte 11                      ## DW_FORM_data1
+  .byte 11                      ## DW_AT_byte_size
+  .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 92                      ## 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:0x55 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:0x15 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 44                      ## DW_AT_name
+  .byte 1                       ## DW_AT_decl_file
+  .byte 1                       ## DW_AT_decl_line
+                                        ## DW_AT_external
+  .byte 3                       ## Abbrev [3] 0x3f:0x19 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 3                       ## DW_AT_decl_line
+  .long 88                      ## DW_AT_type
+                                        ## DW_AT_external
+  .byte 4                       ## Abbrev [4] 0x58:0x7 DW_TAG_base_type
+  .long 51                      ## DW_AT_name
+  .byte 5                       ## DW_AT_encoding
+  .byte 4                       ## DW_AT_byte_size
+  .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 2090499946              ## Hash in Bucket 0
+  .long LNames1-Lnames_begin    ## Offset in Bucket 0
+  .long LNames0-Lnames_begin    ## Offset in Bucket 0
+LNames1:
+  .long 44                      ## g
+  .long 1                       ## Num DIEs
+  .long 42
+  .long 0
+LNames0:
+  .long 46                      ## main
+  .long 1                       ## Num DIEs
+  .long 63
+  .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 1                       ## 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 0                       ## Bucket 0
+  .long 193495088               ## Hash in Bucket 0
+  .long Ltypes0-Ltypes_begin    ## Offset in Bucket 0
+Ltypes0:
+  .long 51                      ## int
+  .long 1                       ## Num DIEs
+  .long 88
+  .short  36
+  .byte 0
+  .long 0
+
+.subsections_via_symbols
+  .section  __DWARF,__debug_line,regular,debug
+Lsection_line:
+Lline_table_start0:

llvm/trunk/test/tools/dsymutil/X86/verify.test

-# Multiple inputs in flat mode
-RUN: not llvm-dsymutil -verify -oso-prepend-path=%p/.. %p/../Inputs/basic.macho.x86_64 %p/../Inputs/basic-archive.macho.x86_64 %p/../Inputs/basic-lto.macho.x86_64 %p/../Inputs/basic-lto-dw4.macho.x86_64 -o %t 2>&1 | FileCheck %s --check-prefix=QUIET
-RUN: not llvm-dsymutil -verify -verbose -oso-prepend-path=%p/.. %p/../Inputs/basic.macho.x86_64 %p/../Inputs/basic-archive.macho.x86_64 %p/../Inputs/basic-lto.macho.x86_64 %p/../Inputs/basic-lto-dw4.macho.x86_64 -o %t 2>&1 | FileCheck %s --check-prefix=QUIET --check-prefix=VERBOSE
+# Positive tests in regular and verbose mode.
+# RUN: llvm-dsymutil -verify -oso-prepend-path=%p/.. %p/../Inputs/basic.macho.x86_64 %p/../Inputs/basic-archive.macho.x86_64 %p/../Inputs/basic-lto.macho.x86_64 %p/../Inputs/basic-lto-dw4.macho.x86_64 -o %t 2>&1 | FileCheck %s --allow-empty --check-prefix=QUIET-SUCCESS
+# RUN: llvm-dsymutil -verify -verbose -oso-prepend-path=%p/.. %p/../Inputs/basic.macho.x86_64 %p/../Inputs/basic-archive.macho.x86_64 %p/../Inputs/basic-lto.macho.x86_64 %p/../Inputs/basic-lto-dw4.macho.x86_64 -o %t 2>&1 | FileCheck %s --check-prefix=QUIET-SUCCESS --check-prefix=VERBOSE
 
-VERBOSE: Verifying DWARF for architecture: x86_64
-VERBOSE: error: DIE has invalid DW_AT_location encoding:
-VERBOSE: error: DIE has invalid DW_AT_location encoding:
-QUIET: error: verification failed
+# VERBOSE: Verifying DWARF for architecture: x86_64
+# QUIET-SUCCESS-NOT: error: verification failed
+
+# Negative tests in regular and verbose mode.
+# (Invalid object generated from ../Inputs/invalid.s by modified the low PC.)
+# RUN: not llvm-dsymutil -verify -oso-prepend-path=%p/../Inputs -y %s -o %t 2>&1 | FileCheck %s --check-prefix=QUIET-FAIL
+# RUN: not llvm-dsymutil -verify -verbose -oso-prepend-path=%p/../Inputs -y %s -o %t 2>&1 | FileCheck %s --check-prefix=QUIET-FAIL --check-prefix=VERBOSE
+
+# QUIET-FAIL: error: verification failed
+
+---
+triple:          'x86_64-apple-darwin'
+objects:
+  - filename:        invalid.o
+    timestamp:       1518197670
+    symbols:
+      - { sym: _main, objAddr: 0x0000000000000010, binAddr: 0x0000000100000FB0, size: 0x00000008 }
+      - { sym: _g, objAddr: 0x0000000000000000, binAddr: 0x0000000100000FA0, size: 0x00000010 }
+...

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

 # RUN: llvm-mc %s -filetype obj -triple x86_64-linux-elf -o - \
 # RUN: | llvm-dwarfdump --debug-loc - \
 # RUN: | FileCheck %s
 
+# RUN: llvm-mc %s -filetype obj -triple x86_64-linux-elf -o - \
+# RUN: | llvm-dwarfdump --verify - \
+# RUN: | FileCheck %s --check-prefix VERIFY
+
 # CHECK: .debug_loc contents:
 
 # CHECK: 0x00000000:
 # CHECK-NEXT: [0x0000000000000000, 0x0000000000000003): DW_OP_reg5 RDI
 # CHECK-NEXT: [0x0000000000000003, 0x0000000000000004): DW_OP_reg0 RAX
 
 # CHECK: 0x00000036:
 # CHECK-NEXT: [0x0000000000000010, 0x0000000000000013): DW_OP_reg5 RDI
 # CHECK-NEXT: [0x0000000000000013, 0x0000000000000014): DW_OP_reg0 RAX
 
+# VERIFY: Verifying .debug_info Unit Header Chain
+# VERIFY-NOT: DIE has invalid DW_AT_location encoding
+
 # Source:
 #   int* foo(int* i) { return i; }
 #   int* bar(int* i) { return i; }
 # Compile with:
 #   clang -g -O3 test.c -S -o debugloc.s
 
   .text
   .file "test.c"