Skip to content

Commit

Permalink
Revert "[DebugInfo] Make most debug line prologue errors non-fatal to…
Browse files Browse the repository at this point in the history 
… parsing"

This reverts commit b94191f.

The change broke both an LLD test and the LLDB build.
  • Loading branch information
@jh7370
jh7370 committed Jan 28, 2020
1 parent b94191f commit 5c05165
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 177 deletions.
10 changes: 3 additions & 7 deletions llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h
View file
Expand Up @@ -138,7 +138,6 @@ class DWARFDebugLine {
void clear();
void dump(raw_ostream &OS, DIDumpOptions DumpOptions) const;
Error parse(const DWARFDataExtractor &DebugLineData, uint64_t *OffsetPtr,
function_ref<void(Error)> RecoverableErrorCallback,
const DWARFContext &Ctx, const DWARFUnit *U = nullptr);
};

Expand Down Expand Up @@ -342,12 +341,9 @@ class DWARFDebugLine {
/// Skip the current line table and go to the following line table (if
/// present) immediately.
///
/// \param RecoverableErrorCallback - report any recoverable prologue
/// parsing issues via this callback.
/// \param UnrecoverableErrorCallback - report any unrecoverable prologue
/// parsing issues via this callback.
void skip(function_ref<void(Error)> RecoverableErrorCallback,
function_ref<void(Error)> UnrecoverableErrorCallback);
/// \param ErrorCallback - report any prologue parsing issues via this
/// callback.
void skip(function_ref<void(Error)> ErrorCallback);

/// Indicates if the parser has parsed as much as possible.
///
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
View file
Expand Up @@ -467,7 +467,7 @@ void DWARFContext::dump(
Optional<uint64_t> DumpOffset) {
while (!Parser.done()) {
if (DumpOffset && Parser.getOffset() != *DumpOffset) {
Parser.skip(dumpWarning, dumpWarning);
Parser.skip(dumpWarning);
continue;
}
OS << "debug_line[" << format("0x%8.8" PRIx64, Parser.getOffset())
Expand Down
47 changes: 16 additions & 31 deletions llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
View file
Expand Up @@ -299,10 +299,10 @@ parseV5DirFileTables(const DWARFDataExtractor &DebugLineData,
return Error::success();
}

Error DWARFDebugLine::Prologue::parse(
const DWARFDataExtractor &DebugLineData, uint64_t *OffsetPtr,
function_ref<void(Error)> RecoverableErrorCallback, const DWARFContext &Ctx,
const DWARFUnit *U) {
Error DWARFDebugLine::Prologue::parse(const DWARFDataExtractor &DebugLineData,
uint64_t *OffsetPtr,
const DWARFContext &Ctx,
const DWARFUnit *U) {
const uint64_t PrologueOffset = *OffsetPtr;

clear();
Expand All @@ -311,18 +311,13 @@ Error DWARFDebugLine::Prologue::parse(
FormParams.Format = dwarf::DWARF64;
TotalLength = DebugLineData.getU64(OffsetPtr);
} else if (TotalLength >= dwarf::DW_LENGTH_lo_reserved) {
// Treat this error as unrecoverable - we have no way of knowing where the
// table ends.
return createStringError(errc::invalid_argument,
"parsing line table prologue at offset 0x%8.8" PRIx64
" unsupported reserved unit length found of value 0x%8.8" PRIx64,
PrologueOffset, TotalLength);
}
FormParams.Version = DebugLineData.getU16(OffsetPtr);
if (getVersion() < 2)
// Treat this error as unrecoverable - we cannot be sure what any of
// the data represents including the length field, so cannot skip it or make
// any reasonable assumptions.
return createStringError(errc::not_supported,
"parsing line table prologue at offset 0x%8.8" PRIx64
" found unsupported version 0x%2.2" PRIx16,
Expand Down Expand Up @@ -357,32 +352,25 @@ Error DWARFDebugLine::Prologue::parse(
if (Error E =
parseV5DirFileTables(DebugLineData, OffsetPtr, FormParams, Ctx, U,
ContentTypes, IncludeDirectories, FileNames)) {
RecoverableErrorCallback(joinErrors(
return 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;
std::move(E));
}
} else
parseV2DirFileTables(DebugLineData, OffsetPtr, EndPrologueOffset,
ContentTypes, IncludeDirectories, FileNames);

if (*OffsetPtr != EndPrologueOffset) {
RecoverableErrorCallback(createStringError(
errc::invalid_argument,
"parsing line table prologue at 0x%8.8" PRIx64
" should have ended at 0x%8.8" PRIx64 " but it ended at 0x%8.8" PRIx64,
PrologueOffset, EndPrologueOffset, *OffsetPtr));
*OffsetPtr = EndPrologueOffset;
}
if (*OffsetPtr != EndPrologueOffset)
return createStringError(errc::invalid_argument,
"parsing line table prologue at 0x%8.8" PRIx64
" should have ended at 0x%8.8" PRIx64
" but it ended at 0x%8.8" PRIx64,
PrologueOffset, EndPrologueOffset, *OffsetPtr);
return Error::success();
}

Expand Down Expand Up @@ -528,8 +516,7 @@ Error DWARFDebugLine::LineTable::parse(

clear();

Error PrologueErr = Prologue.parse(DebugLineData, OffsetPtr,
RecoverableErrorCallback, Ctx, U);
Error PrologueErr = Prologue.parse(DebugLineData, OffsetPtr, Ctx, U);

if (OS) {
// The presence of OS signals verbose dumping.
Expand Down Expand Up @@ -1171,16 +1158,14 @@ DWARFDebugLine::LineTable DWARFDebugLine::SectionParser::parseNext(
}

void DWARFDebugLine::SectionParser::skip(
function_ref<void(Error)> RecoverableErrorCallback,
function_ref<void(Error)> UnrecoverableErrorCallback) {
function_ref<void(Error)> ErrorCallback) {
assert(DebugLineData.isValidOffset(Offset) &&
"parsing should have terminated");
DWARFUnit *U = prepareToParse(Offset);
uint64_t OldOffset = Offset;
LineTable LT;
if (Error Err = LT.Prologue.parse(DebugLineData, &Offset,
RecoverableErrorCallback, Context, U))
UnrecoverableErrorCallback(std::move(Err));
if (Error Err = LT.Prologue.parse(DebugLineData, &Offset, Context, U))
ErrorCallback(std::move(Err));
moveToNextTable(OldOffset, LT.Prologue);
}

Expand Down
113 changes: 51 additions & 62 deletions llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s
View file
Expand Up @@ -64,7 +64,7 @@
.long .Lunit_short_prologue_end - .Lunit_short_prologue_start # unit length
.Lunit_short_prologue_start:
.short 4 # version
.long .Lprologue_short_prologue_end-.Lprologue_short_prologue_start # Length of Prologue
.long .Lprologue_short_prologue_end-.Lprologue_short_prologue_start - 2 # Length of Prologue
.Lprologue_short_prologue_start:
.byte 1 # Minimum Instruction Length
.byte 1 # Maximum Operations per Instruction
Expand All @@ -79,13 +79,9 @@
.asciz "file1" # File table
.byte 0, 0, 0
.asciz "file2"
.byte 1, 2
.byte 1, 2, 3
.byte 0
.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
.byte 0, 1, 1 # DW_LNE_end_sequence
Expand All @@ -95,7 +91,7 @@
.long .Lunit_long_prologue_end - .Lunit_long_prologue_start # unit length
.Lunit_long_prologue_start:
.short 4 # version
.long .Lprologue_long_prologue_end-.Lprologue_long_prologue_start # Length of Prologue
.long .Lprologue_long_prologue_end-.Lprologue_long_prologue_start + 1 # Length of Prologue
.Lprologue_long_prologue_start:
.byte 1 # Minimum Instruction Length
.byte 1 # Maximum Operations per Instruction
Expand All @@ -112,8 +108,6 @@
.asciz "file2"
.byte 1, 2, 3
.byte 0
# Skipped byte (treated as part of prologue).
.byte 6
.Lprologue_long_prologue_end:
.byte 0, 9, 2 # DW_LNE_set_address
.quad 0x1111222233334444
Expand Down Expand Up @@ -186,35 +180,34 @@
.short 5 # DWARF version number
.byte 8 # Address Size
.byte 0 # Segment Selector Size
.long .Linvalid_description_header_end0 - .Linvalid_description_params0 # Length of Prologue (invalid)
.long 15 # Length of Prologue (invalid)
.Linvalid_description_params0:
.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 # Standard Opcode Lengths
.Linvalid_description_header_end0:
# The bytes from here onwards will also be read as part of the main body.
# --- Prologue interpretation --- | --- Main body interpretation ---
.byte 0, 1 # More standard opcodes | First part of DW_LNE_end_sequence
.byte 0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 # Standard Opcode Lengths
# Directory table format
.byte 1 # One element per directory entry | End of DW_LNE_end_sequence
.byte 1 # DW_LNCT_path | DW_LNS_copy
.byte 0x08 # DW_FORM_string | DW_LNS_const_add_pc
.byte 1 # One element per directory entry
.byte 1 # DW_LNCT_path
.byte 0x08 # DW_FORM_string
# Directory table entries
.byte 1 # 1 directory | DW_LNS_copy
.asciz "/tmp" # Directory name | four special opcodes + start of DW_LNE_end_sequence
.byte 1 # 1 directory
.asciz "/tmp"
# File table format
.byte 1 # 1 element per file entry | DW_LNE_end_sequence length
.byte 1 # DW_LNCT_path | DW_LNE_end_sequence opcode
.byte 0x08 # DW_FORM_string | DW_LNS_const_add_pc
.byte 2 # 2 elements per file entry
.byte 1 # DW_LNCT_path
.byte 0x08 # DW_FORM_string
.byte 2 # DW_LNCT_directory_index
.byte 0x0b # DW_FORM_data1
# File table entries
.byte 1 # 1 file | DW_LNS_copy
.asciz "xyz" # File name | three special opcodes + start of DW_LNE_set_address
# Header end
.byte 9, 2 # Remainder of DW_LNE_set_address
.byte 1 # 1 file
.asciz "a.c"
.byte 1
.Linvalid_description_header_end0:
.byte 0, 9, 2 # DW_LNE_set_address
.quad 0xbabb1ebabb1e
.byte 0, 1, 1 # DW_LNE_end_sequence
.Linvalid_description_end0:
Expand All @@ -225,7 +218,7 @@
.short 5 # DWARF version number
.byte 8 # Address Size
.byte 0 # Segment Selector Size
.long .Linvalid_file_header_end0 - .Linvalid_file_params0 # Length of Prologue (invalid)
.long .Linvalid_file_header_end0-.Linvalid_file_params0-7 # Length of Prologue (invalid)
.Linvalid_file_params0:
.byte 1 # Minimum Instruction Length
.byte 1 # Maximum Operations per Instruction
Expand All @@ -246,16 +239,12 @@
.byte 1 # DW_LNCT_path
.byte 0x08 # DW_FORM_string
.byte 2 # DW_LNCT_directory_index
.Linvalid_file_header_end0:
# The bytes from here onwards will also be read as part of the main body.
# --- Prologue interpretation --- | --- Main body interpretation ---
.byte 0x0b # DW_FORM_data1 | DW_LNS_set_epilogue_begin
.byte 0x0b # DW_FORM_data1
# File table entries
.byte 1 # 1 file | DW_LNS_copy
.asciz "xyz" # File name | 3 special opcodes + start of DW_LNE_end_sequence
.byte 1 # Dir index | DW_LNE_end_sequence length
# Header end
.byte 1 # DW_LNE_end_sequence opcode
.byte 1 # 1 file
.asciz "a.c"
.byte 1
.Linvalid_file_header_end0:
.byte 0, 9, 2 # DW_LNE_set_address
.quad 0xab4acadab4a
.byte 0, 1, 1 # DW_LNE_end_sequence
Expand All @@ -267,7 +256,7 @@
.short 5 # DWARF version number
.byte 8 # Address Size
.byte 0 # Segment Selector Size
.long .Linvalid_dir_header_end0 - .Linvalid_dir_params0 # Length of Prologue (invalid)
.long .Linvalid_dir_header_end0-.Linvalid_dir_params0-16 # Length of Prologue (invalid)
.Linvalid_dir_params0:
.byte 1 # Minimum Instruction Length
.byte 1 # Maximum Operations per Instruction
Expand All @@ -282,19 +271,19 @@
.byte 0x08 # DW_FORM_string
# Directory table entries
.byte 1 # 1 directory
.Linvalid_dir_header_end0:
# The bytes from here onwards will also be read as part of the main body.
# --- Prologue interpretation --- | --- Main body interpretation ---
.asciz "/tmp" # Directory name | 4 special opcodes + start of DW_LNE_end_sequence
.asciz "/tmp"
# File table format
.byte 1 # 1 element per file entry | DW_LNE_end_sequence length
.byte 1 # DW_LNCT_path | DW_LNE_end_sequence length opcode
.byte 0x08 # DW_FORM_string | DW_LNS_const_add_pc
.byte 2 # 2 elements per file entry
.byte 1 # DW_LNCT_path
.byte 0x08 # DW_FORM_string
.byte 2 # DW_LNCT_directory_index
.byte 0x0b # DW_FORM_data1
# File table entries
.byte 1 # 1 file | DW_LNS_copy
.asciz "xyz" # File name | start of DW_LNE_set_address
# Header end
.byte 9, 2 # DW_LNE_set_address length + opcode
.byte 1 # 1 file
.asciz "a.c"
.byte 1
.Linvalid_dir_header_end0:
.byte 0, 9, 2 # DW_LNE_set_address
.quad 0x4444333322221111
.byte 0, 1, 1 # DW_LNE_end_sequence
.Linvalid_dir_end0:
Expand Down Expand Up @@ -334,7 +323,7 @@
.asciz "a.c"
.byte 0
# Data to show that the rest of the prologue is skipped.
.byte 1
.byte 6
.Linvalid_md5_header_end0:
.byte 0, 9, 2 # DW_LNE_set_address
.quad 0x1234123412341234
Expand All @@ -348,7 +337,7 @@
.short 5 # DWARF version number
.byte 8 # Address Size
.byte 0 # Segment Selector Size
.long .Linvalid_md5_header_end1 - .Linvalid_md5_params1 # Length of Prologue
.long .Linvalid_md5_header_end1-.Linvalid_md5_params1 - 10 # Length of Prologue
.Linvalid_md5_params1:
.byte 1 # Minimum Instruction Length
.byte 1 # Maximum Operations per Instruction
Expand All @@ -365,20 +354,20 @@
.byte 1 # 1 directory
.asciz "/tmp"
# File table format
.byte 2 # 2 elements per file entry
.byte 3 # 2 elements per file entry
.byte 1 # DW_LNCT_path
.byte 0x08 # DW_FORM_string
.byte 5 # DW_LNCT_MD5
.Linvalid_md5_header_end1:
# The bytes from here onwards will also be read as part of the main body.
# --- Prologue interpretation --- | --- Main body interpretation ---
.byte 0x0b # DW_FORM_data1 | DW_LNS_set_epilogue_begin
.byte 0x0b # DW_FORM_data1
.byte 2 # DW_LNCT_directory_index
.byte 0x0b # DW_FORM_data1
# File table entries
.byte 1 # 1 file | DW_LNS_copy
.asciz "xyz" # File name | 3 special opcodes + DW_LNE_set_address start
.byte 9 # MD5 hash value | DW_LNE_set_address length
# Header end
.byte 2 # DW_LNE_set_address opcode
.byte 1 # 1 file
.asciz "a.c"
.byte 6 # This byte will be consumed when reading the MD5 value.
.byte 0xb # This byte will not be read as part of the prologue.
.Linvalid_md5_header_end1:
.byte 0, 9, 2 # DW_LNE_set_address
.quad 0x4321432143214321
.byte 0, 1, 1 # DW_LNE_end_sequence
.Linvalid_md5_end1:
Expand Down

0 comments on commit 5c05165

Please sign in to comment.