This is an archive of the discontinued LLVM Phabricator instance.

[dwarfdump] Verify DW_AT_type.
ClosedPublic

Authored by JDevlieghere on Sep 18 2018, 2:14 AM.

Details

Summary

This extends the verifier to catch three new errors:

  1. Missing DW_AT_type attributes for DW_TAG_formal_parameter, DW_TAG_variable and DW_TAG_array_type.
  2. Invalid references for DW_AT_type attribute values. Technically we already detect invalid references, but now we can be more specific.
  3. Valid references for DW_AT_type pointing to a non-type tag.

Diff Detail

Event Timeline

JDevlieghere created this revision.Sep 18 2018, 2:14 AM
aprantl added inline comments.Sep 18 2018, 8:36 AM
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
191

Have you tested this? I have a vague recollection that older versions of clang emitted (void) as no type. You may need to add an exception for DWARF2 or something.

Found it: rdar://problem/13291085 (For everyone else: it's a bugreport about exactly this)

194

It might be nice to define an operator<<(const Die&) for this presumably(?) common case.

probinson added inline comments.Sep 18 2018, 10:31 AM
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
191

@aprantl I am unclear what it means to have a variable/parameter with type void, or an array whose elements are type void? (pointer to void, sure, but then there will be a DW_AT_type.)
Omitting the type is how you describe a function with void return type, but offhand I can't think of any other situation where that should occur.
or do you mean there are older buggy compilers out there and we should tolerate their bugs?

aprantl added inline comments.Sep 18 2018, 10:50 AM
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
191

The bugreport is about a clang producing the type for (void *) as

DW_TAG_pointer_type
NULL

I had thought that newer clangs are emitting this as

DW_TAG_pointer_type
  DW_AT_TYPE [1]

[1] DW_TAG_unspecified_type
        DW_AT_name "void"

but, no, this is still how it is represented, and frankly, changing it doesn't seem to have any benefits.

Since, as you point out, you can't have an array[void] or a variable of type void, my concern doesn't really apply to this patch.

JDevlieghere marked 2 inline comments as done.Sep 19 2018, 1:10 AM
JDevlieghere added inline comments.
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
194

I've added a helper function in r342526. The operator isn't an option here because we have to pass the DIDumpOptions, something I apparently forgot here. Another benefit of the new helper :-)

JDevlieghere marked an inline comment as done.
  • Use the new dump method that passes the correct DI dump options.
  • Update test to expect verbose output.
aprantl accepted this revision.Sep 19 2018, 8:31 AM

LGTM with the outstanding DIDumpOptions fix. Sorry for holding things up :-)

This revision is now accepted and ready to land.Sep 19 2018, 8:31 AM
This revision was automatically updated to reflect the committed changes.

DWARFVerifier.cpp:477:5: warning: this statement may fall through [-Wimplicit-fallthrough=]

}
^

note: here

case DW_AT_type: {
^~~~