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 @@ -170,8 +170,11 @@ std::vector &IncludeDirectories, std::vector &FileNames) { bool Terminated = false; - while (*OffsetPtr < EndPrologueOffset) { - StringRef S = DebugLineData.getCStrRef(OffsetPtr); + Error Err = Error::success(); + while (true) { + StringRef S = DebugLineData.getCStrRef(OffsetPtr, &Err); + if (Err || *OffsetPtr > EndPrologueOffset) + break; if (S.empty()) { Terminated = true; break; @@ -181,34 +184,44 @@ IncludeDirectories.push_back(Dir); } - if (!Terminated) + if (!Terminated) { + consumeError(std::move(Err)); 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); + while (true) { + StringRef Name = DebugLineData.getCStrRef(OffsetPtr, &Err); + if (Err || *OffsetPtr > EndPrologueOffset) + break; if (Name.empty()) { Terminated = true; break; } + DWARFDebugLine::FileNameEntry FileEntry; FileEntry.Name = DWARFFormValue::createFromPValue(dwarf::DW_FORM_string, Name.data()); - FileEntry.DirIdx = DebugLineData.getULEB128(OffsetPtr); - FileEntry.ModTime = DebugLineData.getULEB128(OffsetPtr); - FileEntry.Length = DebugLineData.getULEB128(OffsetPtr); + FileEntry.DirIdx = DebugLineData.getULEB128(OffsetPtr, &Err); + FileEntry.ModTime = DebugLineData.getULEB128(OffsetPtr, &Err); + FileEntry.Length = DebugLineData.getULEB128(OffsetPtr, &Err); + + if (Err || *OffsetPtr > EndPrologueOffset) + break; FileNames.push_back(FileEntry); } ContentTypes.HasModTime = true; ContentTypes.HasLength = true; - if (!Terminated) + if (!Terminated) { + consumeError(std::move(Err)); return createStringError(errc::invalid_argument, "file names table was not null terminated before " "the end of the prologue"); + } return Error::success(); } @@ -411,11 +424,9 @@ " 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; + // Reset the offset to the end of the prologue to prevent the length check + // below from executing. + *OffsetPtr = EndPrologueOffset; }; if (getVersion() >= 5) { if (Error E = 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 @@ -77,11 +77,11 @@ ## Prologue with length shorter than parsed. # NONFATAL: debug_line[0x00000081] # NONFATAL-NEXT: Line table prologue -# NONFATAL: file_names[ 2]: -# NONFATAL-NEXT: name: "file2" -# NONFATAL-NEXT: dir_index: 1 -# NONFATAL-NEXT: mod_time: 0x00000002 -# NONFATAL-NEXT: length: 0x00000006 +# NONFATAL: file_names[ 1]: +# NONFATAL-NEXT: name: "file1" +# NONFATAL-NEXT: dir_index: 0 +# NONFATAL-NEXT: mod_time: 0x00000000 +# NONFATAL-NEXT: length: 0x00000000 # NONFATAL: 0x1122334455667788 {{.*}} 0 end_sequence{{$}} ## Prologue with length longer than parsed. @@ -203,7 +203,6 @@ # 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 # OTHER-NEXT: warning: unexpected line op length at offset 0x0000015c expected 0x01 found 0x02 @@ -215,12 +214,11 @@ # ALL-NEXT: warning: failed to parse file entry because the MD5 hash is invalid # 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 # 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: parsing line table prologue at 0x00000361 found an invalid directory or file table description at 0x00000383 # 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: parsing line table prologue at 0x00000390 found an invalid directory or file table description at 0x000003bc # 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 @@ -448,34 +448,39 @@ 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 += 2; - checkDefaultPrologue(Version, Format, Result.Prologue, 0); + + if (Version != 5) { + // Parsing will stop before reading a complete file entry. + ASSERT_EQ(Result.Prologue.IncludeDirectories.size(), 1u); + EXPECT_EQ(toStringRef(Result.Prologue.IncludeDirectories[0]), "a dir"); + EXPECT_EQ(Result.Prologue.FileNames.size(), 0u); + } else { + // Parsing will continue past the presumed end of prologue. + ASSERT_EQ(Result.Prologue.FileNames.size(), 1u); + ASSERT_EQ(Result.Prologue.FileNames[0].Name.getForm(), DW_FORM_string); + ASSERT_EQ(Result.Prologue.FileNames[0].DirIdx, 0u); + EXPECT_EQ(toStringRef(Result.Prologue.FileNames[0].Name), "a file"); + } uint64_t ExpectedEnd = 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)) + Twine::utohexstr(ExpectedEnd + 1)) .str()); Errs.emplace_back("file names table was not null terminated before the end " "of the prologue"); + } else { + 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 + 2)) + .str()); } - 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(ActualEnd)) - .str()); EXPECT_THAT_ERROR(std::move(Recoverable), FailedWithMessageArray(testing::ElementsAreArray(Errs))); }