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 @@ -155,25 +155,36 @@ } // Parse v2-v4 directory and file tables. -static void +static Error parseV2DirFileTables(const DWARFDataExtractor &DebugLineData, uint64_t *OffsetPtr, uint64_t EndPrologueOffset, DWARFDebugLine::ContentTypeTracker &ContentTypes, std::vector &IncludeDirectories, std::vector &FileNames) { + bool Terminated = false; while (*OffsetPtr < EndPrologueOffset) { StringRef S = DebugLineData.getCStrRef(OffsetPtr); - if (S.empty()) + if (S.empty()) { + Terminated = true; break; + } DWARFFormValue Dir = DWARFFormValue::createFromPValue(dwarf::DW_FORM_string, S.data()); IncludeDirectories.push_back(Dir); } + if (!Terminated) + return createStringError(errc::invalid_argument, + "include directories table was not null " + "terminated before the end of the prologue"); + + Terminated = false; while (*OffsetPtr < EndPrologueOffset) { StringRef Name = DebugLineData.getCStrRef(OffsetPtr); - if (Name.empty()) + if (Name.empty()) { + Terminated = true; break; + } DWARFDebugLine::FileNameEntry FileEntry; FileEntry.Name = DWARFFormValue::createFromPValue(dwarf::DW_FORM_string, Name.data()); @@ -185,6 +196,13 @@ ContentTypes.HasModTime = true; ContentTypes.HasLength = true; + + if (!Terminated) + return createStringError(errc::invalid_argument, + "file names table was not null terminated before " + "the end of the prologue"); + + return Error::success(); } // Parse v5 directory/file entry content descriptions. @@ -373,27 +391,30 @@ } } + auto ReportInvalidDirFileTable = [&](Error E) { + RecoverableErrorHandler(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))); + // 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; + }; if (getVersion() >= 5) { if (Error E = parseV5DirFileTables(DebugLineData, OffsetPtr, FormParams, Ctx, U, - ContentTypes, IncludeDirectories, FileNames)) { - RecoverableErrorHandler(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))); - // 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); + ContentTypes, IncludeDirectories, FileNames)) + ReportInvalidDirFileTable(std::move(E)); + } else if (Error E = parseV2DirFileTables(DebugLineData, OffsetPtr, + EndPrologueOffset, ContentTypes, + IncludeDirectories, FileNames)) + ReportInvalidDirFileTable(std::move(E)); if (*OffsetPtr != EndPrologueOffset) { RecoverableErrorHandler(createStringError( 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 @@ -83,8 +83,6 @@ .Lprologue_short_prologue_end: .byte 6 # Read as part of the prologue, # then later again as DW_LNS_negate_stmt. -# FIXME: There should be an additional 0 byte here, but the file name parsing -# code does not recognise a missing null terminator. # Header end .byte 0, 9, 2 # DW_LNE_set_address .quad 0x1122334455667788 @@ -447,6 +445,49 @@ .byte 0, 1, 1 # DW_LNE_end_sequence .Lzero_opcode_base_end: +# V4 table with unterminated include directory table. +.long .Lunterminated_include_end - .Lunterminated_include_start # unit length +.Lunterminated_include_start: +.short 4 # version +.long .Lunterminated_include_prologue_end-.Lunterminated_include_prologue_start # Length of Prologue +.Lunterminated_include_prologue_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 +.Lunterminated_include_prologue_end: +.byte 0, 9, 2 # DW_LNE_set_address +.quad 0xabcdef0123456789 +.byte 0, 1, 1 # DW_LNE_end_sequence +.Lunterminated_include_end: + +# V4 table with unterminated file name table. +.long .Lunterminated_files_end - .Lunterminated_files_start # unit length +.Lunterminated_files_start: +.short 4 # version +.long .Lunterminated_files_prologue_end-.Lunterminated_files_prologue_start # Length of Prologue +.Lunterminated_files_prologue_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 +.byte 0 +.asciz "foo.c" # File table +.byte 1, 2, 3 +.Lunterminated_files_prologue_end: +.byte 0, 9, 2 # DW_LNE_set_address +.quad 0xababcdcdefef0909 +.byte 0, 1, 1 # DW_LNE_end_sequence +.Lunterminated_files_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=0x361 %t-malformed.o 2> %t-malformed-off-last.err \ +# RUN: llvm-dwarfdump -debug-line=0x3c9 %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 @@ -171,7 +171,25 @@ # NONFATAL: 0xffffeeeeddddcccd 1 0 1 0 0 is_stmt{{$}} # NONFATAL: 0xffffeeeeddddcccd 1 0 1 0 0 is_stmt end_sequence{{$}} -# LAST: debug_line[0x00000361] +## V4 table with unterminated include directory table. +# NONFATAL: debug_line[0x00000361] +# NONFATAL-NEXT: Line table prologue +# NONFATAL: include_directories[ 1] = "dir1" +# NONFATAL-NOT: file_names +# NONFATAL: 0xabcdef0123456789 {{.*}} is_stmt end_sequence + +## V4 table with unterminated file name table. +# NONFATAL: debug_line[0x00000390] +# NONFATAL-NEXT: Line table prologue +# NONFATAL: file_names[ 1]: +# NONFATAL-NEXT: name: "foo.c" +# NONFATAL-NEXT: dir_index: 1 +# NONFATAL-NEXT: mod_time: 0x00000002 +# NONFATAL-NEXT: length: 0x00000003 +# NONFATAL-NOT: file_names +# NONFATAL: 0xababcdcdefef0909 {{.*}} is_stmt end_sequence + +# LAST: debug_line[0x000003c9] # LAST: 0x00000000cafebabe {{.*}} end_sequence # RESERVED: warning: parsing line table prologue at offset 0x00000048 unsupported reserved unit length found of value 0xfffffffe @@ -181,6 +199,8 @@ # ALL-NEXT: warning: parsing line table prologue at offset 0x0000004e found unsupported version 1 # 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 +# ALL-NEXT: warning: parsing line table prologue at 0x00000081 found an invalid directory or file table description at 0x000000ba +# ALL-NEXT: warning: file names table was not null terminated before the end of the prologue # 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 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 @@ -197,4 +217,8 @@ # ALL-NEXT: warning: parsing line table prologue at 0x000002ec found an invalid directory or file table description at 0x00000315 # ALL-NEXT: warning: failed to parse directory entry because skipping the form value failed. # ALL-NEXT: warning: parsing line table prologue at offset 0x00000332 found opcode base of 0. Assuming no standard opcodes +# ALL-NEXT: warning: parsing line table prologue at 0x00000361 found an invalid directory or file table description at 0x00000382 +# ALL-NEXT: warning: include directories table was not null terminated before the end of the prologue +# ALL-NEXT: warning: parsing line table prologue at 0x00000390 found an invalid directory or file table description at 0x000003bb +# ALL-NEXT: warning: file names table was not null terminated before the end of the prologue # 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 @@ -449,12 +449,7 @@ LineTable < = Gen->addLineTable(Format); DWARFDebugLine::Prologue Prologue = LT.createBasicPrologue(); - // FIXME: Ideally, we'd test for 1 less than expected, but the code does not - // currently fail if missing only the terminator of a v2-4 file table. - if (Version < 5) - Prologue.PrologueLength -= 2; - else - Prologue.PrologueLength -= 1; + Prologue.PrologueLength -= 2; LT.setPrologue(Prologue); generate(); @@ -465,23 +460,34 @@ 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; + Result.Prologue.PrologueLength += 2; checkDefaultPrologue(Version, Format, Result.Prologue, 0); uint64_t ExpectedEnd = - Prologue.TotalLength - 1 + Prologue.sizeofTotalLength(); - if (Version < 5) - --ExpectedEnd; - checkError( + Prologue.TotalLength - 2 + Prologue.sizeofTotalLength(); + std::vector Errs; + // Parsing of a DWARFv2-4 file table stops at the end of an entry once the + // prologue end has been reached, whether or not the trailing null terminator + // has been found. As such, the expected error message will be slightly + // different. + uint64_t ActualEnd = Version == 5 ? ExpectedEnd + 2 : ExpectedEnd + 1; + if (Version != 5) { + Errs.emplace_back( + (Twine("parsing line table prologue at 0x00000000 found an invalid " + "directory or file table description at 0x000000") + + Twine::utohexstr(ActualEnd)) + .str()); + Errs.emplace_back("file names table was not null terminated before the end " + "of the prologue"); + } + Errs.emplace_back( (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(), - std::move(Recoverable)); + Twine::utohexstr(ActualEnd)) + .str()); + std::vector ErrRefs(Errs.begin(), Errs.end()); + checkError(ErrRefs, std::move(Recoverable)); } INSTANTIATE_TEST_CASE_P(