llvm-dwarfdump: Don't crash if DW_AT_{decl,call}_{file,line} uses signed form

[palves]

Summary

The DWARF spec says:

Any debugging information entry representing the declaration of an object,
module, subprogram or type may have DW_AT_decl_file, DW_AT_decl_line and
DW_AT_decl_column attributes, each of whose value is an unsigned integer

							 ^^^^^^^^

constant.

If however, a producer happens to emit DW_AT_decl_file /
DW_AT_decl_line using a signed integer form, llvm-dwarfdump crashes,
like so:

(... snip ...)
0x000000b4:   DW_TAG_structure_type
                DW_AT_name      ("test_struct")
                DW_AT_byte_size (136)
                DW_AT_decl_file (llvm-dwarfdump: (... snip ...)/llvm/include/llvm/ADT/Optional.h:197: T& llvm::optional_detail::OptionalStorage<T, true>::getValue() &

[with T = long unsigned int]: Assertion `hasVal' failed.

PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.      Program arguments: /opt/rocm/llvm/bin/llvm-dwarfdump ./testsuite/outputs/gdb.rocm/lane-pc-vega20/lane-pc-vega20-kernel.so
 #0 0x000055cc8e78315f PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
 #1 0x000055cc8e780d3d SignalHandler(int) Signals.cpp:0:0
 #2 0x00007f8f2cae8420 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
 #3 0x00007f8f2c58d00b raise /build/glibc-SzIz7B/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #4 0x00007f8f2c56c859 abort /build/glibc-SzIz7B/glibc-2.31/stdlib/abort.c:81:7
 #5 0x00007f8f2c56c729 get_sysdep_segment_value /build/glibc-SzIz7B/glibc-2.31/intl/loadmsgcat.c:509:8
 #6 0x00007f8f2c56c729 _nl_load_domain /build/glibc-SzIz7B/glibc-2.31/intl/loadmsgcat.c:970:34
 #7 0x00007f8f2c57dfd6 (/lib/x86_64-linux-gnu/libc.so.6+0x33fd6)
 #8 0x000055cc8e58ceb9 llvm::DWARFDie::dump(llvm::raw_ostream&, unsigned int, llvm::DIDumpOptions) const (/opt/rocm/llvm/bin/llvm-dwarfdump+0x2e0eb9)
 #9 0x000055cc8e58bec3 llvm::DWARFDie::dump(llvm::raw_ostream&, unsigned int, llvm::DIDumpOptions) const (/opt/rocm/llvm/bin/llvm-dwarfdump+0x2dfec3)
#10 0x000055cc8e5b28a3 llvm::DWARFCompileUnit::dump(llvm::raw_ostream&, llvm::DIDumpOptions) (.part.21) DWARFCompileUnit.cpp:0:0

Likewise with DW_AT_call_file / DW_AT_call_line.

The problem is that the code in llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
dumping these attributes assumes that
FormValue.getAsUnsignedConstant() returns an armed optional. If in
debug mode, we get an assertion line the above. If in release mode,
and asserts are compiled out, then we proceed as if the optional had a
value, running into undefined behavior, printing whatever random
value.

Fix this by checking whether the optional returned by
FormValue.getAsUnsignedConstant() has a value, like done in other
places.

In addition, DWARFVerifier.cpp is validating DW_AT_call_file /
DW_AT_decl_file, but not AT_call_line / DW_AT_decl_line. This commit
fixes that too.

The llvm-dwarfdump/X86/verify_file_encoding.yaml testcase is extended
to cover these cases. Current llvm-dwarfdump crashes running the
newly-extended test.

"make check-llvm-tools-llvm-dwarfdump" shows no regressions, on x86-64
GNU/Linux.

[palves]

This is the first time I've ever uploaded an llvm patch, and I've used arc from the command line. Hope I've not messed this up too badly.

BTW, I ran into this with hand-coded DWARF, using GDB's DWARF-assembler machinery.

For example, here's a GDB testcase fix for a test that was using DW_FORM_sdata forms for decl_line:
https://sourceware.org/pipermail/gdb-patches/2022-July/190570.html

[dblaikie]

Sure, sounds OK.

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

154–158

Generally LLVM code doesn't put {} on single-line blocks like these.

[palves]

In D129392#3639787, @dblaikie wrote:

Sure, sounds OK.

Thanks! Could someone merge it for me then? I don't have write access.

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

154–158

I think I put those in to avoid dangling-else warnings.

[palves]

Remove unnecessary braces.

[palves]

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

154–158

Turns out there's no dangling-else warning here. I've uploaded an update to removes the unnecessary braces.

[palves]

Yes more unnecessary braces removed. I think this is ready now.

[dblaikie]

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

139–148

I /think/ LLVM's style guide now doesn't encourage omitting braces from single statement but multi-line blocks like this, so I left these ones in.

154–158

Ah, removed these in a follow-up commit ( 84960896236338eb1c7b30f34041214e64db0bd1 )

[palves]

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

154–158

Cool, thank you very much, and sorry for the extra work. Much appreciated!

[clayborg]

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

139–148

Do we want to dump the attribute as a number if we can't get the lint table of the value as unsigned here? What happens when we dump a DW_AT_decl_file or DW_AT_call_file that has a signed integer encoding? Nothing comes out? Seems like we should do a "FormValue.dump(OS, DumpOpts);" if either the lit table or optional uint64_t fail to get extracted

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

707–708

Do we want to handle DW_AT_decl_column and DW_AT_call_column here as well?

[dblaikie]

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

139–148

Do we want to dump the attribute as a number if we can't get the lint table of the value as unsigned here?

I believe we do that - at least looks like it according to the test touched in this patch & the fallback at the end of the if/else if/else chain below.

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

707–708

Sure - I'd be open to that.

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

static void dumpAttribute(raw_ostream &OS, const DWARFDie &Die,
 
   OS << "\t(";
 
   StringRef Name;
   std::string File;
   auto Color = HighlightColor::Enumerator;
   if (Attr == DW_AT_decl_file || Attr == DW_AT_call_file) {
     Color = HighlightColor::String;
     if (const auto *LT = U->getContext().getLineTableForUnit(U))
+      if (Optional<uint64_t> Val = FormValue.getAsUnsignedConstant())
         if (LT->getFileNameByIndex(
-              *FormValue.getAsUnsignedConstant(), U->getCompilationDir(),
-              DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath, File)) {
+                *Val, U->getCompilationDir(),
+                DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath,
+                File)) {
           File = '"' + File + '"';
           Name = File;
         }
   } else if (Optional<uint64_t> Val = FormValue.getAsUnsignedConstant())
     Name = AttributeValueString(Attr, *Val);
 
   if (!Name.empty())
     WithColor(OS, Color) << Name;
-  else if (Attr == DW_AT_decl_line || Attr == DW_AT_call_line)
-    OS << *FormValue.getAsUnsignedConstant();
-  else if (Attr == DW_AT_low_pc &&
+  else if (Attr == DW_AT_decl_line || Attr == DW_AT_call_line) {
+    if (Optional<uint64_t> Val = FormValue.getAsUnsignedConstant())
+      OS << *Val;
+    else
+      FormValue.dump(OS, DumpOpts);
+  } else if (Attr == DW_AT_low_pc &&
              (FormValue.getAsAddress() ==
               dwarf::computeTombstoneAddress(U->getAddressByteSize()))) {
     if (DumpOpts.Verbose) {
       FormValue.dump(OS, DumpOpts);
       OS << " (";
     }
     OS << "dead code";
     if (DumpOpts.Verbose)
       OS << ')';
   } else if (Attr == DW_AT_high_pc && !DumpOpts.ShowForm && !DumpOpts.Verbose &&

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

if (auto FileIdx = AttrValue.Value.getAsUnsignedConstant()) {
                     " and the compile unit has no line table");
       }
     } else {
       ReportError("DIE has " + AttributeString(Attr) +
                   " with invalid encoding");
     }
     break;
   }
+  case DW_AT_call_line:
+  case DW_AT_decl_line: {
+    if (!AttrValue.Value.getAsUnsignedConstant()) {
+      ReportError("DIE has " + AttributeString(Attr) +
+                  " with invalid encoding");
+    }
+    break;
+  }
   default:
     break;
   }
   return NumErrors;
 }
 
 unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
                                             DWARFAttribute &AttrValue,

llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml

 # RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error:
 
 # CHECK:      error: DIE has DW_AT_decl_file with invalid encoding{{[[:space:]]}}
 # CHECK-NEXT: 0x0000001a: DW_TAG_subprogram
 # CHECK-NEXT:               DW_AT_name        [DW_FORM_strp] ( .debug_str[0x0000000d] = "main")
 # CHECK-NEXT:               DW_AT_low_pc      [DW_FORM_addr] (0x0000000000001000)
 # CHECK-NEXT:               DW_AT_high_pc     [DW_FORM_addr] (0x0000000000002000)
 # CHECK-NEXT:               DW_AT_decl_file   [DW_FORM_strp] ( .debug_str[0x00000012] = "")
 # CHECK-NEXT:               DW_AT_call_line   [DW_FORM_data1] (5){{[[:space:]]}}
 # CHECK-NEXT: error: DIE has DW_AT_call_file with invalid encoding{{[[:space:]]}}
 # CHECK-NEXT: 0x00000034: DW_TAG_inlined_subroutine
 # CHECK-NEXT:               DW_AT_name        [DW_FORM_strp] ( .debug_str[0x00000013] = "inline1")
 # CHECK-NEXT:               DW_AT_low_pc      [DW_FORM_addr] (0x0000000000001100)
 # CHECK-NEXT:               DW_AT_high_pc     [DW_FORM_data4] (0x00000100)
 # CHECK-NEXT:               DW_AT_call_file   [DW_FORM_strp] ( .debug_str[0x00000012] = "")
 # CHECK-NEXT:               DW_AT_call_line   [DW_FORM_data1] (10){{[[:space:]]}}
+# CHECK-NEXT: error: DIE has DW_AT_decl_file with invalid encoding{{[[:space:]]}}
+# CHECK-NEXT: 0x0000004a: DW_TAG_subprogram
+# CHECK-NEXT:               DW_AT_name        [DW_FORM_strp] ( .debug_str[0x0000001b] = "foo")
+# CHECK-NEXT:               DW_AT_low_pc      [DW_FORM_addr] (0x0000000000002000)
+# CHECK-NEXT:               DW_AT_high_pc     [DW_FORM_addr] (0x0000000000003000)
+# CHECK-NEXT:               DW_AT_decl_file   [DW_FORM_sdata] (2)
+# CHECK-NEXT:               DW_AT_decl_line   [DW_FORM_sdata] (3)
+# CHECK-NEXT:               DW_AT_call_file   [DW_FORM_sdata] (4)
+# CHECK-NEXT:               DW_AT_call_line   [DW_FORM_sdata] (5){{[[:space:]]}}
+# CHECK-NEXT: error: DIE has DW_AT_decl_line with invalid encoding{{[[:space:]]}}
+# CHECK-NEXT: 0x0000004a: DW_TAG_subprogram
+# CHECK-NEXT:               DW_AT_name        [DW_FORM_strp] ( .debug_str[0x0000001b] = "foo")
+# CHECK-NEXT:               DW_AT_low_pc      [DW_FORM_addr] (0x0000000000002000)
+# CHECK-NEXT:               DW_AT_high_pc     [DW_FORM_addr] (0x0000000000003000)
+# CHECK-NEXT:               DW_AT_decl_file   [DW_FORM_sdata] (2)
+# CHECK-NEXT:               DW_AT_decl_line   [DW_FORM_sdata] (3)
+# CHECK-NEXT:               DW_AT_call_file   [DW_FORM_sdata] (4)
+# CHECK-NEXT:               DW_AT_call_line   [DW_FORM_sdata] (5){{[[:space:]]}}
+# CHECK-NEXT: error: DIE has DW_AT_call_file with invalid encoding{{[[:space:]]}}
+# CHECK-NEXT: 0x0000004a: DW_TAG_subprogram
+# CHECK-NEXT:               DW_AT_name        [DW_FORM_strp] ( .debug_str[0x0000001b] = "foo")
+# CHECK-NEXT:               DW_AT_low_pc      [DW_FORM_addr] (0x0000000000002000)
+# CHECK-NEXT:               DW_AT_high_pc     [DW_FORM_addr] (0x0000000000003000)
+# CHECK-NEXT:               DW_AT_decl_file   [DW_FORM_sdata] (2)
+# CHECK-NEXT:               DW_AT_decl_line   [DW_FORM_sdata] (3)
+# CHECK-NEXT:               DW_AT_call_file   [DW_FORM_sdata] (4)
+# CHECK-NEXT:               DW_AT_call_line   [DW_FORM_sdata] (5){{[[:space:]]}}
+# CHECK-NEXT: error: DIE has DW_AT_call_line with invalid encoding{{[[:space:]]}}
+# CHECK-NEXT: 0x0000004a: DW_TAG_subprogram
+# CHECK-NEXT:               DW_AT_name        [DW_FORM_strp] ( .debug_str[0x0000001b] = "foo")
+# CHECK-NEXT:               DW_AT_low_pc      [DW_FORM_addr] (0x0000000000002000)
+# CHECK-NEXT:               DW_AT_high_pc     [DW_FORM_addr] (0x0000000000003000)
+# CHECK-NEXT:               DW_AT_decl_file   [DW_FORM_sdata] (2)
+# CHECK-NEXT:               DW_AT_decl_line   [DW_FORM_sdata] (3)
+# CHECK-NEXT:               DW_AT_call_file   [DW_FORM_sdata] (4)
+# CHECK-NEXT:               DW_AT_call_line   [DW_FORM_sdata] (5){{[[:space:]]}}
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
   Machine: EM_X86_64
 DWARF:
   debug_str:
     - ''
     - '/tmp/main.c'
     - main
     - ''
     - inline1
+    - foo
   debug_abbrev:
     - Table:
         - Code:            0x0000000000000001
           Tag:             DW_TAG_compile_unit
           Children:        DW_CHILDREN_yes
           Attributes:
             - Attribute:       DW_AT_name
               Form:            DW_FORM_strp
- Table:
             - Attribute:       DW_AT_low_pc
               Form:            DW_FORM_addr
             - Attribute:       DW_AT_high_pc
               Form:            DW_FORM_data4
             - Attribute:       DW_AT_call_file
               Form:            DW_FORM_strp
             - Attribute:       DW_AT_call_line
               Form:            DW_FORM_data1
+        - Code:            0x0000000000000004
+          Tag:             DW_TAG_subprogram
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute:       DW_AT_name
+              Form:            DW_FORM_strp
+            - Attribute:       DW_AT_low_pc
+              Form:            DW_FORM_addr
+            - Attribute:       DW_AT_high_pc
+              Form:            DW_FORM_addr
+            - Attribute:       DW_AT_decl_file
+              Form:            DW_FORM_sdata
+            - Attribute:       DW_AT_decl_line
+              Form:            DW_FORM_sdata
+            - Attribute:       DW_AT_call_file
+              Form:            DW_FORM_sdata
+            - Attribute:       DW_AT_call_line
+              Form:            DW_FORM_sdata
   debug_info:
-    - Length:          0x0000000000000048
+    - Length:          0x0000000000000061
       Version:         4
       AbbrOffset:      0x0000000000000000
       AddrSize:        8
       Entries:
         - AbbrCode:        0x00000001
           Values:
             - Value:           0x0000000000000001
             - Value:           0x0000000000000002
             - Value:           0x0000000000000000
         - AbbrCode:        0x00000002
           Values:
             - Value:           0x000000000000000D
             - Value:           0x0000000000001000
             - Value:           0x0000000000002000
             - Value:           0x0000000000000012
             - Value:           0x0000000000000005
         - AbbrCode:        0x00000003
           Values:
             - Value:           0x0000000000000013
             - Value:           0x0000000000001100
             - Value:           0x0000000000000100
             - Value:           0x0000000000000012
             - Value:           0x000000000000000A
+        - AbbrCode:        0x00000004
+          Values:
+            - Value:           0x000000000000001B
+            - Value:           0x0000000000002000
+            - Value:           0x0000000000003000
+            - Value:           0x0000000000000002
+            - Value:           0x0000000000000003
+            - Value:           0x0000000000000004
+            - Value:           0x0000000000000005
         - AbbrCode:        0x00000000
           Values:          []
         - AbbrCode:        0x00000000
           Values:          []
 ...