Skip to content

Commit 814a79a

Browse files
authored
[DebugInfo] Separate error generation from reporting in DWARFHeaderUnit::extract (#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.
1 parent c80b503 commit 814a79a

File tree

5 files changed

+124
-52
lines changed

5 files changed

+124
-52
lines changed

llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ class DWARFUnitHeader {
7979
/// Note that \p SectionKind is used as a hint to guess the unit type
8080
/// for DWARF formats prior to DWARFv5. In DWARFv5 the unit type is
8181
/// explicitly defined in the header and the hint is ignored.
82-
bool extract(DWARFContext &Context, const DWARFDataExtractor &debug_info,
83-
uint64_t *offset_ptr, DWARFSectionKind SectionKind);
82+
Error extract(DWARFContext &Context, const DWARFDataExtractor &debug_info,
83+
uint64_t *offset_ptr, DWARFSectionKind SectionKind);
8484
// For units in DWARF Package File, remember the index entry and update
8585
// the abbreviation offset read by extract().
8686
bool applyIndexEntry(const DWARFUnitIndex::Entry *Entry);

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,12 @@ void fixupIndexV4(DWARFContext &C, DWARFUnitIndex &Index) {
8989
DWARFDataExtractor Data(DObj, S, C.isLittleEndian(), 0);
9090
while (Data.isValidOffset(Offset)) {
9191
DWARFUnitHeader Header;
92-
if (!Header.extract(C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
92+
if (Error ExtractionErr = Header.extract(
93+
C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
9394
logAllUnhandledErrors(
94-
createError("Failed to parse CU header in DWP file"), errs());
95+
createError("Failed to parse CU header in DWP file: " +
96+
toString(std::move(ExtractionErr))),
97+
errs());
9598
Map.clear();
9699
break;
97100
}
@@ -149,9 +152,12 @@ void fixupIndexV5(DWARFContext &C, DWARFUnitIndex &Index) {
149152
uint64_t Offset = 0;
150153
while (Data.isValidOffset(Offset)) {
151154
DWARFUnitHeader Header;
152-
if (!Header.extract(C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
155+
if (Error ExtractionErr = Header.extract(
156+
C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
153157
logAllUnhandledErrors(
154-
createError("Failed to parse unit header in DWP file"), errs());
158+
createError("Failed to parse CU header in DWP file: " +
159+
toString(std::move(ExtractionErr))),
160+
errs());
155161
break;
156162
}
157163
bool CU = Header.getUnitType() == DW_UT_split_compile;

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

Lines changed: 34 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,11 @@ void DWARFUnitVector::addUnitsImpl(
8181
if (!Data.isValidOffset(Offset))
8282
return nullptr;
8383
DWARFUnitHeader Header;
84-
if (!Header.extract(Context, Data, &Offset, SectionKind))
84+
if (Error ExtractErr =
85+
Header.extract(Context, Data, &Offset, SectionKind)) {
86+
Context.getWarningHandler()(std::move(ExtractErr));
8587
return nullptr;
88+
}
8689
if (!IndexEntry && IsDWO) {
8790
const DWARFUnitIndex &Index = getDWARFUnitIndex(
8891
Context, Header.isTypeUnit() ? DW_SECT_EXT_TYPES : DW_SECT_INFO);
@@ -244,10 +247,10 @@ Expected<uint64_t> DWARFUnit::getStringOffsetSectionItem(uint32_t Index) const {
244247
return DA.getRelocatedValue(ItemSize, &Offset);
245248
}
246249

247-
bool DWARFUnitHeader::extract(DWARFContext &Context,
248-
const DWARFDataExtractor &debug_info,
249-
uint64_t *offset_ptr,
250-
DWARFSectionKind SectionKind) {
250+
Error DWARFUnitHeader::extract(DWARFContext &Context,
251+
const DWARFDataExtractor &debug_info,
252+
uint64_t *offset_ptr,
253+
DWARFSectionKind SectionKind) {
251254
Offset = *offset_ptr;
252255
Error Err = Error::success();
253256
IndexEntry = nullptr;
@@ -277,72 +280,58 @@ bool DWARFUnitHeader::extract(DWARFContext &Context,
277280
} else if (UnitType == DW_UT_split_compile || UnitType == DW_UT_skeleton)
278281
DWOId = debug_info.getU64(offset_ptr, &Err);
279282

280-
if (Err) {
281-
Context.getWarningHandler()(joinErrors(
283+
if (Err)
284+
return joinErrors(
282285
createStringError(
283286
errc::invalid_argument,
284287
"DWARF unit at 0x%8.8" PRIx64 " cannot be parsed:", Offset),
285-
std::move(Err)));
286-
return false;
287-
}
288+
std::move(Err));
288289

289290
// Header fields all parsed, capture the size of this unit header.
290291
assert(*offset_ptr - Offset <= 255 && "unexpected header size");
291292
Size = uint8_t(*offset_ptr - Offset);
292293
uint64_t NextCUOffset = Offset + getUnitLengthFieldByteSize() + getLength();
293294

294-
if (!debug_info.isValidOffset(getNextUnitOffset() - 1)) {
295-
Context.getWarningHandler()(
296-
createStringError(errc::invalid_argument,
297-
"DWARF unit from offset 0x%8.8" PRIx64 " incl. "
298-
"to offset 0x%8.8" PRIx64 " excl. "
299-
"extends past section size 0x%8.8zx",
300-
Offset, NextCUOffset, debug_info.size()));
301-
return false;
302-
}
295+
if (!debug_info.isValidOffset(getNextUnitOffset() - 1))
296+
return createStringError(errc::invalid_argument,
297+
"DWARF unit from offset 0x%8.8" PRIx64 " incl. "
298+
"to offset 0x%8.8" PRIx64 " excl. "
299+
"extends past section size 0x%8.8zx",
300+
Offset, NextCUOffset, debug_info.size());
303301

304-
if (!DWARFContext::isSupportedVersion(getVersion())) {
305-
Context.getWarningHandler()(createStringError(
302+
if (!DWARFContext::isSupportedVersion(getVersion()))
303+
return createStringError(
306304
errc::invalid_argument,
307305
"DWARF unit at offset 0x%8.8" PRIx64 " "
308306
"has unsupported version %" PRIu16 ", supported are 2-%u",
309-
Offset, getVersion(), DWARFContext::getMaxSupportedVersion()));
310-
return false;
311-
}
307+
Offset, getVersion(), DWARFContext::getMaxSupportedVersion());
312308

313309
// Type offset is unit-relative; should be after the header and before
314310
// the end of the current unit.
315-
if (isTypeUnit() && TypeOffset < Size) {
316-
Context.getWarningHandler()(
317-
createStringError(errc::invalid_argument,
318-
"DWARF type unit at offset "
319-
"0x%8.8" PRIx64 " "
320-
"has its relocated type_offset 0x%8.8" PRIx64 " "
321-
"pointing inside the header",
322-
Offset, Offset + TypeOffset));
323-
return false;
324-
}
325-
if (isTypeUnit() &&
326-
TypeOffset >= getUnitLengthFieldByteSize() + getLength()) {
327-
Context.getWarningHandler()(createStringError(
311+
if (isTypeUnit() && TypeOffset < Size)
312+
return createStringError(errc::invalid_argument,
313+
"DWARF type unit at offset "
314+
"0x%8.8" PRIx64 " "
315+
"has its relocated type_offset 0x%8.8" PRIx64 " "
316+
"pointing inside the header",
317+
Offset, Offset + TypeOffset);
318+
319+
if (isTypeUnit() && TypeOffset >= getUnitLengthFieldByteSize() + getLength())
320+
return createStringError(
328321
errc::invalid_argument,
329322
"DWARF type unit from offset 0x%8.8" PRIx64 " incl. "
330323
"to offset 0x%8.8" PRIx64 " excl. has its "
331324
"relocated type_offset 0x%8.8" PRIx64 " pointing past the unit end",
332-
Offset, NextCUOffset, Offset + TypeOffset));
333-
return false;
334-
}
325+
Offset, NextCUOffset, Offset + TypeOffset);
335326

336327
if (Error SizeErr = DWARFContext::checkAddressSizeSupported(
337328
getAddressByteSize(), errc::invalid_argument,
338-
"DWARF unit at offset 0x%8.8" PRIx64, Offset)) {
339-
Context.getWarningHandler()(std::move(SizeErr));
340-
return false;
341-
}
329+
"DWARF unit at offset 0x%8.8" PRIx64, Offset))
330+
return SizeErr;
342331

343332
// Keep track of the highest DWARF version we encounter across all units.
344333
Context.setMaxVersionIfGreater(getVersion());
345-
return true;
334+
return Error::success();
346335
}
347336

348337
bool DWARFUnitHeader::applyIndexEntry(const DWARFUnitIndex::Entry *Entry) {
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# This test checks that llvm-dwarfdump correctly reports errors when parsing
2+
# DWARF Unit Headers in DWP files
3+
4+
# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t.o \
5+
# RUN: -split-dwarf-file=%t.dwo -dwarf-version=5
6+
# RUN: llvm-dwp %t.dwo -o %t.dwp
7+
# RUN: llvm-dwarfdump -debug-info -debug-cu-index -debug-tu-index \
8+
# RUN: -manaully-generate-unit-index %t.dwp 2>&1 | FileCheck %s
9+
10+
## Note: In order to check whether the type unit index is generated
11+
## there is no need to add the missing DIEs for the structure type of the type unit.
12+
13+
# CHECK-NOT: .debug_info.dwo contents:
14+
15+
# CHECK-DAG: .debug_cu_index contents:
16+
# CHECK: Failed to parse CU header in DWP file: DWARF unit at offset 0x00000000 has unsupported version 6, supported are 2-5
17+
18+
# CHECK-DAG: .debug_tu_index contents:
19+
# CHECK: Failed to parse CU header in DWP file: DWARF unit at offset 0x00000000 has unsupported version 6, supported are 2-5
20+
21+
.section .debug_info.dwo,"e",@progbits
22+
.long .Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit
23+
.Ldebug_info_dwo_start0:
24+
.short 6 # DWARF version number
25+
.byte 6 # DWARF Unit Type (DW_UT_split_type)
26+
.byte 8 # Address Size (in bytes)
27+
.long 0 # Offset Into Abbrev. Section
28+
.quad 5657452045627120676 # Type Signature
29+
.long 25 # Type DIE Offset
30+
.byte 2 # Abbrev [2] DW_TAG_type_unit
31+
.byte 3 # Abbrev [3] DW_TAG_structure_type
32+
.byte 0 # End Of Children Mark
33+
.Ldebug_info_dwo_end0:
34+
.section .debug_info.dwo,"e",@progbits
35+
.long .Ldebug_info_dwo_end1-.Ldebug_info_dwo_start1 # Length of Unit
36+
.Ldebug_info_dwo_start1:
37+
.short 6 # DWARF version number
38+
.byte 6 # DWARF Unit Type (DW_UT_split_type)
39+
.byte 8 # Address Size (in bytes)
40+
.long 0 # Offset Into Abbrev. Section
41+
.quad -8528522068957683993 # Type Signature
42+
.long 25 # Type DIE Offset
43+
.byte 4 # Abbrev [4] DW_TAG_type_unit
44+
.byte 5 # Abbrev [5] DW_TAG_structure_type
45+
.byte 0 # End Of Children Mark
46+
.Ldebug_info_dwo_end1:
47+
.section .debug_info.dwo,"e",@progbits
48+
.long .Ldebug_info_dwo_end2-.Ldebug_info_dwo_start2 # Length of Unit
49+
.Ldebug_info_dwo_start2:
50+
.short 6 # DWARF version number
51+
.byte 5 # DWARF Unit Type (DW_UT_split_compile)
52+
.byte 8 # Address Size (in bytes)
53+
.long 0 # Offset Into Abbrev. Section
54+
.quad 1152943841751211454
55+
.byte 1 # Abbrev [1] DW_TAG_compile_unit
56+
.Ldebug_info_dwo_end2:
57+
.section .debug_abbrev.dwo,"e",@progbits
58+
.byte 1 # Abbreviation Code
59+
.byte 17 # DW_TAG_compile_unit
60+
.byte 0 # DW_CHILDREN_no
61+
.byte 0 # EOM(1)
62+
.byte 0 # EOM(2)
63+
.byte 2 # Abbreviation Code
64+
.byte 65 # DW_TAG_type_unit
65+
.byte 1 # DW_CHILDREN_yes
66+
.byte 0 # EOM
67+
.byte 0 # EOM
68+
.byte 4 # Abbreviation Code
69+
.byte 65 # DW_TAG_type_unit
70+
.byte 1 # DW_CHILDREN_yes
71+
.byte 0 # EOM
72+
.byte 0 # EOM
73+
.byte 0 # EOM

llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2170,7 +2170,11 @@ TEST(DWARFDebugInfo, TestDWARF64UnitLength) {
21702170
DWARFDataExtractor Data(Obj, Sec, /* IsLittleEndian = */ true,
21712171
/* AddressSize = */ 4);
21722172
uint64_t Offset = 0;
2173-
EXPECT_FALSE(Header.extract(*Context, Data, &Offset, DW_SECT_INFO));
2173+
ASSERT_THAT_ERROR(
2174+
Header.extract(*Context, Data, &Offset, DW_SECT_INFO),
2175+
FailedWithMessage(
2176+
"DWARF unit from offset 0x00000000 incl. to offset "
2177+
"0x1122334455667794 excl. extends past section size 0x00000018"));
21742178
// Header.extract() returns false because there is not enough space
21752179
// in the section for the declared length. Anyway, we can check that
21762180
// the properties are read correctly.

0 commit comments

Comments
 (0)