[llvm] [lldb][llvm] Return an error instead of crashing when parsing a line table prologue. (PR #80769)
Greg Clayton via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 6 15:50:26 PST 2024
https://github.com/clayborg updated https://github.com/llvm/llvm-project/pull/80769
>From 81a093cf7d42bbde6302d8300031a3673e24817d Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Fri, 2 Feb 2024 15:18:26 -0800
Subject: [PATCH 1/3] [lldb] Return an error instead of crash when parsing line
table prologue.
We recently ran into some bad DWARF where the DW_AT_stmt_list of many compile units was randomly set to invalid values and was causing LLDB to crash due to an assertion. Instead of asserting, we should return an appropriate llvm::Error.
---
llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp | 44 ++++++++++++++++---
.../DebugInfo/DWARF/DWARFDebugLineTest.cpp | 13 ++----
2 files changed, 41 insertions(+), 16 deletions(-)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
index 28f05644a3aa11..60513f09936aaa 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
@@ -372,26 +372,56 @@ Error DWARFDebugLine::Prologue::parse(
std::tie(TotalLength, FormParams.Format) =
DebugLineData.getInitialLength(Cursor);
+ // Report an error, but also make sure the cursor is not in a bad state. If
+ // the cursor is in a bad state, return its error instead. Many times during
+ // parsing we might end up bailing if we don't like what we see, but often
+ // times when we decode items with a bad cursor, we will get zero back which
+ // might not be valid data to extract.
+ auto ReportErrorOrCursor = [&](llvm::Error Error) -> llvm::Error {
+ if (Cursor)
+ return Error;
+ llvm::consumeError(std::move(Error));
+ *OffsetPtr = Cursor.tell();
+ std::string CursorErrorStr = llvm::toString(Cursor.takeError());
+ return createStringError(
+ errc::not_supported,
+ "parsing line table prologue at offset 0x%8.8" PRIx64 ": %s",
+ PrologueOffset, CursorErrorStr.c_str());
+ };
+
DebugLineData =
DWARFDataExtractor(DebugLineData, Cursor.tell() + TotalLength);
FormParams.Version = DebugLineData.getU16(Cursor);
- if (Cursor && !versionIsSupported(getVersion())) {
+ 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.
- *OffsetPtr = Cursor.tell();
- return createStringError(
+ return ReportErrorOrCursor(createStringError(
errc::not_supported,
"parsing line table prologue at offset 0x%8.8" PRIx64
": unsupported version %" PRIu16,
- PrologueOffset, getVersion());
+ PrologueOffset, getVersion()));
}
if (getVersion() >= 5) {
FormParams.AddrSize = DebugLineData.getU8(Cursor);
- assert((!Cursor || DebugLineData.getAddressSize() == 0 ||
- DebugLineData.getAddressSize() == getAddressSize()) &&
- "Line table header and data extractor disagree");
+ const uint8_t DataAddrSize = DebugLineData.getAddressSize();
+ const uint8_t PrologueAddrSize = FormParams.AddrSize;
+ if (DataAddrSize == 0) {
+ if (PrologueAddrSize != 4 && PrologueAddrSize != 8) {
+ return ReportErrorOrCursor(createStringError(
+ errc::not_supported,
+ "parsing line table prologue at offset 0x%8.8" PRIx64
+ ": invalid address size %" PRIu8, PrologueOffset,
+ PrologueAddrSize));
+ }
+ } else if (DataAddrSize != getAddressSize()) {
+ return ReportErrorOrCursor(createStringError(
+ errc::not_supported,
+ "parsing line table prologue at offset 0x%8.8" PRIx64 ": address "
+ "size %" PRIu8 " doesn't match architecture address size %" PRIu8,
+ PrologueOffset, PrologueAddrSize, DataAddrSize));
+ }
SegSelectorSize = DebugLineData.getU8(Cursor);
}
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
index d42a626fa9c1cb..d7d04312a1c4c5 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
@@ -821,15 +821,10 @@ TEST_F(DebugLineBasicFixture, ErrorForUnsupportedAddressSizeDefinedInHeader) {
auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
nullptr, RecordRecoverable);
- EXPECT_THAT_ERROR(
- std::move(Recoverable),
- FailedWithMessage("address size 0x09 of DW_LNE_set_address opcode at "
- "offset 0x00000038 is unsupported"));
- ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
- ASSERT_EQ((*ExpectedLineTable)->Rows.size(), 3u);
- EXPECT_EQ((*ExpectedLineTable)->Sequences.size(), 1u);
- // Show that the set address opcode is ignored in this case.
- EXPECT_EQ((*ExpectedLineTable)->Rows[0].Address.Address, 0u);
+ ASSERT_THAT_EXPECTED(ExpectedLineTable,
+ FailedWithMessage("parsing line table prologue at "
+ "offset 0x00000000: invalid address "
+ "size 9"));
}
TEST_F(DebugLineBasicFixture, CallbackUsedForUnterminatedSequence) {
>From a3dd8ecc483ef2d5adcdb42a58e83af5d95d0248 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Tue, 6 Feb 2024 15:47:04 -0800
Subject: [PATCH 2/3] Change to make address size issues recoverable.
---
llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp | 50 +++++++------------
.../DebugInfo/DWARF/DWARFDebugLineTest.cpp | 15 ++++--
2 files changed, 29 insertions(+), 36 deletions(-)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
index 60513f09936aaa..4a1b679982a11c 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
@@ -372,55 +372,41 @@ Error DWARFDebugLine::Prologue::parse(
std::tie(TotalLength, FormParams.Format) =
DebugLineData.getInitialLength(Cursor);
- // Report an error, but also make sure the cursor is not in a bad state. If
- // the cursor is in a bad state, return its error instead. Many times during
- // parsing we might end up bailing if we don't like what we see, but often
- // times when we decode items with a bad cursor, we will get zero back which
- // might not be valid data to extract.
- auto ReportErrorOrCursor = [&](llvm::Error Error) -> llvm::Error {
- if (Cursor)
- return Error;
- llvm::consumeError(std::move(Error));
- *OffsetPtr = Cursor.tell();
- std::string CursorErrorStr = llvm::toString(Cursor.takeError());
- return createStringError(
- errc::not_supported,
- "parsing line table prologue at offset 0x%8.8" PRIx64 ": %s",
- PrologueOffset, CursorErrorStr.c_str());
- };
-
DebugLineData =
DWARFDataExtractor(DebugLineData, Cursor.tell() + TotalLength);
FormParams.Version = DebugLineData.getU16(Cursor);
- if (!versionIsSupported(getVersion())) {
+ if (Cursor && !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.
- return ReportErrorOrCursor(createStringError(
+ *OffsetPtr = Cursor.tell();
+ return createStringError(
errc::not_supported,
"parsing line table prologue at offset 0x%8.8" PRIx64
": unsupported version %" PRIu16,
- PrologueOffset, getVersion()));
+ PrologueOffset, getVersion());
}
if (getVersion() >= 5) {
FormParams.AddrSize = DebugLineData.getU8(Cursor);
const uint8_t DataAddrSize = DebugLineData.getAddressSize();
const uint8_t PrologueAddrSize = FormParams.AddrSize;
- if (DataAddrSize == 0) {
- if (PrologueAddrSize != 4 && PrologueAddrSize != 8) {
- return ReportErrorOrCursor(createStringError(
+ if (Cursor) {
+ if (DataAddrSize == 0) {
+ if (PrologueAddrSize != 4 && PrologueAddrSize != 8) {
+ RecoverableErrorHandler(createStringError(
+ errc::not_supported,
+ "parsing line table prologue at offset 0x%8.8" PRIx64
+ ": invalid address size %" PRIu8,
+ PrologueOffset, PrologueAddrSize));
+ }
+ } else if (DataAddrSize != getAddressSize()) {
+ RecoverableErrorHandler(createStringError(
errc::not_supported,
- "parsing line table prologue at offset 0x%8.8" PRIx64
- ": invalid address size %" PRIu8, PrologueOffset,
- PrologueAddrSize));
+ "parsing line table prologue at offset 0x%8.8" PRIx64 ": address "
+ "size %" PRIu8 " doesn't match architecture address size %" PRIu8,
+ PrologueOffset, PrologueAddrSize, DataAddrSize));
}
- } else if (DataAddrSize != getAddressSize()) {
- return ReportErrorOrCursor(createStringError(
- errc::not_supported,
- "parsing line table prologue at offset 0x%8.8" PRIx64 ": address "
- "size %" PRIu8 " doesn't match architecture address size %" PRIu8,
- PrologueOffset, PrologueAddrSize, DataAddrSize));
}
SegSelectorSize = DebugLineData.getU8(Cursor);
}
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
index d7d04312a1c4c5..980b627625eef6 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
@@ -821,10 +821,17 @@ TEST_F(DebugLineBasicFixture, ErrorForUnsupportedAddressSizeDefinedInHeader) {
auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
nullptr, RecordRecoverable);
- ASSERT_THAT_EXPECTED(ExpectedLineTable,
- FailedWithMessage("parsing line table prologue at "
- "offset 0x00000000: invalid address "
- "size 9"));
+ EXPECT_THAT_ERROR(
+ std::move(Recoverable),
+ FailedWithMessage("parsing line table prologue at offset 0x00000000: "
+ "invalid address size 9",
+ "address size 0x09 of DW_LNE_set_address opcode at "
+ "offset 0x00000038 is unsupported"));
+ ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
+ ASSERT_EQ((*ExpectedLineTable)->Rows.size(), 3u);
+ EXPECT_EQ((*ExpectedLineTable)->Sequences.size(), 1u);
+ // Show that the set address opcode is ignored in this case.
+ EXPECT_EQ((*ExpectedLineTable)->Rows[0].Address.Address, 0u);
}
TEST_F(DebugLineBasicFixture, CallbackUsedForUnterminatedSequence) {
>From 682c3925b05135c4b43fe6fc3672c71bb0ec0214 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Tue, 6 Feb 2024 15:49:53 -0800
Subject: [PATCH 3/3] Re-use PrologueAddrSize so I don't mix PrologueAddrSize
and getAddressSize().
---
llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
index 4a1b679982a11c..572628f45fc23a 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
@@ -390,7 +390,7 @@ Error DWARFDebugLine::Prologue::parse(
if (getVersion() >= 5) {
FormParams.AddrSize = DebugLineData.getU8(Cursor);
const uint8_t DataAddrSize = DebugLineData.getAddressSize();
- const uint8_t PrologueAddrSize = FormParams.AddrSize;
+ const uint8_t PrologueAddrSize = getAddressSize();
if (Cursor) {
if (DataAddrSize == 0) {
if (PrologueAddrSize != 4 && PrologueAddrSize != 8) {
@@ -400,7 +400,7 @@ Error DWARFDebugLine::Prologue::parse(
": invalid address size %" PRIu8,
PrologueOffset, PrologueAddrSize));
}
- } else if (DataAddrSize != getAddressSize()) {
+ } else if (DataAddrSize != PrologueAddrSize) {
RecoverableErrorHandler(createStringError(
errc::not_supported,
"parsing line table prologue at offset 0x%8.8" PRIx64 ": address "
More information about the llvm-commits
mailing list