This is an archive of the discontinued LLVM Phabricator instance.

Paths

Table of Contentst

-
llvm/
-
lib/DebugInfo/DWARF/
-
DebugInfo/
-
DWARF/
1
DWARFVerifier.cpp
-
test/tools/llvm-dwarfdump/X86/
-
tools/
-
llvm-dwarfdump/
-
X86/
-
no-children.s

Differential D88048

[dwarfdump] Add verifier check to find DIEs with DW_CHILDREN_yes but without children.
ClosedPublic

Authored by JDevlieghere on Sep 21 2020, 2:41 PM.

Download Raw Diff

Details

Reviewers

aprantl
dblaikie
probinson
jhenderson
jdoerfert

Commits

rGe1ef7183c6f0: [dwarfdump] Warn for tags with DW_CHILDREN_yes but no children.

Summary

Flag DIEs that have DW_CHILDREN_yes set in their abbreviation but don't actually have any children.

rdar://59809554

Diff Detail

Event Timeline

JDevlieghere created this revision.Sep 21 2020, 2:41 PM

Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2020, 2:41 PM

Herald added subscribers: cmtice, hiraditya. · View Herald Transcript

JDevlieghere requested review of this revision.Sep 21 2020, 2:41 PM

Herald added a reviewer: jdoerfert. · View Herald TranscriptSep 21 2020, 2:41 PM

Herald added subscribers: sstefan1, ormris, MaskRay. · View Herald Transcript

JDevlieghere edited the summary of this revision. (Show Details)Sep 21 2020, 2:41 PM

does this classify as a warning? Is there a distinction between ill-formed DWARF and "this is valid, but unlikely to be great"?

In D88048#2286251, @dblaikie wrote:

does this classify as a warning? Is there a distinction between ill-formed DWARF and "this is valid, but unlikely to be great"?

I was on the fence myself, I couldn't find anything conclusive in the standard. I've made it a warning.

maybe slightly long test case - could do this with the CU DIE itself? (perhaps a case where hand-crafted/substantially hand-modified assembly is OK, because it can be made /so/ simple - or maybe yaml2obj dwarf support is adequate for this use case?)

In D88048#2286589, @dblaikie wrote:

maybe slightly long test case - could do this with the CU DIE itself? (perhaps a case where hand-crafted/substantially hand-modified assembly is OK, because it can be made /so/ simple - or maybe yaml2obj dwarf support is adequate for this use case?)

Hi, currently, obj2yaml doesn't work properly when dumping the .debug_abbrev/__debug_abbrev section. We might not be able to get the correct output when there are multiple DIEs in one abbrev table. D87179 has fixed it but hasn't been landed yet. I've created one test case for this patch. Hope it works :-)

--- !ELF
FileHeader:
  Class: ELFCLASS64
  Data:  ELFDATA2LSB
  Type:  ET_EXEC
DWARF:
  debug_abbrev:
    - Table:
        - Tag:      DW_TAG_compile_unit
          Children: DW_CHILDREN_yes
          Attributes:
            - Attribute: DW_AT_low_pc
              Form:      DW_FORM_data4
  debug_info:
    - Version: 4
      Entries:
        - AbbrCode: 1
          Values:
            - Value: 0x1234
        - AbbrCode: 0   ## Terminator for the current DIE.

However, it seems that the assertion isValid() && "must check validity prior to calling" is triggered when the DIE isn't terminated.

--- !ELF
FileHeader:
  Class: ELFCLASS64
  Data:  ELFDATA2LSB
  Type:  ET_EXEC
DWARF:
  debug_abbrev:
    - Table:
        - Tag:      DW_TAG_compile_unit
          Children: DW_CHILDREN_yes
          Attributes:
            - Attribute: DW_AT_low_pc
              Form:      DW_FORM_data4
  debug_info:
    - Version: 4
      Entries:
        - AbbrCode: 1
          Values:
            - Value: 0x1234

See my inline comments.

Higuoxing added inline comments.Sep 21 2020, 7:58 PM

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
176

Warning seems fine to me since it's more an inefficient encoding (an empty list of children) than malformed.

LGTM once everyone else is happy.

This revision is now accepted and ready to land.Sep 22 2020, 9:13 AM

Closed by commit rGe1ef7183c6f0: [dwarfdump] Warn for tags with DW_CHILDREN_yes but no children. (authored by JDevlieghere). · Explain WhySep 23 2020, 10:13 PM

This revision was automatically updated to reflect the committed changes.

JDevlieghere added a commit: rGe1ef7183c6f0: [dwarfdump] Warn for tags with DW_CHILDREN_yes but no children..

In D88048#2286776, @Higuoxing wrote:

In D88048#2286589, @dblaikie wrote:

maybe slightly long test case - could do this with the CU DIE itself? (perhaps a case where hand-crafted/substantially hand-modified assembly is OK, because it can be made /so/ simple - or maybe yaml2obj dwarf support is adequate for this use case?)

Hi, currently, obj2yaml doesn't work properly when dumping the .debug_abbrev/__debug_abbrev section. We might not be able to get the correct output when there are multiple DIEs in one abbrev table. D87179 has fixed it but hasn't been landed yet. I've created one test case for this patch. Hope it works :-)

(I forgot to submit the comment before landing this)

Awesome, thanks a lot, that's a great test case! :-)

Revision Contents

Path

Size

llvm/

lib/

DebugInfo/

DWARF/

DWARFVerifier.cpp

9 lines

test/

tools/

llvm-dwarfdump/

X86/

no-children.s

215 lines

Diff 293257

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

Show First 20 Lines • Show All 166 Lines • ▼ Show 20 Lines

for (unsigned I = 0; I < NumDies; ++I) {

if (Die.getTag() == DW_TAG_null)

continue;

for (auto AttrValue : Die.attributes()) {

NumUnitErrors += verifyDebugInfoAttribute(Die, AttrValue);

NumUnitErrors += verifyDebugInfoForm(Die, AttrValue);

}

if (Die.hasChildren()) {

if (Die.getFirstChild().getTag() == DW_TAG_null) {

HiguoxingUnsubmitted

Not Done

if (Die.hasChildren()) {

- if (Die.getFirstChild().getTag() == DW_TAG_null) {

+ if (!Die.getFirstChild().isValid() ||

+ Die.getFirstChild().getTag() == DW_TAG_null) {

warn() << dwarf::TagString(Die.getTag())

Higuoxing:

error() << dwarf::TagString(Die.getTag())

<< " has DW_CHILDREN_yes but DIE has no children: ";

Die.dump(OS);

NumUnitErrors++;

}

NumUnitErrors += verifyDebugInfoCallSite(Die);

}

DWARFDie Die = Unit.getUnitDIE(/* ExtractUnitDIEOnly = */ false);

if (!Die) {

error() << "Compilation unit without DIE.\n";