From ac6df7fcc52ba41d328e242d15e0374f5fc35675 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 8 Jul 2022 21:15:52 +0000 Subject: [PATCH] llvm-dwarfdump: Don't crash if DW_AT_{decl,call}_{file,line} uses signed form 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::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. Reviewed By: dblaikie Differential Revision: https://reviews.llvm.org/D129392 --- llvm/lib/DebugInfo/DWARF/DWARFDie.cpp | 30 +++++---- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 8 +++ .../X86/verify_file_encoding.yaml | 66 ++++++++++++++++++- 3 files changed, 92 insertions(+), 12 deletions(-) diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp index 96c546250974c5..33f79baf483da3 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp @@ -136,23 +136,31 @@ static void dumpAttribute(raw_ostream &OS, const DWARFDie &Die, 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 (LT->getFileNameByIndex( - *FormValue.getAsUnsignedConstant(), U->getCompilationDir(), - DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath, File)) { - File = '"' + File + '"'; - Name = File; + if (const auto *LT = U->getContext().getLineTableForUnit(U)) { + if (Optional Val = FormValue.getAsUnsignedConstant()) { + if (LT->getFileNameByIndex( + *Val, U->getCompilationDir(), + DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath, + File)) { + File = '"' + File + '"'; + Name = File; + } } + } } else if (Optional 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 && - (FormValue.getAsAddress() == - dwarf::computeTombstoneAddress(U->getAddressByteSize()))) { + else if (Attr == DW_AT_decl_line || Attr == DW_AT_call_line) { + if (Optional 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 << " ("; diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index c704f8f583af83..2be2a12aa0256a 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -704,6 +704,14 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die, } 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; } diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml index f9029952cf5856..af55a3a7d10344 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml @@ -14,6 +14,42 @@ # 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: @@ -28,6 +64,7 @@ DWARF: - main - '' - inline1 + - foo debug_abbrev: - Table: - Code: 0x0000000000000001 @@ -68,8 +105,26 @@ DWARF: 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 @@ -93,6 +148,15 @@ DWARF: - 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