[llvm] c475856 - [DWARFDebugLine] Check for errors when parsing v2 file/dir lists
Pavel Labath via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 21 07:55:47 PDT 2020
Author: Pavel Labath
Date: 2020-04-21T16:55:36+02:00
New Revision: c475856d0532995a74206e5f4bd5ebf80ce85750
URL: https://github.com/llvm/llvm-project/commit/c475856d0532995a74206e5f4bd5ebf80ce85750
DIFF: https://github.com/llvm/llvm-project/commit/c475856d0532995a74206e5f4bd5ebf80ce85750.diff
LOG: [DWARFDebugLine] Check for errors when parsing v2 file/dir lists
Summary:
Without this we could silently accept an invalid prologue because the
default DataExtractor behavior is to return an empty string when
reaching the end of file. And empty string is also used to terminate
these lists.
This makes the parsing code slightly more complicated, but this
complexity will go away once the parser starts working with truncating
data extractors. The reason I am doing it this way is because without
this, the truncation would regress the quality of error messages (right
now, we produce bad error messages only near EOF, but truncation would
make everything behave as if it was near EOF).
Reviewers: dblaikie, probinson, jhenderson
Subscribers: hiraditya, MaskRay, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D77555
Added:
llvm/test/tools/llvm-dwarfdump/X86/debug_line_short_prologue_v4.s
Modified:
llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s
llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test
llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
index bd1954a73e1d..6fec697941b5 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
@@ -169,47 +169,48 @@ parseV2DirFileTables(const DWARFDataExtractor &DebugLineData,
DWARFDebugLine::ContentTypeTracker &ContentTypes,
std::vector<DWARFFormValue> &IncludeDirectories,
std::vector<DWARFDebugLine::FileNameEntry> &FileNames) {
- bool Terminated = false;
- while (*OffsetPtr < EndPrologueOffset) {
- StringRef S = DebugLineData.getCStrRef(OffsetPtr);
- if (S.empty()) {
- Terminated = true;
- break;
+ while (true) {
+ Error Err = Error::success();
+ StringRef S = DebugLineData.getCStrRef(OffsetPtr, &Err);
+ if (Err || *OffsetPtr > EndPrologueOffset) {
+ consumeError(std::move(Err));
+ return createStringError(errc::invalid_argument,
+ "include directories table was not null "
+ "terminated before the end of the prologue");
}
+ if (S.empty())
+ 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()) {
- Terminated = true;
+ ContentTypes.HasModTime = true;
+ ContentTypes.HasLength = true;
+
+ while (true) {
+ Error Err = Error::success();
+ StringRef Name = DebugLineData.getCStrRef(OffsetPtr, &Err);
+ if (!Err && *OffsetPtr <= EndPrologueOffset && Name.empty())
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) {
+ consumeError(std::move(Err));
+ return createStringError(
+ errc::invalid_argument,
+ "file names table was not null terminated before "
+ "the end of the prologue");
+ }
FileNames.push_back(FileEntry);
}
- 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();
}
@@ -402,7 +403,13 @@ Error DWARFDebugLine::Prologue::parse(
}
}
- auto ReportInvalidDirFileTable = [&](Error E) {
+ Error E =
+ getVersion() >= 5
+ ? parseV5DirFileTables(DebugLineData, OffsetPtr, FormParams, Ctx, U,
+ ContentTypes, IncludeDirectories, FileNames)
+ : parseV2DirFileTables(DebugLineData, OffsetPtr, EndPrologueOffset,
+ ContentTypes, IncludeDirectories, FileNames);
+ if (E) {
RecoverableErrorHandler(joinErrors(
createStringError(
errc::invalid_argument,
@@ -411,21 +418,8 @@ Error DWARFDebugLine::Prologue::parse(
" 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))
- ReportInvalidDirFileTable(std::move(E));
- } else if (Error E = parseV2DirFileTables(DebugLineData, OffsetPtr,
- EndPrologueOffset, ContentTypes,
- IncludeDirectories, FileNames))
- ReportInvalidDirFileTable(std::move(E));
+ return Error::success();
+ }
if (*OffsetPtr != EndPrologueOffset) {
RecoverableErrorHandler(createStringError(
@@ -735,6 +729,7 @@ Error DWARFDebugLine::LineTable::parse(
ParsingState State(this, DebugLineOffset, RecoverableErrorHandler);
+ *OffsetPtr = DebugLineOffset + Prologue.getLength();
while (*OffsetPtr < EndOffset) {
if (OS)
*OS << format("0x%08.08" PRIx64 ": ", *OffsetPtr);
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
index 090ebe6203cd..d8809d11daab 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s
@@ -77,7 +77,7 @@
.asciz "dir2"
.byte 0
.asciz "file1" # File table
-.byte 0, 0, 0
+.byte 1, 2, 3
.asciz "file2"
.byte 1, 2
.Lprologue_short_prologue_end:
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test b/llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test
index 797eb2fb4c8c..7fa13fc9b9ad 100644
--- 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: file_names[ 1]:
+# NONFATAL-NEXT: name: "file1"
# NONFATAL-NEXT: dir_index: 1
# NONFATAL-NEXT: mod_time: 0x00000002
-# NONFATAL-NEXT: length: 0x00000006
+# NONFATAL-NEXT: length: 0x00000003
# 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 0x000003bf
# ALL-NEXT: warning: file names table was not null terminated before the end of the prologue
# ALL-NOT: warning:
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug_line_short_prologue_v4.s b/llvm/test/tools/llvm-dwarfdump/X86/debug_line_short_prologue_v4.s
new file mode 100644
index 000000000000..d5a28cbcb74e
--- /dev/null
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug_line_short_prologue_v4.s
@@ -0,0 +1,78 @@
+## Test cases when we run into the end of section while parsing a line table
+## prologue.
+
+# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj --defsym CASE=0 -o %t0
+# RUN: llvm-dwarfdump -debug-line %t0 2>&1 | FileCheck %s --check-prefixes=ALL,C0
+
+# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj --defsym CASE=1 -o %t1
+# RUN: llvm-dwarfdump -debug-line %t1 2>&1 | FileCheck %s --check-prefixes=ALL,C1
+
+# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj --defsym CASE=2 -o %t2
+# RUN: llvm-dwarfdump -debug-line %t2 2>&1 | FileCheck %s --check-prefixes=ALL,C1
+
+# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj --defsym CASE=3 -o %t3
+# RUN: llvm-dwarfdump -debug-line %t3 2>&1 | FileCheck %s --check-prefixes=ALL,C1
+
+# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj --defsym CASE=4 -o %t4
+# RUN: llvm-dwarfdump -debug-line %t4 2>&1 | FileCheck %s --check-prefixes=ALL,C1
+
+# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj --defsym CASE=5 -o %t5
+# RUN: llvm-dwarfdump -debug-line %t5 2>&1 | FileCheck %s --check-prefixes=ALL,OK
+
+# ALL: debug_line[0x00000000]
+
+# C0-NEXT: warning: parsing line table prologue at 0x00000000 found an invalid directory or file table description at 0x00000021
+# C0-NEXT: warning: include directories table was not null terminated before the end of the prologue
+# C0: include_directories[ 1] = "dir1"
+
+# C1-NEXT: warning: parsing line table prologue at 0x00000000 found an invalid directory or file table description
+# C1-NEXT: warning: file names table was not null terminated before the end of the prologue
+# C1: include_directories[ 2] = "dir2"
+# C1-NEXT: file_names[ 1]:
+# C1-NEXT: name: "file1"
+# C1-NEXT: dir_index: 1
+# C1-NEXT: mod_time: 0x00000002
+# C1-NEXT: length: 0x00000003
+
+# OK: file_names[ 2]:
+# OK-NEXT: name: "file2"
+# OK-NEXT: dir_index: 1
+# OK-NEXT: mod_time: 0x00000005
+# OK-NEXT: length: 0x00000006
+
+.section .debug_line,"", at progbits
+.long .Lend-.Lstart # Length of Unit
+.Lstart:
+.short 4 # DWARF version number
+.long .Lprologue_end-.Lprologue_start # Length of Prologue
+.Lprologue_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
+.if CASE >= 1
+.asciz "dir2"
+.byte 0
+.asciz "file1" # File table
+.byte 1, 2, 3
+.if CASE >= 2
+.asciz "file2"
+.if CASE >= 3
+.byte 1
+.if CASE >= 4
+.byte 5
+.if CASE >= 5
+.byte 6
+.byte 0
+.endif
+.endif
+.endif
+.endif
+.endif
+
+.Lprologue_end:
+.Lend:
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
index cf2fb5375c5c..be61c03b779c 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
@@ -448,34 +448,39 @@ TEST_P(DebugLineParameterisedFixture, ErrorForTooShortPrologueLength) {
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<std::string> 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
- //
diff erent.
- 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)));
}
More information about the llvm-commits
mailing list