[llvm] [DebugInfo] Separate error generation from reporting in DWARFHeaderUnit::extract (PR #68242)
Alex Langford via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 4 11:07:40 PDT 2023
https://github.com/bulbazord created https://github.com/llvm/llvm-project/pull/68242
Instead of reporting the error directly through the DWARFContext passed in as an argument, it would be more flexible to have extract return the error and allow the caller to react appropriately.
This will be useful for using llvm's DWARFHeaderUnit from lldb which may report header extraction errors through a different mechanism.
>From 04840770b96818a3ec7998b13643ee89b3698b24 Mon Sep 17 00:00:00 2001
From: Alex Langford <alangford at apple.com>
Date: Tue, 3 Oct 2023 18:04:21 -0700
Subject: [PATCH] [DebugInfo] Separate error generation from reporting in
DWARFHeaderUnit::extract
Instead of reporting the error directly through the DWARFContext passed
in as an argument, it would be more flexible to have extract return the
error and allow the caller to react appropriately.
This will be useful for using llvm's DWARFHeaderUnit from lldb which may
report header extraction errors through a different mechanism.
---
llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h | 4 +-
llvm/lib/DebugInfo/DWARF/DWARFContext.cpp | 14 +++-
llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp | 79 ++++++++-----------
.../DebugInfo/DWARF/DWARFDebugInfoTest.cpp | 6 +-
4 files changed, 51 insertions(+), 52 deletions(-)
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
index 3c0770787463e6c..7084081ce61a43a 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
@@ -79,8 +79,8 @@ class DWARFUnitHeader {
/// Note that \p SectionKind is used as a hint to guess the unit type
/// for DWARF formats prior to DWARFv5. In DWARFv5 the unit type is
/// explicitly defined in the header and the hint is ignored.
- bool extract(DWARFContext &Context, const DWARFDataExtractor &debug_info,
- uint64_t *offset_ptr, DWARFSectionKind SectionKind);
+ Error extract(DWARFContext &Context, const DWARFDataExtractor &debug_info,
+ uint64_t *offset_ptr, DWARFSectionKind SectionKind);
// For units in DWARF Package File, remember the index entry and update
// the abbreviation offset read by extract().
bool applyIndexEntry(const DWARFUnitIndex::Entry *Entry);
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index 1e1ab814673f423..b6bca561c66da5c 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -89,9 +89,12 @@ void fixupIndexV4(DWARFContext &C, DWARFUnitIndex &Index) {
DWARFDataExtractor Data(DObj, S, C.isLittleEndian(), 0);
while (Data.isValidOffset(Offset)) {
DWARFUnitHeader Header;
- if (!Header.extract(C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
+ if (Error ExtractionErr = Header.extract(
+ C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
logAllUnhandledErrors(
- createError("Failed to parse CU header in DWP file"), errs());
+ joinErrors(createError("Failed to parse CU header in DWP file"),
+ std::move(ExtractionErr)),
+ errs());
Map.clear();
break;
}
@@ -149,9 +152,12 @@ void fixupIndexV5(DWARFContext &C, DWARFUnitIndex &Index) {
uint64_t Offset = 0;
while (Data.isValidOffset(Offset)) {
DWARFUnitHeader Header;
- if (!Header.extract(C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
+ if (Error ExtractionErr = Header.extract(
+ C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
logAllUnhandledErrors(
- createError("Failed to parse unit header in DWP file"), errs());
+ joinErrors(createError("Failed to parse CU header in DWP file"),
+ std::move(ExtractionErr)),
+ errs());
break;
}
bool CU = Header.getUnitType() == DW_UT_split_compile;
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
index 387345a4ac2d601..a45e069e63f9a83 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
@@ -81,8 +81,11 @@ void DWARFUnitVector::addUnitsImpl(
if (!Data.isValidOffset(Offset))
return nullptr;
DWARFUnitHeader Header;
- if (!Header.extract(Context, Data, &Offset, SectionKind))
+ if (Error ExtractErr =
+ Header.extract(Context, Data, &Offset, SectionKind)) {
+ Context.getWarningHandler()(std::move(ExtractErr));
return nullptr;
+ }
if (!IndexEntry && IsDWO) {
const DWARFUnitIndex &Index = getDWARFUnitIndex(
Context, Header.isTypeUnit() ? DW_SECT_EXT_TYPES : DW_SECT_INFO);
@@ -244,10 +247,10 @@ Expected<uint64_t> DWARFUnit::getStringOffsetSectionItem(uint32_t Index) const {
return DA.getRelocatedValue(ItemSize, &Offset);
}
-bool DWARFUnitHeader::extract(DWARFContext &Context,
- const DWARFDataExtractor &debug_info,
- uint64_t *offset_ptr,
- DWARFSectionKind SectionKind) {
+Error DWARFUnitHeader::extract(DWARFContext &Context,
+ const DWARFDataExtractor &debug_info,
+ uint64_t *offset_ptr,
+ DWARFSectionKind SectionKind) {
Offset = *offset_ptr;
Error Err = Error::success();
IndexEntry = nullptr;
@@ -277,72 +280,58 @@ bool DWARFUnitHeader::extract(DWARFContext &Context,
} else if (UnitType == DW_UT_split_compile || UnitType == DW_UT_skeleton)
DWOId = debug_info.getU64(offset_ptr, &Err);
- if (Err) {
- Context.getWarningHandler()(joinErrors(
+ if (Err)
+ return joinErrors(
createStringError(
errc::invalid_argument,
"DWARF unit at 0x%8.8" PRIx64 " cannot be parsed:", Offset),
- std::move(Err)));
- return false;
- }
+ std::move(Err));
// Header fields all parsed, capture the size of this unit header.
assert(*offset_ptr - Offset <= 255 && "unexpected header size");
Size = uint8_t(*offset_ptr - Offset);
uint64_t NextCUOffset = Offset + getUnitLengthFieldByteSize() + getLength();
- if (!debug_info.isValidOffset(getNextUnitOffset() - 1)) {
- Context.getWarningHandler()(
- createStringError(errc::invalid_argument,
- "DWARF unit from offset 0x%8.8" PRIx64 " incl. "
- "to offset 0x%8.8" PRIx64 " excl. "
- "extends past section size 0x%8.8zx",
- Offset, NextCUOffset, debug_info.size()));
- return false;
- }
+ if (!debug_info.isValidOffset(getNextUnitOffset() - 1))
+ return createStringError(errc::invalid_argument,
+ "DWARF unit from offset 0x%8.8" PRIx64 " incl. "
+ "to offset 0x%8.8" PRIx64 " excl. "
+ "extends past section size 0x%8.8zx",
+ Offset, NextCUOffset, debug_info.size());
- if (!DWARFContext::isSupportedVersion(getVersion())) {
- Context.getWarningHandler()(createStringError(
+ if (!DWARFContext::isSupportedVersion(getVersion()))
+ return createStringError(
errc::invalid_argument,
"DWARF unit at offset 0x%8.8" PRIx64 " "
"has unsupported version %" PRIu16 ", supported are 2-%u",
- Offset, getVersion(), DWARFContext::getMaxSupportedVersion()));
- return false;
- }
+ Offset, getVersion(), DWARFContext::getMaxSupportedVersion());
// Type offset is unit-relative; should be after the header and before
// the end of the current unit.
- if (isTypeUnit() && TypeOffset < Size) {
- Context.getWarningHandler()(
- createStringError(errc::invalid_argument,
- "DWARF type unit at offset "
- "0x%8.8" PRIx64 " "
- "has its relocated type_offset 0x%8.8" PRIx64 " "
- "pointing inside the header",
- Offset, Offset + TypeOffset));
- return false;
- }
- if (isTypeUnit() &&
- TypeOffset >= getUnitLengthFieldByteSize() + getLength()) {
- Context.getWarningHandler()(createStringError(
+ if (isTypeUnit() && TypeOffset < Size)
+ return createStringError(errc::invalid_argument,
+ "DWARF type unit at offset "
+ "0x%8.8" PRIx64 " "
+ "has its relocated type_offset 0x%8.8" PRIx64 " "
+ "pointing inside the header",
+ Offset, Offset + TypeOffset);
+
+ if (isTypeUnit() && TypeOffset >= getUnitLengthFieldByteSize() + getLength())
+ return createStringError(
errc::invalid_argument,
"DWARF type unit from offset 0x%8.8" PRIx64 " incl. "
"to offset 0x%8.8" PRIx64 " excl. has its "
"relocated type_offset 0x%8.8" PRIx64 " pointing past the unit end",
- Offset, NextCUOffset, Offset + TypeOffset));
- return false;
- }
+ Offset, NextCUOffset, Offset + TypeOffset);
if (Error SizeErr = DWARFContext::checkAddressSizeSupported(
getAddressByteSize(), errc::invalid_argument,
- "DWARF unit at offset 0x%8.8" PRIx64, Offset)) {
- Context.getWarningHandler()(std::move(SizeErr));
- return false;
- }
+ "DWARF unit at offset 0x%8.8" PRIx64, Offset))
+ return SizeErr;
// Keep track of the highest DWARF version we encounter across all units.
Context.setMaxVersionIfGreater(getVersion());
- return true;
+ return Error::success();
}
bool DWARFUnitHeader::applyIndexEntry(const DWARFUnitIndex::Entry *Entry) {
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
index 2adc2403eaca9e6..9f4fe9c54a928fd 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
@@ -2170,7 +2170,11 @@ TEST(DWARFDebugInfo, TestDWARF64UnitLength) {
DWARFDataExtractor Data(Obj, Sec, /* IsLittleEndian = */ true,
/* AddressSize = */ 4);
uint64_t Offset = 0;
- EXPECT_FALSE(Header.extract(*Context, Data, &Offset, DW_SECT_INFO));
+ ASSERT_THAT_ERROR(
+ Header.extract(*Context, Data, &Offset, DW_SECT_INFO),
+ FailedWithMessage(
+ "DWARF unit from offset 0x00000000 incl. to offset "
+ "0x1122334455667794 excl. extends past section size 0x00000018"));
// Header.extract() returns false because there is not enough space
// in the section for the declared length. Anyway, we can check that
// the properties are read correctly.
More information about the llvm-commits
mailing list