diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h @@ -138,6 +138,7 @@ void clear(); void dump(raw_ostream &OS, DIDumpOptions DumpOptions) const; Error parse(const DWARFDataExtractor &DebugLineData, uint64_t *OffsetPtr, + function_ref RecoverableErrorCallback, const DWARFContext &Ctx, const DWARFUnit *U = nullptr); }; @@ -341,9 +342,12 @@ /// Skip the current line table and go to the following line table (if /// present) immediately. /// - /// \param ErrorCallback - report any prologue parsing issues via this - /// callback. - void skip(function_ref ErrorCallback); + /// \param RecoverableErrorCallback - report any recoverable prologue + /// parsing issues via this callback. + /// \param UnrecoverableErrorCallback - report any unrecoverable prologue + /// parsing issues via this callback. + void skip(function_ref RecoverableErrorCallback, + function_ref UnrecoverableErrorCallback); /// Indicates if the parser has parsed as much as possible. /// diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp --- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp @@ -462,7 +462,7 @@ Optional DumpOffset) { while (!Parser.done()) { if (DumpOffset && Parser.getOffset() != *DumpOffset) { - Parser.skip(dumpWarning); + Parser.skip(dumpWarning, dumpWarning); continue; } OS << "debug_line[" << format("0x%8.8" PRIx64, Parser.getOffset()) 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 @@ -299,10 +299,10 @@ return Error::success(); } -Error DWARFDebugLine::Prologue::parse(const DWARFDataExtractor &DebugLineData, - uint64_t *OffsetPtr, - const DWARFContext &Ctx, - const DWARFUnit *U) { +Error DWARFDebugLine::Prologue::parse( + const DWARFDataExtractor &DebugLineData, uint64_t *OffsetPtr, + function_ref RecoverableErrorCallback, const DWARFContext &Ctx, + const DWARFUnit *U) { const uint64_t PrologueOffset = *OffsetPtr; clear(); @@ -310,14 +310,19 @@ if (TotalLength == dwarf::DW_LENGTH_DWARF64) { FormParams.Format = dwarf::DWARF64; TotalLength = DebugLineData.getU64(OffsetPtr); - } else if (TotalLength >= dwarf::DW_LENGTH_lo_reserved) { + } else if (TotalLength >= dwarf::DW_LENGTH_lo_reserved) { + // Treat this error as unrecoverable - we have no way of knowing where the + // table ends. return createStringError(errc::invalid_argument, "parsing line table prologue at offset 0x%8.8" PRIx64 " unsupported reserved unit length found of value 0x%8.8" PRIx64, PrologueOffset, TotalLength); } FormParams.Version = DebugLineData.getU16(OffsetPtr); - if (getVersion() < 2) + if (getVersion() < 2) + // Treat this error as unrecoverable - we cannot be sure what any of + // the data represents including the length field, so cannot skip it or make + // any reasonable assumptions. return createStringError(errc::not_supported, "parsing line table prologue at offset 0x%8.8" PRIx64 " found unsupported version 0x%2.2" PRIx16, @@ -352,25 +357,32 @@ if (Error e = parseV5DirFileTables( DebugLineData, OffsetPtr, EndPrologueOffset, FormParams, Ctx, U, ContentTypes, IncludeDirectories, FileNames)) { - return joinErrors( + RecoverableErrorCallback(joinErrors( createStringError( errc::invalid_argument, "parsing line table prologue at 0x%8.8" PRIx64 " found an invalid directory or file table description at" " 0x%8.8" PRIx64, PrologueOffset, *OffsetPtr), - std::move(e)); + std::move(e))); + // Skip to the end of the prologue, since the chances are that the parser + // did not read the whole table. This prevents the length check below from + // executing. + if (*OffsetPtr < EndPrologueOffset) + *OffsetPtr = EndPrologueOffset; } } else parseV2DirFileTables(DebugLineData, OffsetPtr, EndPrologueOffset, ContentTypes, IncludeDirectories, FileNames); if (*OffsetPtr != EndPrologueOffset) - return createStringError(errc::invalid_argument, - "parsing line table prologue at 0x%8.8" PRIx64 - " should have ended at 0x%8.8" PRIx64 - " but it ended at 0x%8.8" PRIx64, - PrologueOffset, EndPrologueOffset, *OffsetPtr); + RecoverableErrorCallback(createStringError( + errc::invalid_argument, + "parsing line table prologue at 0x%8.8" PRIx64 + " should have ended at 0x%8.8" PRIx64 " but it ended at 0x%8.8" PRIx64, + PrologueOffset, EndPrologueOffset, *OffsetPtr)); + if (*OffsetPtr < EndPrologueOffset) + *OffsetPtr = EndPrologueOffset; return Error::success(); } @@ -516,7 +528,8 @@ clear(); - Error PrologueErr = Prologue.parse(DebugLineData, OffsetPtr, Ctx, U); + Error PrologueErr = Prologue.parse(DebugLineData, OffsetPtr, + RecoverableErrorCallback, Ctx, U); if (OS) { // The presence of OS signals verbose dumping. @@ -1137,14 +1150,16 @@ } void DWARFDebugLine::SectionParser::skip( - function_ref ErrorCallback) { + function_ref RecoverableErrorCallback, + function_ref UnrecoverableErrorCallback) { assert(DebugLineData.isValidOffset(Offset) && "parsing should have terminated"); DWARFUnit *U = prepareToParse(Offset); uint64_t OldOffset = Offset; LineTable LT; - if (Error Err = LT.Prologue.parse(DebugLineData, &Offset, Context, U)) - ErrorCallback(std::move(Err)); + if (Error Err = LT.Prologue.parse(DebugLineData, &Offset, + RecoverableErrorCallback, Context, U)) + UnrecoverableErrorCallback(std::move(Err)); moveToNextTable(OldOffset, LT.Prologue); } 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 @@ -64,7 +64,7 @@ .long .Lunit_short_prologue_end - .Lunit_short_prologue_start # unit length .Lunit_short_prologue_start: .short 4 # version -.long .Lprologue_short_prologue_end-.Lprologue_short_prologue_start - 2 # Length of Prologue +.long .Lprologue_short_prologue_end-.Lprologue_short_prologue_start - 1 # Length of Prologue .Lprologue_short_prologue_start: .byte 1 # Minimum Instruction Length .byte 1 # Maximum Operations per Instruction @@ -80,7 +80,8 @@ .byte 0, 0, 0 .asciz "file2" .byte 1, 2, 3 -.byte 0 +# FIXME: There should be an additional 0 byte here, but the file name parsing +# code does not recognise a missing null terminator. .Lprologue_short_prologue_end: .byte 0, 9, 2 # DW_LNE_set_address .quad 0x1122334455667788 @@ -91,7 +92,7 @@ .long .Lunit_long_prologue_end - .Lunit_long_prologue_start # unit length .Lunit_long_prologue_start: .short 4 # version -.long .Lprologue_long_prologue_end-.Lprologue_long_prologue_start + 1 # Length of Prologue +.long .Lprologue_long_prologue_end-.Lprologue_long_prologue_start # Length of Prologue .Lprologue_long_prologue_start: .byte 1 # Minimum Instruction Length .byte 1 # Maximum Operations per Instruction @@ -108,6 +109,8 @@ .asciz "file2" .byte 1, 2, 3 .byte 0 +# Skipped byte (treated as part of prologue). Would be DW_LNS_negate_stmt. +.byte 6 .Lprologue_long_prologue_end: .byte 0, 9, 2 # DW_LNE_set_address .quad 0x1111222233334444 @@ -323,7 +326,8 @@ .asciz "a.c" .byte 0 # Data to show that the rest of the prologue is skipped. -.byte 6 +.byte 6 # Would be interpreted as DW_LNS_negate_stmt if parsed + # as part of program. .Linvalid_md5_header_end0: .byte 0, 9, 2 # DW_LNE_set_address .quad 0x1234123412341234 @@ -364,8 +368,11 @@ # File table entries .byte 1 # 1 file .asciz "a.c" -.byte 6 # This byte will be consumed when reading the MD5 value. -.byte 0xb # This byte will not be read as part of the prologue. +.byte 6 # This byte will be consumed when reading the MD5 value and + # not interpreted as DW_LNS_negate_stmt in the program. +.byte 0xb # This byte will not be read as part of the prologue, and + # will be treated as part of the line table + # (DW_LNS_set_epilogue_begin) instead. .Linvalid_md5_header_end1: .byte 0, 9, 2 # DW_LNE_set_address .quad 0x4321432143214321 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 @@ -53,8 +53,7 @@ # FATAL-NEXT: total_length: 0xfffffffe # FATAL-NOT: debug_line -## For non-fatal prologue issues, the table prologue should be dumped, and any -## subsequent tables should also be. +## For non-fatal issues, the table data should be dumped. ## Case 1: Version 0 table. # NONFATAL: debug_line[0x00000048] # NONFATAL-NEXT: Line table prologue @@ -70,7 +69,7 @@ # NONFATAL-NEXT: Line table prologue # NONFATAL-NOT: include_directories # NONFATAL-NOT: file_names -# NONFATAL-NOT: Address +# NONFATAL: 0x8877665544332211 {{.*}} end_sequence ## Case 4: Prologue with length shorter than parsed. # NONFATAL: debug_line[0x00000081] @@ -80,11 +79,10 @@ # NONFATAL-NEXT: dir_index: 1 # NONFATAL-NEXT: mod_time: 0x00000002 # NONFATAL-NEXT: length: 0x00000003 -# NONFATAL-NOT: file_names -# NONFATAL-NOT: Address +# NONFATAL: 0x1122334455667788 {{.*}} end_sequence ## Case 5: Prologue with length longer than parsed. -# NONFATAL: debug_line[0x000000c9] +# NONFATAL: debug_line[0x000000c8] # NONFATAL-NEXT: Line table prologue # NONFATAL: file_names[ 2]: # NONFATAL-NEXT: name: "file2" @@ -92,7 +90,7 @@ # NONFATAL-NEXT: mod_time: 0x00000002 # NONFATAL-NEXT: length: 0x00000003 # NONFATAL-NOT: file_names -# NONFATAL-NOT: Address +# NONFATAL: 0x1111222233334444 {{.*}} is_stmt end_sequence ## Case 6: Extended opcode with incorrect length versus expected. # NONFATAL: debug_line[0x00000111] @@ -114,7 +112,7 @@ # NONFATAL-NEXT: file_names[ 0]: # NONFATAL-NEXT: name: "a.c" # NONFATAL-NEXT: dir_index: 1 -# NONFATAL-NOT: Address +# NONFATAL: 0x0000babb1ebabb1e {{.*}} end_sequence ## Case 9: V5 prologue ends during file table. # NONFATAL: debug_line[0x000001f2] @@ -123,7 +121,7 @@ # NONFATAL-NEXT: file_names[ 0]: # NONFATAL-NEXT: name: "a.c" # NONFATAL-NEXT: dir_index: 1 -# NONFATAL-NOT: Address +# NONFATAL: 0x00000ab4acadab4a {{.*}} end_sequence ## Case 10: V5 prologue ends during directory table. # NONFATAL: debug_line[0x00000232] @@ -132,14 +130,14 @@ # NONFATAL-NEXT: file_names[ 0]: # NONFATAL-NEXT: name: "a.c" # NONFATAL-NEXT: dir_index: 1 -# NONFATAL-NOT: Address +# NONFATAL: 0x4444333322221111 {{.*}} end_sequence ## Case 11: V5 invalid MD5 hash form when there is still data to be read. # NONFATAL: debug_line[0x00000272] # NONFATAL-NEXT: Line table prologue # NONFATAL: include_directories[ 0] = "/tmp" # NONFATAL-NOT: file_names -# NONFATAL-NOT: Address +# NONFATAL: 0x1234123412341234 {{.*}} is_stmt end_sequence ## Case 12: V5 invalid MD5 hash form when data beyond the prologue length has ## been read before the MD5 problem is identified. @@ -147,7 +145,7 @@ # NONFATAL-NEXT: Line table prologue # NONFATAL: include_directories[ 0] = "/tmp" # NONFATAL-NOT: file_names -# NONFATAL-NOT: Address +# NONFATAL: 0x4321432143214321 {{.*}} is_stmt epilogue_begin end_sequence # LAST: debug_line[0x000002f8] # LAST: 0x00000000cafebabe {{.*}} end_sequence @@ -159,9 +157,8 @@ # ALL-NEXT: warning: parsing line table prologue at offset 0x0000004e found unsupported version 0x01 # ALL-NEXT: warning: parsing line table prologue at 0x00000054 found an invalid directory or file table description at 0x00000073 # ALL-NEXT: warning: failed to parse entry content descriptions because no path was found -# FIXME - The latter offset in the next line should be 0xad. The filename parsing code does not notice a missing terminating byte. # ALL-NEXT: warning: parsing line table prologue at 0x00000081 should have ended at 0x000000b9 but it ended at 0x000000ba -# ALL-NEXT: warning: parsing line table prologue at 0x000000c9 should have ended at 0x00000104 but it ended at 0x00000103 +# ALL-NEXT: warning: parsing line table prologue at 0x000000c8 should have ended at 0x00000103 but it ended at 0x00000102 # OTHER-NEXT: warning: unexpected line op length at offset 0x00000158 expected 0x02 found 0x01 # OTHER-NEXT: warning: unexpected line op length at offset 0x0000015c expected 0x01 found 0x02 # OTHER-NEXT: warning: last sequence in debug line table is not terminated! @@ -172,4 +169,5 @@ # ALL-NEXT: warning: failed to parse file entry because the MD5 hash is invalid # ALL-NEXT: warning: parsing line table prologue at 0x000002b5 found an invalid directory or file table description at 0x000002e9 # ALL-NEXT: warning: failed to parse file entry because the MD5 hash is invalid +# ALL-NEXT: warning: parsing line table prologue at 0x000002b5 should have ended at 0x000002e0 but it ended at 0x000002e9 # 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 @@ -359,10 +359,15 @@ generate(); - checkGetOrParseLineTableEmitsFatalError( + auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context, + nullptr, RecordRecoverable); + EXPECT_THAT_EXPECTED(ExpectedLineTable, Succeeded()); + + checkError( {"parsing line table prologue at 0x00000000 found an invalid directory " "or file table description at 0x00000014", - "failed to parse entry content descriptions because no path was found"}); + "failed to parse entry content descriptions because no path was found"}, + std::move(Recoverable)); } TEST_P(DebugLineParameterisedFixture, ErrorForTooLargePrologueLength) { @@ -379,14 +384,24 @@ generate(); + auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context, + nullptr, RecordRecoverable); + ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded()); + DWARFDebugLine::LineTable Result(**ExpectedLineTable); + // Undo the earlier modification so that it can be compared against a + // "default" prologue. + --Result.Prologue.PrologueLength; + checkDefaultPrologue(Version, Format, Result.Prologue, 0); + uint64_t ExpectedEnd = Prologue.TotalLength + 1 + Prologue.sizeofTotalLength(); - checkGetOrParseLineTableEmitsFatalError( + checkError( (Twine("parsing line table prologue at 0x00000000 should have ended at " "0x000000") + Twine::utohexstr(ExpectedEnd) + " but it ended at 0x000000" + Twine::utohexstr(ExpectedEnd - 1)) - .str()); + .str(), + std::move(Recoverable)); } TEST_P(DebugLineParameterisedFixture, ErrorForTooShortPrologueLength) { @@ -408,16 +423,29 @@ generate(); + auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context, + nullptr, RecordRecoverable); + ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded()); + DWARFDebugLine::LineTable Result(**ExpectedLineTable); + // Undo the earlier modification so that it can be compared against a + // "default" prologue. + if (Version < 5) + Result.Prologue.PrologueLength += 2; + else + Result.Prologue.PrologueLength += 1; + checkDefaultPrologue(Version, Format, Result.Prologue, 0); + uint64_t ExpectedEnd = Prologue.TotalLength - 1 + Prologue.sizeofTotalLength(); if (Version < 5) --ExpectedEnd; - checkGetOrParseLineTableEmitsFatalError( + checkError( (Twine("parsing line table prologue at 0x00000000 should have ended at " "0x000000") + Twine::utohexstr(ExpectedEnd) + " but it ended at 0x000000" + Twine::utohexstr(ExpectedEnd + 1)) - .str()); + .str(), + std::move(Recoverable)); } INSTANTIATE_TEST_CASE_P( @@ -605,14 +633,15 @@ EXPECT_EQ(Parser.getOffset(), 0u); ASSERT_FALSE(Parser.done()); - Parser.skip(RecordUnrecoverable); + Parser.skip(RecordRecoverable, RecordUnrecoverable); EXPECT_EQ(Parser.getOffset(), 62u); ASSERT_FALSE(Parser.done()); - Parser.skip(RecordUnrecoverable); + Parser.skip(RecordRecoverable, RecordUnrecoverable); EXPECT_EQ(Parser.getOffset(), 136u); EXPECT_TRUE(Parser.done()); + EXPECT_FALSE(Recoverable); EXPECT_FALSE(Unrecoverable); } @@ -657,10 +686,11 @@ generate(); DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs); - Parser.skip(RecordUnrecoverable); + Parser.skip(RecordRecoverable, RecordUnrecoverable); EXPECT_EQ(Parser.getOffset(), 4u); EXPECT_TRUE(Parser.done()); + EXPECT_FALSE(Recoverable); checkError("parsing line table prologue at offset 0x00000000 unsupported " "reserved unit length found of value 0xfffffff0", @@ -735,11 +765,12 @@ generate(); DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs); - Parser.skip(RecordUnrecoverable); + Parser.skip(RecordRecoverable, RecordUnrecoverable); ASSERT_FALSE(Parser.done()); - Parser.skip(RecordUnrecoverable); + Parser.skip(RecordRecoverable, RecordUnrecoverable); EXPECT_TRUE(Parser.done()); + EXPECT_FALSE(Recoverable); checkError({"parsing line table prologue at offset 0x00000000 found " "unsupported version 0x00", @@ -757,9 +788,10 @@ generate(); DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs); - Parser.skip(RecordUnrecoverable); + Parser.skip(RecordRecoverable, RecordUnrecoverable); EXPECT_TRUE(Parser.done()); + EXPECT_FALSE(Recoverable); EXPECT_FALSE(Unrecoverable); }