On 10/10/2014 11:42 AM, Roland McGrath wrote:
The description in your prologue and the libdw.h comment do not match
what
the code actually does. You describe it as going until it hits one of the
known categories of actual type. But what it actually does is go only as
long as it is seeing one of the known categories of wrapper type. I'm not
really sure which it should do (or if it should instead do something
different from either), but the description should match the behavior.
As described, it would fall down if a new tag were introduced for a new
category of actual type. As implemented, it would fall down if a new tag
were introduced for a new category of wrapper type.
I think peeling through something unrecognized is worse than stopping,
so I'd update the description to match the current code.
FWIW, DWARF4 Figure 15 (under section 5.2) lists type modifier tags. Of
those those we should peel through const, restrict, and volatile, and
stop at pointer, ref, and rvalue_ref. I'm not sure about packed or
shared though. It's odd to me that typedef didn't make this list, but
it should be peeled too.
A third potential approach would be to attempt future-proofing for
those
cases. That is, just keep going as long as there is a DW_AT_type
attribute. But that would need a special case for DW_TAG_pointer_type and
DW_TAG_reference_type, where you want to stop even though it has a
DW_AT_type referring to another type. I can't think of any other cases
where an "actual" type has a DW_AT_type, but there might well be some. If
any new tag were like DW_TAG_pointer_type rather than like
DW_TAG_const_type et al, then this approach would fall down there.
Going through DWARF4 appendix A for those DW_TAG_*_type which suggest
DW_AT_type, I'd also exclude these from peeling:
DW_TAG_array_type
DW_TAG_enumeration_type
DW_TAG_ptr_to_member_type
DW_TAG_set_type
DW_TAG_subrange_type
DW_TAG_subroutine_type
So firstly we need to decide which kinds of future addition we expect
and
thus how to handle the future-proofing. Then we need to have descriptions
and implementation that match.
IMO, a blacklist of what not to peel is more dangerous to the future
than a whitelist of what we know can be peeled.