Re: [PATCH v2 2/4] perf dwarf-aux: Add die_has_flex_array() helper

[Namhyung Kim <namhyung@kernel.org>]

Hi Masami,

On Tue, Sep 15, 2026 at 08:40:48AM +0900, Masami Hiramatsu wrote:
> On Sun, 13 Sep 2026 23:45:33 -0700
> Namhyung Kim <namhyung@kernel.org> wrote:
> 
> > The die_has_flex_array() returns true when the given type is a compound
> > type and contains an array at the end.  To prevent an infinite recursion
> > add a depth field to the internal function.
> 
> Hi, thanks for this nice extension!
> BTW, I have a comment on this implementation.

Thanks for your review!

> 
> > 
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/dwarf-aux.c | 77 +++++++++++++++++++++++++++++++++++++
> >  tools/perf/util/dwarf-aux.h |  3 ++
> >  2 files changed, 80 insertions(+)
> > 
> > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > index d7160f87ac7d7ab3..465824e6513eb0ac 100644
> > --- a/tools/perf/util/dwarf-aux.c
> > +++ b/tools/perf/util/dwarf-aux.c
> > @@ -2180,3 +2180,80 @@ Dwarf_Die *die_deref_ptr_type(Dwarf_Die *ptr_die, int offset,
> >  
> >  	return die_get_member_type(&type_die, offset, die_mem);
> >  }
> > +
> > +static bool is_flex_array_member(Dwarf_Die *mb_die)
> > +{
> > +	Dwarf_Die type_die;
> > +	Dwarf_Word size;
> > +
> > +	/* get the type of the member */
> > +	if (die_get_real_type(mb_die, &type_die) == NULL)
> > +		return false;
> > +
> > +	if (dwarf_tag(&type_die) != DW_TAG_array_type)
> > +		return false;
> > +
> > +	return dwarf_aggregate_size(&type_die, &size) < 0 || size == 0;
> > +}
> > +
> > +#define MAX_FLEX_ARRAY_RECURSION  256  /* arbitrary */
> > +
> > +static bool die_has_flex_array_recurse(Dwarf_Die *parent_die, int depth)
> > +{
> > +	Dwarf_Die die_mem, last_mb;
> > +	int tag = dwarf_tag(parent_die);
> > +
> > +	if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type)
> > +		return false;
> 
> What happen if the parent_die is "typedef struct {..." ?
> I think you need to use die_get_real_type() here. ;)

Currently I expect callers to do it before calling.  But probably I can
add it to the outer function.

> 
> > +
> > +	/* prevent infinite recursion */
> > +	if (depth > MAX_FLEX_ARRAY_RECURSION)
> > +		return false;
> > +
> > +	if (dwarf_child(parent_die, &die_mem))
> > +		return false;
> > +
> > +	do {
> > +		if (dwarf_tag(&die_mem) != DW_TAG_member)
> > +			continue;
> 
> If the member is a "const" member, you may have to use
> die_get_real_type() to get the actual type.
> (in this case, you need a cursor DIE for dwarf_siblingof()) 

Doesn't it belong to a type of the member?  In my simple test program,
it shows the struct type only has children of members and their types
including const are referenced from the member DIEs.

Also I don't think flexible arrays will be const as they cannot be
initialized.

> 
> > +
> > +		if (tag == DW_TAG_union_type) {
> > +			if (is_flex_array_member(&die_mem))
> > +				return true;
> > +
> > +			if (die_get_real_type(&die_mem, &last_mb) &&
> > +			    die_has_flex_array_recurse(&last_mb, depth + 1))
> > +				return true;
> > +		}
> > +
> > +		if (tag == DW_TAG_structure_type)
> > +			memcpy(&last_mb, &die_mem, sizeof(last_mb));
> 
> To find the last member, I think you'd better check the
> DW_AT_data_member_location and DW_AT_decl_line to ensure the
> DIE is the last member.

Good point.  I can add the member location check.  I'm afraid decl line
may be incorrect if compiler randomized the layout.

So far I haven't seen DWARF data having unsorted members (by location),
but it'd be better to make sure it's sorted.

Thanks,
Namhyung