[llvm] ced4597 - Recommit "[DWARFDebugLine] Avoid dumping prologue members we did not parse"
Pavel Labath via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 26 07:42:46 PST 2020
Author: Pavel Labath
Date: 2020-02-26T16:42:25+01:00
New Revision: ced45978a2a5b258421590214f46848e726144b8
URL: https://github.com/llvm/llvm-project/commit/ced45978a2a5b258421590214f46848e726144b8
DIFF: https://github.com/llvm/llvm-project/commit/ced45978a2a5b258421590214f46848e726144b8.diff
LOG: Recommit "[DWARFDebugLine] Avoid dumping prologue members we did not parse"
The patch was reverted in 69da40033 because of test failures on windows.
The problem was the unpredictable order of some of the error messages,
which I've tried to strenghten in that patch.
It turns out this is not possible to do in verbose mode because there
the data is being writted as it is being parsed. No amount of flushing
(as I've done in the non-verbose mode) will help that. Indeed, even
without any buffering the warning messages can end in the middle of a
line in non-verbose mode.
In this patch, I have reverted the changes which tested the relative
position of the warning message, except for the messages about
unsupported initial length, which are the ones I really wanted to test,
and which do come out reasonably.
The original commit message was:
This patch if motivated by D74560, specifically the subthread about what
to print upon encountering reserved initial length values.
If the debug_line prologue has an unsupported version, we skip parsing
the rest of the data. If we encounter an reserved initial length field,
we don't even parse the version. However, we still print out all members
(with value 0) in the dump function.
This patch introduces early exits in the Prologue::dump function so that
we print only the fields that were parsed successfully. In case of an
unsupported version, we skip printing all subsequent prologue fields --
because we don't even know if this version has those fields. In case of a
reserved unit length, we don't print anything -- if the very first field
of the prologue is invalid, it's hard to say if we even have a prologue
to begin with.
Note that the user will still be able to see the invalid/reserved
initial length value in the error message. I've modified (reordered)
debug_line_invalid.test to show that the error message comes straight
after the debug_line offset. I've also added some flush() calls to the
dumping code to ensure this is the case in all situations (without that,
the warnings could get out of sync if the output was not a terminal -- I
guess this is why std::iostreams have the tie() function).
Reviewers: jhenderson, ikudrin, dblaikie
Subscribers: hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D75043
Added:
Modified:
llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test
Removed:
################################################################################
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index 73cfd16be9b3..e46c7eb150af 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -474,6 +474,7 @@ void DWARFContext::dump(
}
OS << "debug_line[" << format("0x%8.8" PRIx64, Parser.getOffset())
<< "]\n";
+ OS.flush();
if (DumpOpts.Verbose) {
Parser.parseNext(DumpOpts.WarningHandler, DumpOpts.WarningHandler, &OS);
} else {
@@ -481,6 +482,7 @@ void DWARFContext::dump(
Parser.parseNext(DumpOpts.WarningHandler, DumpOpts.WarningHandler);
LineTable.dump(OS, DumpOpts);
}
+ OS.flush();
}
};
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
index 436318ba8b16..d4a6bebd5c8a 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
@@ -42,6 +42,10 @@ using ContentDescriptors = SmallVector<ContentDescriptor, 4>;
} // end anonymous namespace
+static bool versionIsSupported(uint16_t Version) {
+ return Version >= 2 && Version <= 5;
+}
+
void DWARFDebugLine::ContentTypeTracker::trackContentType(
dwarf::LineNumberEntryFormat ContentType) {
switch (ContentType) {
@@ -100,9 +104,13 @@ void DWARFDebugLine::Prologue::clear() {
void DWARFDebugLine::Prologue::dump(raw_ostream &OS,
DIDumpOptions DumpOptions) const {
+ if (!totalLengthIsValid())
+ return;
OS << "Line table prologue:\n"
<< format(" total_length: 0x%8.8" PRIx64 "\n", TotalLength)
<< format(" version: %u\n", getVersion());
+ if (!versionIsSupported(getVersion()))
+ return;
if (getVersion() >= 5)
OS << format(" address_size: %u\n", getAddressSize())
<< format(" seg_select_size: %u\n", SegSelectorSize);
@@ -345,7 +353,7 @@ Error DWARFDebugLine::Prologue::parse(
PrologueOffset, TotalLength);
}
FormParams.Version = DebugLineData.getU16(OffsetPtr);
- if (getVersion() < 2 || getVersion() > 5)
+ if (!versionIsSupported(getVersion()))
// 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.
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 f93035a4a11c..871d6e9e78ef 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test
@@ -3,23 +3,19 @@
## Show that a bad length stops parsing of the section.
# RUN: llvm-mc -triple x86_64-pc-linux %S/Inputs/debug_line_reserved_length.s -filetype=obj -o %t-reserved.o
-# RUN: llvm-dwarfdump -debug-line %t-reserved.o 2> %t-reserved.err \
-# RUN: | FileCheck %s --check-prefixes=FIRST,FATAL
-# RUN: FileCheck %s --input-file=%t-reserved.err --check-prefix=RESERVED
-# RUN: llvm-dwarfdump -debug-line %t-reserved.o -verbose 2> %t-reserved-verbose.err \
-# RUN: | FileCheck %s --check-prefixes=FIRST,FATAL
-# RUN: FileCheck %s --input-file=%t-reserved-verbose.err --check-prefix=RESERVED
+# RUN: llvm-dwarfdump -debug-line %t-reserved.o 2>&1 \
+# RUN: | FileCheck %s --check-prefixes=FIRST,FATAL,RESERVED
+# RUN: llvm-dwarfdump -debug-line %t-reserved.o -verbose 2>&1 \
+# RUN: | FileCheck %s --check-prefixes=FIRST,FATAL,RESERVED
## We only produce warnings for malformed tables after the specified unit if
## parsing can continue.
-# RUN: llvm-dwarfdump -debug-line=0 %t-reserved.o 2> %t-reserved-off-first.err \
-# RUN: | FileCheck %s --check-prefixes=FIRST,NOLATER
-# RUN: FileCheck %s --input-file=%t-reserved-off-first.err --check-prefix=RESERVED
+# RUN: llvm-dwarfdump -debug-line=0 %t-reserved.o 2>&1 \
+# RUN: | FileCheck %s --check-prefixes=FIRST,NOLATER,RESERVED
## Stop looking for the specified unit, if a fatally-bad prologue is detected.
-# RUN: llvm-dwarfdump -debug-line=0x4b %t-reserved.o 2> %t-reserved-off-last.err \
-# RUN: | FileCheck %s --check-prefixes=NOFIRST,NOLATER
-# RUN: FileCheck %s --input-file=%t-reserved-off-last.err --check-prefix=RESERVED
+# RUN: llvm-dwarfdump -debug-line=0x4b %t-reserved.o 2>&1 \
+# RUN: | FileCheck %s --check-prefixes=NOFIRST,NOLATER,RESERVED
## Show that non-fatal errors do not prevent parsing the rest of the section.
# RUN: llvm-mc -triple x86_64-pc-linux %S/Inputs/debug_line_malformed.s -filetype=obj -o %t-malformed.o
@@ -49,8 +45,9 @@
## For fatal issues, the following table(s) should not be dumped:
# FATAL: debug_line[0x00000048]
-# FATAL-NEXT: Line table prologue
-# FATAL-NEXT: total_length: 0xfffffffe
+# RESERVED-NOT: prologue
+# RESERVED: warning: parsing line table prologue at offset 0x00000048 unsupported reserved unit length found of value 0xfffffffe
+# RESERVED-NOT: prologue
# FATAL-NOT: debug_line
## For non-fatal issues, the table data should be dumped:
@@ -58,16 +55,21 @@
## Version 0 table.
# NONFATAL: debug_line[0x00000048]
# NONFATAL-NEXT: Line table prologue
-# NONFATAL-NOT: Address
+# NONFATAL-NEXT: total_length: 0x00000002
+# NONFATAL-NEXT: version: 0
+# NONFATAL-NOT: prologue_length
## Version 1 table.
# NONFATAL: debug_line[0x0000004e]
# NONFATAL-NEXT: Line table prologue
-# NONFATAL-NOT: Address
+# NONFATAL-NEXT: total_length: 0x00000002
+# NONFATAL-NEXT: version: 1
+# NONFATAL-NOT: prologue_length
## Malformed directory format with no path component.
# NONFATAL: debug_line[0x00000054]
# NONFATAL-NEXT: Line table prologue
+# NONFATAL: prologue_length: 0x00000013
# NONFATAL-NOT: include_directories
# NONFATAL-NOT: file_names
# NONFATAL: 0x8877665544332211 {{.*}} end_sequence
@@ -96,12 +98,14 @@
## Extended opcode with incorrect length versus expected.
# NONFATAL: debug_line[0x00000111]
# NONFATAL-NEXT: Line table prologue
+# NONFATAL: prologue_length: 0x00000030
# NONFATAL: 0x00000000abbadaba {{.*}} end_sequence
# NONFATAL: 0x00000000babb1e45 {{.*}} 10 is_stmt prologue_end end_sequence{{$}}
## No end of sequence.
# NONFATAL: debug_line[0x0000016c]
# NONFATAL-NEXT: Line table prologue
+# NONFATAL: prologue_length: 0x00000030
# NONFATAL: 0x00000000deadfade {{.*}} is_stmt
# NONFATAL-NOT: end_sequence
@@ -192,8 +196,6 @@
# 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
-
# ALL-NOT: warning:
# ALL: warning: parsing line table prologue at offset 0x00000048 found unsupported version 0
# ALL-NEXT: warning: parsing line table prologue at offset 0x0000004e found unsupported version 1
More information about the llvm-commits
mailing list