Skip to content

Add support for verifying local type units in .debug_names. #101133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 33 additions & 14 deletions llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1481,9 +1481,9 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(

unsigned
DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) {
if (NI.getLocalTUCount() + NI.getForeignTUCount() > 0) {
warn() << formatv("Name Index @ {0:x}: Verifying indexes of type units is "
"not currently supported.\n",
if (NI.getForeignTUCount() > 0) {
warn() << formatv("Name Index @ {0:x}: Verifying indexes of foreign type "
"units is not currently supported.\n",
NI.getUnitOffset());
return 0;
}
Expand Down Expand Up @@ -1512,10 +1512,12 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) {
NumErrors += verifyNameIndexAttribute(NI, Abbrev, AttrEnc);
}

if (NI.getCUCount() > 1 && !Attributes.count(dwarf::DW_IDX_compile_unit)) {
if (NI.getCUCount() > 1 && !Attributes.count(dwarf::DW_IDX_compile_unit) &&
!Attributes.count(dwarf::DW_IDX_type_unit)) {
ErrorCategory.Report("Abbreviation contains no attribute", [&]() {
error() << formatv("NameIndex @ {0:x}: Indexing multiple compile units "
"and abbreviation {1:x} has no {2} attribute.\n",
"and abbreviation {1:x} has no DW_IDX_compile_unit "
"or DW_IDX_type_unit attribute.\n",
NI.getUnitOffset(), Abbrev.Code,
dwarf::DW_IDX_compile_unit);
});
Expand Down Expand Up @@ -1574,8 +1576,8 @@ static SmallVector<std::string, 3> getNames(const DWARFDie &DIE,
unsigned DWARFVerifier::verifyNameIndexEntries(
const DWARFDebugNames::NameIndex &NI,
const DWARFDebugNames::NameTableEntry &NTE) {
// Verifying type unit indexes not supported.
if (NI.getLocalTUCount() + NI.getForeignTUCount() > 0)
// Verifying foreign type unit indexes not supported.
if (NI.getForeignTUCount() > 0)
return 0;

const char *CStr = NTE.getString();
Expand All @@ -1596,18 +1598,35 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
Expected<DWARFDebugNames::Entry> EntryOr = NI.getEntry(&NextEntryID);
for (; EntryOr; ++NumEntries, EntryID = NextEntryID,
EntryOr = NI.getEntry(&NextEntryID)) {
uint32_t CUIndex = *EntryOr->getCUIndex();
if (CUIndex > NI.getCUCount()) {

std::optional<uint64_t> CUIndex = EntryOr->getCUIndex();
std::optional<uint64_t> TUIndex = EntryOr->getLocalTUIndex();
if (CUIndex && *CUIndex >= NI.getCUCount()) {
ErrorCategory.Report("Name Index entry contains invalid CU index", [&]() {
error() << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an "
"invalid CU index ({2}).\n",
NI.getUnitOffset(), EntryID, CUIndex);
NI.getUnitOffset(), EntryID, *CUIndex);
});
++NumErrors;
continue;
}
uint64_t CUOffset = NI.getCUOffset(CUIndex);
uint64_t DIEOffset = CUOffset + *EntryOr->getDIEUnitOffset();
if (TUIndex && *TUIndex >= NI.getLocalTUCount()) {
ErrorCategory.Report("Name Index entry contains invalid TU index", [&]() {
error() << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an "
"invalid TU index ({2}).\n",
NI.getUnitOffset(), EntryID, *TUIndex);
});
++NumErrors;
continue;
}
std::optional<uint64_t> UnitOffset;
if (TUIndex)
UnitOffset = NI.getLocalTUOffset(*TUIndex);
else if (CUIndex)
UnitOffset = NI.getCUOffset(*CUIndex);
if (!UnitOffset)
continue;
uint64_t DIEOffset = *UnitOffset + *EntryOr->getDIEUnitOffset();
DWARFDie DIE = DCtx.getDIEForOffset(DIEOffset);
if (!DIE) {
ErrorCategory.Report("NameIndex references nonexistent DIE", [&]() {
Expand All @@ -1618,12 +1637,12 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
++NumErrors;
continue;
}
if (DIE.getDwarfUnit()->getOffset() != CUOffset) {
if (DIE.getDwarfUnit()->getOffset() != *UnitOffset) {
ErrorCategory.Report("Name index contains mismatched CU of DIE", [&]() {
error() << formatv(
"Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of "
"DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n",
NI.getUnitOffset(), EntryID, DIEOffset, CUOffset,
NI.getUnitOffset(), EntryID, DIEOffset, *UnitOffset,
DIE.getDwarfUnit()->getOffset());
});
++NumErrors;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
# CHECK: NameIndex @ 0x0: Abbreviation 0x1: DW_IDX_die_offset uses an unknown form: DW_FORM_unknown_1fff.
# CHECK: warning: NameIndex @ 0x0: Abbreviation 0x3 references an unknown tag: DW_TAG_unknown_8080.
# CHECK: error: NameIndex @ 0x0: Abbreviation 0x5 has no DW_IDX_die_offset attribute.
# CHECK: error: NameIndex @ 0x55: Indexing multiple compile units and abbreviation 0x1 has no DW_IDX_compile_unit attribute.
# CHECK: error: NameIndex @ 0x55: Indexing multiple compile units and abbreviation 0x1 has no DW_IDX_compile_unit or DW_IDX_type_unit attribute.

.section .debug_str,"MS",@progbits,1
.Lstring_producer:
Expand Down
116 changes: 116 additions & 0 deletions llvm/test/tools/llvm-dwarfdump/debug-names-local-tu-bad-die.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
# RUN: yaml2obj %s -o - | not llvm-dwarfdump -verify - | FileCheck %s
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Judging by the strings in this file, you've produced this by running (meta:P) clang over some file, and then tweaking the output. Would it be possible to make the same modifications on the intermediate assembly file instead?

I see there is already a precedent for yaml input files in this folder, so I wouldn't say this is a hard requirement, but I think it'd much easier to see what's going on here by looking at the (annotated) assembly than at a raw hex dump. (At this point, I've gotten pretty good at reading dwarf from assembly, but I'm not l33t enough to do it from hex :P)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a linked executable. It doesn't work on .o files with multiple DWARF units. I tried to make one and ended up with:

Verifying /Users/gclayton/Documents/src/github/Debug/test/tools/llvm-dwarfdump/X86/Output/debug-names-verify-tu-bad-die.s.tmp:  file format elf64-x86-64
Verifying .debug_abbrev...
Verifying .debug_info Unit Header Chain...
Verifying .debug_types Unit Header Chain...
Verifying non-dwo Units...
Verifying unit: 1 / 2
Verifying unit: 2 / 2, "main.cpp"
Verifying dwo Units...
Verifying .debug_line...
warning: .debug_line[0x00000000].prologue.file_names[1] is a duplicate of file_names[0]
Verifying .debug_str_offsets...
Verifying .debug_names...
error: Name Index @ 0x0: Entry @ 0x86: mismatched Tag of DIE @ 0x23: index - DW_TAG_subprogram; debug_info - DW_TAG_structure_type.
error: Name Index @ 0x0: Entry @ 0x86: mismatched Name of DIE @ 0x23: index - main; debug_info - Foo.
error: Aggregated error counts:
error: Name Index contains mismatched Tag of DIE occurred 1 time(s).
error: Name Index contains mismatched name of DIE occurred 1 time(s).
Errors detected.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this was for a .o file straight from the compiler. I am guessing llvm-dwarfdump might not be applying relocations, but that is beyond the scope of this patch.

Is there a way to get a .s file for linked executable?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no way to get a .s version of a linked executable. I guess it's possible there's something wrong with the relocations, though I'm somewhat surprised by that since llvm-dwarfdump is reguarly used (both in tests and in real word) on unlinked files. What's the input you used to reproduce this?


# Verifying local type units. Local type units and compile units are identical
# except that the DWARF Unit offset for the type unit is specified using a
# DW_IDX_type_unit. This is a single test that tests if we get an error for a
# local type unit. We only need to verify that errors work for type units and
# we can rely on the test for compile units to verify all other errors that can
# happen in the .debug_names entries.

# CHECK: Verifying .debug_names...
# CHECK: error: Name Index @ 0x0: Entry @ 0x7f references a non-existing DIE @ 0x25.
# CHECK: error: Aggregated error counts:
# CHECK: error: NameIndex references nonexistent DIE occurred 1 time(s).
# Errors detected.

--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_EXEC
Machine: EM_X86_64
ProgramHeaders:
- Type: PT_PHDR
Flags: [ PF_R ]
VAddr: 0x200040
Align: 0x8
Offset: 0x40
- Type: PT_LOAD
Flags: [ PF_R ]
FirstSec: .eh_frame
LastSec: .eh_frame
VAddr: 0x200000
Align: 0x1000
Offset: 0x0
- Type: PT_LOAD
Flags: [ PF_X, PF_R ]
FirstSec: .text
LastSec: .text
VAddr: 0x201160
Align: 0x1000
Offset: 0x160
- Type: PT_GNU_STACK
Flags: [ PF_W, PF_R ]
Align: 0x0
Offset: 0x0
Sections:
- Name: .eh_frame
Type: SHT_PROGBITS
Flags: [ SHF_ALLOC ]
Address: 0x200120
AddressAlign: 0x8
Content: 1400000000000000017A5200017810011B0C0708900100001C0000001C000000201000000F00000000410E108602430D064A0C070800000000000000
- Name: .text
Type: SHT_PROGBITS
Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
Address: 0x201160
AddressAlign: 0x10
Content: 554889E5C745FC0000000031C05DC3
- Name: .debug_info
Type: SHT_PROGBITS
AddressAlign: 0x1
Content: 260000000500020800000000F23532F3E4235D67230000000121000000000008000000020506010001004800000005000108000000000300210001080000000000000002000F0000000800000004000F00000001560300033E0000000502917B05000442000000000604050407F23532F3E4235D6700
- Name: .debug_abbrev
Type: SHT_PROGBITS
AddressAlign: 0x1
Content: 0141011305101772170000021300360B03250B0B3A0B3B0B0000031101252513050325721710171B25111B120673170000042E01111B1206401803253A0B3B0B49133F190000053400021803253A0B3B0B4913000006240003253E0B0B0B00000713003C196920000000
- Name: .debug_str_offsets
Type: SHT_PROGBITS
AddressAlign: 0x1
Content: 20000000050000000400000096000000A80000009F000000A400000000000000D1000000
- Name: .debug_names
Type: SHT_PROGBITS
AddressAlign: 0x4
Content: 900000000500000001000000010000000000000003000000030000001B000000080000004C4C564D303730302A000000000000000100000002000000030000008973880B6A7F9A7C3080880BD10000009F000000A400000000000000070000000D0000000113020B031304190000022E03130419000003240313041900000001002500000000022300000000033E000000000000
- Name: .comment
Type: SHT_PROGBITS
Flags: [ SHF_MERGE, SHF_STRINGS ]
AddressAlign: 0x1
EntSize: 0x1
Content: 46616365626F6F6B20636C616E672076657273696F6E2031352E302E30202868747470733A2F2F6769742E696E7465726E616C2E7466626E772E6E65742F7265706F732F6769742F726F2F6F736D6574612F65787465726E616C2F6C6C766D2D70726F6A656374203864356561396432616431633161356139303862623632343663303261626162323235643562633829004C696E6B65723A204C4C442031392E302E300000
- Name: .debug_line
Type: SHT_PROGBITS
AddressAlign: 0x1
Content: 590000000500080037000000010101FB0E0D00010101010000000100000101011F010900000003011F020F051E01000000000014E7C4DFD187393499D4332D94A1154D040000090260112000000000001405030AAE060B2E0202000101
- Name: .debug_line_str
Type: SHT_PROGBITS
Flags: [ SHF_MERGE, SHF_STRINGS ]
AddressAlign: 0x1
EntSize: 0x1
Content: 6D61696E2E637070002F55736572732F67636C6179746F6E2F446F63756D656E74732F7372632F7665726966792D74757300
Symbols:
- Name: main.cpp
Type: STT_FILE
Index: SHN_ABS
- Name: main
Type: STT_FUNC
Section: .text
Binding: STB_GLOBAL
Value: 0x201160
Size: 0xF
DWARF:
debug_str:
- foo
- 'Facebook clang version 15.0.0 (https://git.internal.tfbnw.net/repos/git/ro/osmeta/external/llvm-project 8d5ea9d2ad1c1a5a908bb6246c02abab225d5bc8)'
- main.cpp
- main
- int
- '/Users/gclayton/Documents/src/verify-tus'
- Foo
debug_addr:
- Length: 0xC
Version: 0x5
AddressSize: 0x8
Entries:
- Address: 0x201160
...
114 changes: 114 additions & 0 deletions llvm/test/tools/llvm-dwarfdump/debug-names-local-tu-bad-tu-index.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# RUN: yaml2obj %s -o - | not llvm-dwarfdump -verify - | FileCheck %s

# Verifying local type units. Local type units and compile units are identical
# except that the DWARF Unit offset for the type unit is specified using a
# DW_IDX_type_unit. This is a single test that tests that the type unit index
# in the DW_IDX_type_unit value is valid.

# CHECK: Verifying .debug_names...
# CHECK: error: Name Index @ 0x0: Entry @ 0x7f contains an invalid TU index (1).
# CHECK: error: Aggregated error counts:
# CHECK: error: Name Index entry contains invalid TU index occurred 1 time(s).
# Errors detected.

--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_EXEC
Machine: EM_X86_64
ProgramHeaders:
- Type: PT_PHDR
Flags: [ PF_R ]
VAddr: 0x200040
Align: 0x8
Offset: 0x40
- Type: PT_LOAD
Flags: [ PF_R ]
FirstSec: .eh_frame
LastSec: .eh_frame
VAddr: 0x200000
Align: 0x1000
Offset: 0x0
- Type: PT_LOAD
Flags: [ PF_X, PF_R ]
FirstSec: .text
LastSec: .text
VAddr: 0x201160
Align: 0x1000
Offset: 0x160
- Type: PT_GNU_STACK
Flags: [ PF_W, PF_R ]
Align: 0x0
Offset: 0x0
Sections:
- Name: .eh_frame
Type: SHT_PROGBITS
Flags: [ SHF_ALLOC ]
Address: 0x200120
AddressAlign: 0x8
Content: 1400000000000000017A5200017810011B0C0708900100001C0000001C000000201000000F00000000410E108602430D064A0C070800000000000000
- Name: .text
Type: SHT_PROGBITS
Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
Address: 0x201160
AddressAlign: 0x10
Content: 554889E5C745FC0000000031C05DC3
- Name: .debug_info
Type: SHT_PROGBITS
AddressAlign: 0x1
Content: 260000000500020800000000F23532F3E4235D67230000000121000000000008000000020506010001004800000005000108000000000300210001080000000000000002000F0000000800000004000F00000001560300033E0000000502917B05000442000000000604050407F23532F3E4235D6700
- Name: .debug_abbrev
Type: SHT_PROGBITS
AddressAlign: 0x1
Content: 0141011305101772170000021300360B03250B0B3A0B3B0B0000031101252513050325721710171B25111B120673170000042E01111B1206401803253A0B3B0B49133F190000053400021803253A0B3B0B4913000006240003253E0B0B0B00000713003C196920000000
- Name: .debug_str_offsets
Type: SHT_PROGBITS
AddressAlign: 0x1
Content: 20000000050000000400000096000000A80000009F000000A400000000000000D1000000
- Name: .debug_names
Type: SHT_PROGBITS
AddressAlign: 0x4
Content: 900000000500000001000000010000000000000003000000030000001B000000080000004C4C564D303730302A000000000000000100000002000000030000008973880B6A7F9A7C3080880BD10000009F000000A400000000000000070000000D0000000113020B031304190000022E03130419000003240313041900000001012500000000022300000000033E000000000000
- Name: .comment
Type: SHT_PROGBITS
Flags: [ SHF_MERGE, SHF_STRINGS ]
AddressAlign: 0x1
EntSize: 0x1
Content: 46616365626F6F6B20636C616E672076657273696F6E2031352E302E30202868747470733A2F2F6769742E696E7465726E616C2E7466626E772E6E65742F7265706F732F6769742F726F2F6F736D6574612F65787465726E616C2F6C6C766D2D70726F6A656374203864356561396432616431633161356139303862623632343663303261626162323235643562633829004C696E6B65723A204C4C442031392E302E300000
- Name: .debug_line
Type: SHT_PROGBITS
AddressAlign: 0x1
Content: 590000000500080037000000010101FB0E0D00010101010000000100000101011F010900000003011F020F051E01000000000014E7C4DFD187393499D4332D94A1154D040000090260112000000000001405030AAE060B2E0202000101
- Name: .debug_line_str
Type: SHT_PROGBITS
Flags: [ SHF_MERGE, SHF_STRINGS ]
AddressAlign: 0x1
EntSize: 0x1
Content: 6D61696E2E637070002F55736572732F67636C6179746F6E2F446F63756D656E74732F7372632F7665726966792D74757300
Symbols:
- Name: main.cpp
Type: STT_FILE
Index: SHN_ABS
- Name: main
Type: STT_FUNC
Section: .text
Binding: STB_GLOBAL
Value: 0x201160
Size: 0xF
DWARF:
debug_str:
- foo
- 'Facebook clang version 15.0.0 (https://git.internal.tfbnw.net/repos/git/ro/osmeta/external/llvm-project 8d5ea9d2ad1c1a5a908bb6246c02abab225d5bc8)'
- main.cpp
- main
- int
- '/Users/gclayton/Documents/src/verify-tus'
- Foo
debug_addr:
- Length: 0xC
Version: 0x5
AddressSize: 0x8
Entries:
- Address: 0x201160
...
Loading