Re: [PATCH v3 1/3] perf dwarf-aux: Add die_has_flex_array() helper

[Namhyung Kim <namhyung@kernel.org>]

On Tue, Sep 15, 2026 at 06:50:26AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] perf dwarf-aux: incorrect offset comparison masks flex arrays after empty structs
> - [Medium] perf dwarf-aux: missing DW_AT_data_bit_offset support causes member offset collapse
> - [Medium] perf dwarf-aux: top-level type resolution drops const/volatile modifiers
> --
> 
> commit 93f74aec3d433215197c18f94befb7b584381fa4
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf dwarf-aux: Add die_has_flex_array() helper
> 
> This commit adds the die_has_flex_array() helper to identify if a given
> DWARF type is a compound type containing a flexible array at the end. It
> also introduces a depth parameter to the internal recursive function to
> prevent infinite recursion.
> 
> > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > index d7160f87ac7d7..a5aefe7d1d62d 100644
> > --- a/tools/perf/util/dwarf-aux.c
> > +++ b/tools/perf/util/dwarf-aux.c
> 
> [ ... ]
> 
> > +		if (tag == DW_TAG_structure_type) {
> > +			if (die_get_data_member_location(&die_mem, &loc) < 0)
> > +				loc = 0;
> 
> [Severity: Medium]
> Will this fallback to 0 corrupt the tracking of the final member if the
> struct contains members using DWARF 5 DW_AT_data_bit_offset?
> 
> When die_get_data_member_location() returns < 0 for bit-offset attributes,
> setting loc = 0 silently overrides the actual offset. This could cause the
> maximum-offset calculation to fail.

I don't think we are interested in bitfield members.  I suspect it's not
possible to have a flexible array with bitfields?

> 
> > +
> > +			if (!found || last_loc < loc) {
> > +				memcpy(&last_mb, &die_mem, sizeof(last_mb));
> > +				last_loc = loc;
> > +			}
> 
> [Severity: Medium]
> Does this strict inequality prevent the tracking variable from updating to
> the true final member if multiple members share the same maximum offset?
> 
> If a struct ends with an empty struct followed by a flexible array, both
> members will have the exact same byte offset. Using a strict less-than
> comparison means the tracking variable won't update to the flexible array.
> This could lead to false negatives when analyzing binaries built with GNU C
> empty struct extensions.

Fair enough, will update with the same offset too.

> 
> > +		}
> > +
> > +		found = true;
> > +	} while (dwarf_siblingof(&die_mem, &die_mem) == 0);
> 
> [ ... ]
> 
> > +bool die_has_flex_array(Dwarf_Die *type_die)
> > +{
> > +	Dwarf_Die real_type;
> > +
> > +	if (dwarf_tag(type_die) == DW_TAG_typedef) {
> > +		if (die_get_real_type(type_die, &real_type) == NULL)
> > +			return false;
> > +
> > +		type_die = &real_type;
> > +	}
> > +
> > +	return die_has_flex_array_recurse(type_die, 0);
> > +}
> 
> [Severity: Medium]
> Does this type stripping logic ignore standard qualifiers like const or
> volatile?
> 
> By explicitly limiting type resolution to DW_TAG_typedef, variables
> declared as const struct foo or volatile struct foo will pass a
> DW_TAG_const_type or DW_TAG_volatile_type directly to
> die_has_flex_array_recurse(). The recursive function will then reject it
> because its tag is not a struct or union, causing flexible arrays in
> qualified types to be silently ignored.

Ok, I'll skip qualifiers as well.

Thanks,
Namhyung