Re: [PATCH 4/6] perf annotate-data: Bound the member nesting recursion

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Fri, Sep 25, 2026 at 08:39:21AM -0700, Namhyung Kim wrote:
> On Fri, Sep 25, 2026 at 05:06:55PM +0200, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > Members are added recursively, and a broken DIE can make a member's
> > type point back at one of its own ancestors, recursing until the stack
> > is gone; nothing usable comes out of nesting members 32 deep anyway, so
> > stop there, marking the member as truncated and giving up on member
> > types that don't resolve.
> > 
> > The check is where the children of an aggregate would be expanded, so
> > only struct/union members are marked as truncated.
> > 
> > Assisted-by: LLM
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  tools/perf/util/annotate-data.c | 20 ++++++++++++++++++--
> >  tools/perf/util/annotate-data.h |  3 +++
> >  2 files changed, 21 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> > index bc9698abc6a9e055..19a6ecd67f28719d 100644
> > --- a/tools/perf/util/annotate-data.c
> > +++ b/tools/perf/util/annotate-data.c
> > @@ -222,6 +222,12 @@ static bool data_type_less(struct rb_node *node_a, const struct rb_node *node_b)
> >  	return strcmp(a->self.type_name, b->self.type_name) < 0;
> >  }
> >  
> > +/*
> > + * A broken type can point back at one of its own ancestors: bound the
> > + * nesting so it doesn't recurse until the stack is gone.
> > + */
> > +#define MAX_MEMBER_DEPTH 32
> > +
> >  /* Recursively add new members for struct/union */
> >  static int __add_member_cb(Dwarf_Die *die, void *arg)
> >  {
> > @@ -236,6 +242,9 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
> >  	if (dwarf_tag(die) != DW_TAG_member)
> >  		return DIE_FIND_CB_SIBLING;
> >  
> > +	if (die_get_real_type(die, &die_mem) == NULL)
> > +		return DIE_FIND_CB_SIBLING;
> > +
> >  	member = zalloc(sizeof(*member));
> >  	if (member == NULL)
> >  		return DIE_FIND_CB_END;
> > @@ -248,8 +257,6 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
> >  	if (die_get_typename(die, &sb) < 0)
> >  		strbuf_add(&sb, "(unknown type)", 14);
> >  
> > -	die_get_real_type(die, &die_mem);
> > -
> >  	if (dwarf_aggregate_size(&die_mem, &size) < 0 || size == 0) {
> >  		if (dwarf_tag(&die_mem) == DW_TAG_array_type) { /* flex-array? */
> >  			die_get_real_type(&die_mem, &die_mem);
> > @@ -299,6 +306,7 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
> >  	}
> >  	member->size = size;
> >  	member->offset = loc + parent->offset;
> > +	member->depth = parent->depth + 1;
> >  	INIT_LIST_HEAD(&member->children);
> >  
> >  	list_for_each_entry_reverse(prev, &parent->children, node) {
> > @@ -313,6 +321,14 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
> >  		member->is_union = true;
> >  		/* fall through */
> >  	case DW_TAG_structure_type:
> > +		/* Only aggregates have children to expand, so only they get truncated. */
> > +		if (member->depth >= MAX_MEMBER_DEPTH) {
> > +			/* Consumed by the JSON exporter added in a later series. */
> > +			member->truncated = true;
> > +			pr_debug_dtp("member nesting limit reached at %s\n",
> > +				     member->type_name ?: "(unknown type)");
> > +			break;
> > +		}
> >  		die_find_child(&die_mem, __add_member_cb, member, &die_mem);
> >  		break;
> >  	default:
> > diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
> > index 14b8113521a927cf..ca0abfb2a036d229 100644
> > --- a/tools/perf/util/annotate-data.h
> > +++ b/tools/perf/util/annotate-data.h
> > @@ -61,6 +61,9 @@ struct annotated_member {
> >  	int size;
> >  	bool is_union;
> >  	bool is_flex_array;
> > +	unsigned int depth;
> > +	/* Children not expanded because the nesting limit was reached */
> > +	bool truncated;
> 
> Nit: it'd be better to switch the order of the last two fields to reduce
> the padding.

I should have used pahole... 8-)

Will fix, thanks!

- Arnaldo