[llvm] [DebugInfo] Separate error generation from reporting in DWARFHeaderUnit::extract (PR #68242)
Alex Langford via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 17 10:45:42 PDT 2023
https://github.com/bulbazord updated https://github.com/llvm/llvm-project/pull/68242
>From f162e71b17ba98cb985bc1dce96e88379f44591e 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 1/3] [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 372897835cce199..61d81709b1f1c5c 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 0cd45bde3e25349..9f455fa7e96a7ef 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.
>From 9ec56388ae072fb654771ae0316a620efaaa8cb5 Mon Sep 17 00:00:00 2001
From: Alex Langford <alangford at apple.com>
Date: Mon, 16 Oct 2023 17:02:04 -0700
Subject: [PATCH 2/3] [DebugInfo] Add new test
---
.../X86/cu_tu_units_manual_v5_invalid.s | 75 +++++++++++++++++++
1 file changed, 75 insertions(+)
create mode 100644 llvm/test/tools/llvm-dwp/X86/cu_tu_units_manual_v5_invalid.s
diff --git a/llvm/test/tools/llvm-dwp/X86/cu_tu_units_manual_v5_invalid.s b/llvm/test/tools/llvm-dwp/X86/cu_tu_units_manual_v5_invalid.s
new file mode 100644
index 000000000000000..b8dd84be46db268
--- /dev/null
+++ b/llvm/test/tools/llvm-dwp/X86/cu_tu_units_manual_v5_invalid.s
@@ -0,0 +1,75 @@
+# This test checks that llvm-dwarfdump correctly reports errors when parsing
+# DWARF Unit Headers in DWP files
+
+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t.o \
+# RUN: -split-dwarf-file=%t.dwo -dwarf-version=5
+# RUN: llvm-dwp %t.dwo -o %t.dwp
+# RUN: llvm-dwarfdump -debug-info -debug-cu-index -debug-tu-index \
+# RUN: -manaully-generate-unit-index %t.dwp 2>&1 | FileCheck %s
+
+## Note: In order to check whether the type unit index is generated
+## there is no need to add the missing DIEs for the structure type of the type unit.
+
+# CHECK-NOT: .debug_info.dwo contents:
+
+# CHECK-DAG: .debug_cu_index contents:
+# CHECK: Failed to parse CU header in DWP file
+# CHECK-NEXT: DWARF unit at offset 0x00000000 has unsupported version 6, supported are 2-5
+
+# CHECK-DAG: .debug_tu_index contents:
+# CHECK: Failed to parse CU header in DWP file
+# CHECK-NEXT: DWARF unit at offset 0x00000000 has unsupported version 6, supported are 2-5
+
+ .section .debug_info.dwo,"e", at progbits
+ .long .Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit
+.Ldebug_info_dwo_start0:
+ .short 6 # DWARF version number
+ .byte 6 # DWARF Unit Type (DW_UT_split_type)
+ .byte 8 # Address Size (in bytes)
+ .long 0 # Offset Into Abbrev. Section
+ .quad 5657452045627120676 # Type Signature
+ .long 25 # Type DIE Offset
+ .byte 2 # Abbrev [2] DW_TAG_type_unit
+ .byte 3 # Abbrev [3] DW_TAG_structure_type
+ .byte 0 # End Of Children Mark
+.Ldebug_info_dwo_end0:
+ .section .debug_info.dwo,"e", at progbits
+ .long .Ldebug_info_dwo_end1-.Ldebug_info_dwo_start1 # Length of Unit
+.Ldebug_info_dwo_start1:
+ .short 6 # DWARF version number
+ .byte 6 # DWARF Unit Type (DW_UT_split_type)
+ .byte 8 # Address Size (in bytes)
+ .long 0 # Offset Into Abbrev. Section
+ .quad -8528522068957683993 # Type Signature
+ .long 25 # Type DIE Offset
+ .byte 4 # Abbrev [4] DW_TAG_type_unit
+ .byte 5 # Abbrev [5] DW_TAG_structure_type
+ .byte 0 # End Of Children Mark
+.Ldebug_info_dwo_end1:
+ .section .debug_info.dwo,"e", at progbits
+ .long .Ldebug_info_dwo_end2-.Ldebug_info_dwo_start2 # Length of Unit
+.Ldebug_info_dwo_start2:
+ .short 6 # DWARF version number
+ .byte 5 # DWARF Unit Type (DW_UT_split_compile)
+ .byte 8 # Address Size (in bytes)
+ .long 0 # Offset Into Abbrev. Section
+ .quad 1152943841751211454
+ .byte 1 # Abbrev [1] DW_TAG_compile_unit
+.Ldebug_info_dwo_end2:
+ .section .debug_abbrev.dwo,"e", at progbits
+ .byte 1 # Abbreviation Code
+ .byte 17 # DW_TAG_compile_unit
+ .byte 0 # DW_CHILDREN_no
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 2 # Abbreviation Code
+ .byte 65 # DW_TAG_type_unit
+ .byte 1 # DW_CHILDREN_yes
+ .byte 0 # EOM
+ .byte 0 # EOM
+ .byte 4 # Abbreviation Code
+ .byte 65 # DW_TAG_type_unit
+ .byte 1 # DW_CHILDREN_yes
+ .byte 0 # EOM
+ .byte 0 # EOM
+ .byte 0 # EOM
>From 46f8bacd8bdece81a70f2d5015fd010d7bd532d7 Mon Sep 17 00:00:00 2001
From: Alex Langford <alangford at apple.com>
Date: Tue, 17 Oct 2023 10:45:19 -0700
Subject: [PATCH 3/3] [DebugInfo] Create new error instead of joining existing
errors
---
llvm/lib/DebugInfo/DWARF/DWARFContext.cpp | 8 ++++----
.../tools/llvm-dwp/X86/cu_tu_units_manual_v5_invalid.s | 6 ++----
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index 61d81709b1f1c5c..724f816ad094a75 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -92,8 +92,8 @@ void fixupIndexV4(DWARFContext &C, DWARFUnitIndex &Index) {
if (Error ExtractionErr = Header.extract(
C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
logAllUnhandledErrors(
- joinErrors(createError("Failed to parse CU header in DWP file"),
- std::move(ExtractionErr)),
+ createError("Failed to parse CU header in DWP file: " +
+ toString(std::move(ExtractionErr))),
errs());
Map.clear();
break;
@@ -155,8 +155,8 @@ void fixupIndexV5(DWARFContext &C, DWARFUnitIndex &Index) {
if (Error ExtractionErr = Header.extract(
C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
logAllUnhandledErrors(
- joinErrors(createError("Failed to parse CU header in DWP file"),
- std::move(ExtractionErr)),
+ createError("Failed to parse CU header in DWP file: " +
+ toString(std::move(ExtractionErr))),
errs());
break;
}
diff --git a/llvm/test/tools/llvm-dwp/X86/cu_tu_units_manual_v5_invalid.s b/llvm/test/tools/llvm-dwp/X86/cu_tu_units_manual_v5_invalid.s
index b8dd84be46db268..d1ab9f75b74c8fa 100644
--- a/llvm/test/tools/llvm-dwp/X86/cu_tu_units_manual_v5_invalid.s
+++ b/llvm/test/tools/llvm-dwp/X86/cu_tu_units_manual_v5_invalid.s
@@ -13,12 +13,10 @@
# CHECK-NOT: .debug_info.dwo contents:
# CHECK-DAG: .debug_cu_index contents:
-# CHECK: Failed to parse CU header in DWP file
-# CHECK-NEXT: DWARF unit at offset 0x00000000 has unsupported version 6, supported are 2-5
+# CHECK: Failed to parse CU header in DWP file: DWARF unit at offset 0x00000000 has unsupported version 6, supported are 2-5
# CHECK-DAG: .debug_tu_index contents:
-# CHECK: Failed to parse CU header in DWP file
-# CHECK-NEXT: DWARF unit at offset 0x00000000 has unsupported version 6, supported are 2-5
+# CHECK: Failed to parse CU header in DWP file: DWARF unit at offset 0x00000000 has unsupported version 6, supported are 2-5
.section .debug_info.dwo,"e", at progbits
.long .Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit
More information about the llvm-commits
mailing list