Re: [PATCH] libdw: Add dwarf_peel_type. Use it in dwarf_aggregate_size.

[Josh Stone <jistone at redhat dot com>]

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.