[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
This commit is contained in:
James Henderson 2020-02-12 14:37:10 +00:00
parent 9d0fa600e4
commit 28d5339ea7
4 changed files with 134 additions and 42 deletions

View File

@ -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(

View File

@ -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:

View File

@ -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:

View File

@ -449,12 +449,7 @@ TEST_P(DebugLineParameterisedFixture, ErrorForTooShortPrologueLength) {
LineTable &LT = 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
// 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<StringRef> ErrRefs(Errs.begin(), Errs.end());
checkError(ErrRefs, std::move(Recoverable));
}
INSTANTIATE_TEST_CASE_P(