diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp --- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp @@ -916,6 +916,25 @@ " is not terminated", DebugLineOffset)); + // Check that we haven't gone past the expected end of the table. This can + // only happen when an opcode starts before the expected end, and finishes + // after. As the offset isn't incremented by the data extractor when + // reading past the end of the available data, this will not detect cases + // where the overflowing opcode appears at the end of the section. However, in + // such cases, an unterminated sequence error will be raised instead, as the + // data extractor will return zeroes for the trailing bytes (which do not + // correspond to an end sequence instruction). + if (*OffsetPtr > EndOffset) { + RecoverableErrorCallback(createStringError( + errc::invalid_argument, + "last opcode in line table at offset 0x%8.8" PRIx64 + " ended at offset 0x%8.8" PRIx64 + " which is past the expected table end at offset 0x%8.8" PRIx64, + DebugLineOffset, *OffsetPtr, EndOffset)); + // Recover from this error by resetting the offset back to the expected end. + *OffsetPtr = EndOffset; + } + // Sort all sequences so that address lookup will work faster. if (!Sequences.empty()) { llvm::sort(Sequences, Sequence::orderByHighPC); diff --git a/llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s b/llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s --- a/llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s +++ b/llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s @@ -383,6 +383,35 @@ .byte 0, 1, 1 # DW_LNE_end_sequence .Linvalid_md5_end1: +# Opcode extends past the end of the table, as claimed by the unit length field. +.long .Lshort_unit_length_end - .Lshort_unit_length_start # Length of Unit +.Lshort_unit_length_start: +.short 4 # DWARF version number +.long .Lprologue_short_unit_length_end-.Lprologue_short_unit_length_start # Length of Prologue +.Lprologue_short_unit_length_start: +.byte 1 # Minimum Instruction Length +.byte 1 # Maximum Operations per Instruction +.byte 1 # Default is_stmt +.byte -5 # Line Base +.byte 14 # Line Range +.byte 13 # Opcode Base +.byte 0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 # Standard Opcode Lengths +.asciz "dir1" # Include table +.asciz "dir2" +.byte 0 +.asciz "file1" # File table +.byte 0, 0, 0 +.asciz "file2" +.byte 1, 0, 0 +.byte 0 +.Lprologue_short_unit_length_end: +.byte 0, 9, 2 # DW_LNE_set_address +.quad 0xfeedfeed +.byte 1 # DW_LNS_copy +.byte 0, 9, 2 # DW_LNE_set_address +.long 0xf001 # Truncated address (should be 8 bytes) +.Lshort_unit_length_end: + # Trailing good section. .long .Lunit_good_end - .Lunit_good_start # Length of Unit (DWARF-32 format) .Lunit_good_start: diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test b/llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test --- a/llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test +++ b/llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test @@ -36,7 +36,7 @@ # RUN: FileCheck %s --input-file=%t-malformed-off-first.err --check-prefix=ALL ## Don't stop looking for the later unit if non-fatal issues are found. -# RUN: llvm-dwarfdump -debug-line=0x2ec %t-malformed.o 2> %t-malformed-off-last.err \ +# RUN: llvm-dwarfdump -debug-line=0x339 %t-malformed.o 2> %t-malformed-off-last.err \ # RUN: | FileCheck %s --check-prefix=LAST --implicit-check-not='debug_line[{{.*}}]' # RUN: FileCheck %s --input-file=%t-malformed-off-last.err --check-prefix=ALL @@ -150,7 +150,15 @@ # NONFATAL: 0x0000000000000000 {{.*}} epilogue_begin # NONFATAL: 0x4321432143214321 {{.*}} is_stmt end_sequence -# LAST: debug_line[0x000002ec] +## Case 13: Last operand goes past unit end. +# NONFATAL: debug_line[0x000002ec] +# NONFATAL-NEXT: Line table prologue +# NONFATAL: 0x00000000feedfeed {{.*}} is_stmt +## The truncated row is not added by any operation, so is not recorded in the +## table. +# NONFATAL-NOT: is_stmt + +# LAST: debug_line[0x00000339] # LAST: 0x00000000cafebabe {{.*}} end_sequence # RESERVED: warning: parsing line table prologue at offset 0x00000048 unsupported reserved unit length found of value 0xfffffffe @@ -173,4 +181,6 @@ # ALL-NEXT: warning: parsing line table prologue at 0x000002ae found an invalid directory or file table description at 0x000002e0 # ALL-NEXT: warning: failed to parse file entry because the MD5 hash is invalid # ALL-NEXT: warning: parsing line table prologue at 0x000002ae should have ended at 0x000002d9 but it ended at 0x000002e0 +# OTHER-NEXT: warning: last sequence in debug line table at offset 0x000002ec is not terminated +# OTHER-NEXT: warning: last opcode in line table at offset 0x000002ec ended at offset 0x0000033d which is past the expected table end at offset 0x00000339 # ALL-NOT: warning: diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp --- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp +++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp @@ -378,6 +378,7 @@ (Format == DWARF64 ? "DWARF64" : "DWARF32")); LineTable < = Gen->addLineTable(Format); + LT.addByte(0xaa); DWARFDebugLine::Prologue Prologue = LT.createBasicPrologue(); ++Prologue.PrologueLength; LT.setPrologue(Prologue); @@ -391,10 +392,9 @@ // Undo the earlier modification so that it can be compared against a // "default" prologue. --Result.Prologue.PrologueLength; - checkDefaultPrologue(Version, Format, Result.Prologue, 0); + checkDefaultPrologue(Version, Format, Result.Prologue, 1); - uint64_t ExpectedEnd = - Prologue.TotalLength + 1 + Prologue.sizeofTotalLength(); + uint64_t ExpectedEnd = Prologue.TotalLength + Prologue.sizeofTotalLength(); checkError( (Twine("parsing line table prologue at 0x00000000 should have ended at " "0x000000") + @@ -526,9 +526,8 @@ LT.addStandardOpcode(DW_LNS_const_add_pc, {}); LT.addExtendedOpcode(1, DW_LNE_end_sequence, {}); DWARFDebugLine::Prologue Prologue = LT.createBasicPrologue(); - // Set the total length to 1 higher than the actual length. The program body - // has size 5. - Prologue.TotalLength += 6; + // Set the total length to 1 higher than the actual length. + ++Prologue.TotalLength; LT.setPrologue(Prologue); generate(); @@ -543,6 +542,39 @@ EXPECT_EQ((*ExpectedLineTable)->Sequences.size(), 1u); } +TEST_F(DebugLineBasicFixture, ErrorForUnitLengthTruncatingOpcode) { + if (!setupGenerator()) + return; + + LineTable &Padding = Gen->addLineTable(); + // Add some padding to show that a non-zero offset is handled correctly. + Padding.setCustomPrologue({{0, LineTable::Byte}}); + LineTable < = Gen->addLineTable(); + LT.addStandardOpcode(DW_LNS_copy, {}); + LT.addStandardOpcode(DW_LNS_const_add_pc, {}); + LT.addExtendedOpcode(1, DW_LNE_end_sequence, {}); + DWARFDebugLine::Prologue Prologue = LT.createBasicPrologue(); + --Prologue.TotalLength; + LT.setPrologue(Prologue); + + generate(); + + DWARFDebugLine::LineTable Table; + uint64_t Offset = 1; + ASSERT_THAT_ERROR( + Table.parse(LineData, &Offset, *Context, nullptr, RecordRecoverable), + Succeeded()); + checkError( + "last opcode in line table at offset 0x00000001 ended at offset " + "0x00000034 which is past the expected table end at offset 0x00000033", + std::move(Recoverable)); + EXPECT_EQ(Table.Rows.size(), 2u); + EXPECT_EQ(Table.Sequences.size(), 1u); + // Show that the output offset is based on the unit length, not the actual + // amount of data parsed. + EXPECT_EQ(Offset, Prologue.TotalLength + Prologue.sizeofTotalLength() + 1); +} + TEST_F(DebugLineBasicFixture, ErrorForMismatchedAddressSize) { if (!setupGenerator(4, 8)) return; @@ -758,7 +790,10 @@ return; LineTable < = Gen->addLineTable(DWARF32); - LT.addExtendedOpcode(0x42, DW_LNE_end_sequence, {}); + LT.addExtendedOpcode(0x2, DW_LNE_end_sequence, {}); + // Arbitrary padding byte to ensure the end of the opcode as claimed by the + // opcode length field does not go past the table end. + LT.addByte(0xaa); LineTable <2 = Gen->addLineTable(DWARF32); LT2.addExtendedOpcode(9, DW_LNE_set_address, {{0x1234567890abcdef, LineTable::Quad}}); @@ -771,7 +806,7 @@ EXPECT_FALSE(Unrecoverable); ASSERT_FALSE(Parser.done()); checkError( - "unexpected line op length at offset 0x00000030 expected 0x42 found 0x01", + "unexpected line op length at offset 0x00000030 expected 0x02 found 0x01", std::move(Recoverable)); // Reset the error state so that it does not confuse the next set of checks. @@ -779,7 +814,7 @@ Parser.parseNext(RecordRecoverable, RecordUnrecoverable); EXPECT_TRUE(Parser.done()); - checkError("last sequence in debug line table at offset 0x00000031 is not " + checkError("last sequence in debug line table at offset 0x00000032 is not " "terminated", std::move(Recoverable)); EXPECT_FALSE(Unrecoverable); diff --git a/llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp b/llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp --- a/llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp +++ b/llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp @@ -175,6 +175,7 @@ P.TotalLength += 4; P.FormParams.Format = DWARF64; } + P.TotalLength += Contents.size(); P.FormParams.Version = Version; P.MinInstLength = 1; P.MaxOpsPerInst = 1;