Re: [PATCH v2 1/3] perf dwarf-aux: Add die_is_compound_type() to handle C++ class types

[Namhyung Kim <namhyung@kernel.org>]

On Wed, Sep 30, 2026 at 09:10:51PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] perf/dwarf-aux: stack overflow in __die_find_member_offset_cb via cyclic inheritance
> - [Medium] perf/dwarf-aux: flex array detection fails for C++ classes
> --
> 
> commit 3f3b9b56b2e28f1713af3b40ab8946d5bbc8d245
> Author: Yanbo Zhao <yzhao62@ncsu.edu>
> 
> perf dwarf-aux: Add die_is_compound_type() to handle C++ class types
> 
> This commit introduces die_is_compound_type() to support C++ classes alongside
> structs and unions. It also updates offset lookups to descend into base class
> subobjects using DW_TAG_inheritance entries.
> 
> > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > index 54f8b5ec74a2f..eb4b8f3475df1 100644
> > --- a/tools/perf/util/dwarf-aux.c
> > +++ b/tools/perf/util/dwarf-aux.c
> > @@ -61,6 +61,14 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die)
> >  	return dwarf_formstring(&attr);
> >  }
> >  
> > +bool die_is_compound_type(Dwarf_Die *type_die)
> > +{
> > +	int tag = dwarf_tag(type_die);
> > +
> > +	return tag == DW_TAG_structure_type || tag == DW_TAG_union_type ||
> > +	       tag == DW_TAG_class_type;
> > +}
> 
> [Severity: Medium]
> Does die_has_flex_array_recurse() also need to be updated to use this new
> helper?
> 
> If we look at die_has_flex_array_recurse() in tools/perf/util/dwarf-aux.c,
> it currently forces flex array detection to return false for C++ classes
> because it explicitly ignores DW_TAG_class_type:
> 
>     if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type)
>         return false;
>     ...
>     if (tag == DW_TAG_structure_type && found) {
>         if (is_flex_array_member(&last_mb))
>             return true;
> 
> Could this cause accesses to flex array members inside C++ classes to be
> wrongly rejected as out-of-bounds?

IIUC C++ officially doesn't support flexible arrays and there are
better alternatives so I guess it's rare to see them in C++ classes.
Maybe it's ok to ignore them until we find it's really needed.

> 
> [ ... ]
> 
> > @@ -2164,6 +2180,30 @@ static int __die_find_member_offset_cb(Dwarf_Die *die_mem, void *arg)
> >  			loc = 0;
> >  	}
> >  
> > +	if (tag == DW_TAG_inheritance) {
> > +		Dwarf_Die base_die, member_die;
> > +
> > +		/*
> > +		 * A base class subobject can be smaller than the size of the
> > +		 * class type: an empty base takes no space (EBO) and a member
> > +		 * of the derived class can be placed in the tail padding of
> > +		 * the base.  In both cases a member of the derived class is
> > +		 * at the same offset as the base, so only match the base if
> > +		 * it actually has a member at the offset.
> > +		 */
> > +		if (offset < loc)
> > +			return DIE_FIND_CB_SIBLING;
> > +
> > +		if (die_get_real_type(die_mem, &base_die) == NULL)
> > +			return DIE_FIND_CB_SIBLING;
> > +
> > +		if (die_find_child(&base_die, __die_find_member_offset_cb,
> > +				   (void *)(long)(offset - loc), &member_die))
> > +			return DIE_FIND_CB_END;
> 
> [Severity: High]
> Does this recursive call into the DWARF child tree lack a depth limit?
> 
> Unlike __add_member_cb(), there is no recursion depth limit tracked or
> enforced here. If a maliciously crafted or corrupted DWARF file contains
> cyclic inheritance (e.g., A inherits from B, B inherits from A), could
> this trigger unbounded mutual recursion between die_find_child() and
> __die_find_member_offset_cb(), rapidly exhausting the stack?

It looks like a corner case which requires some refactoring.  I feel
like we can add it in a separate patch.

Thanks,
Namhyung