[llvm] bf4d8f2 - [DebugInfo] Add checks for v2 directory and file name table terminators
James Henderson via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 12 06:49:48 PST 2020
Author: James Henderson
Date: 2020-02-12T14:49:22Z
New Revision: bf4d8f29524449c4114b457875cc6a8031c46092
URL: https://github.com/llvm/llvm-project/commit/bf4d8f29524449c4114b457875cc6a8031c46092
DIFF: https://github.com/llvm/llvm-project/commit/bf4d8f29524449c4114b457875cc6a8031c46092.diff
LOG: [DebugInfo] Add checks for v2 directory and file name table terminators
The DWARFv2-4 specification for the line table header states that the
include directories and file name tables both end with a single null
byte. Prior to this change, the parser did not detect if this byte was
missing, because it also stopped reading the tables once it reached the
prologue end, as claimed by the header_length field. This change adds a
check that the terminator has been seen at the end of each table.
Reviewed by: dblaikie, MaskRay
Differential Revision: https://reviews.llvm.org/D74413
Added:
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 2c869dfdd243..7bdc72ae7aa8 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
@@ -155,25 +155,36 @@ void DWARFDebugLine::Prologue::dump(raw_ostream &OS,
}
// 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<DWARFFormValue> &IncludeDirectories,
std::vector<DWARFDebugLine::FileNameEntry> &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 @@ parseV2DirFileTables(const DWARFDataExtractor &DebugLineData,
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 @@ Error DWARFDebugLine::Prologue::parse(
}
}
+ 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
index 0477a668d5a7..090ebe6203cd 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
@@ -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
index d4b504592cfd..f93035a4a11c 100644
--- 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
index 795b9b4436ed..d6d96916c4e9 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
@@ -449,12 +449,7 @@ TEST_P(DebugLineParameterisedFixture, ErrorForTooShortPrologueLength) {
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 @@ TEST_P(DebugLineParameterisedFixture, ErrorForTooShortPrologueLength) {
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<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))
+ .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<StringRef> ErrRefs(Errs.begin(), Errs.end());
+ checkError(ErrRefs, std::move(Recoverable));
}
INSTANTIATE_TEST_CASE_P(
More information about the llvm-commits
mailing list