Re: [PATCH v5 4/4] perf annotate-data: Adjust type offset for flex-array

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf annotate-data: incorrect flex array adjustment skips elements in struct padding
--

commit b87f5c6310512e6a55305050539cf57984efb087
Author: Namhyung Kim <namhyung@kernel.org>

perf annotate-data: Adjust type offset for flex-array

The flex array members are located beyond the original type size.  Also
it needs to adjust the offset in an array to find a corresponding
element using module operation.  Note that we focus on access to type and
field, so array index is not important.

> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 5dd6c6ec2d424..e9b58a3d587be 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c

[ ... ]

> @@ -1741,6 +1816,7 @@ struct annotated_data_type *find_data_type(struct data_loc_info *dloc)
>  {
>  	struct dso *dso = map__dso(dloc->ms->map);
>  	Dwarf_Die type_die;
> +	struct annotated_data_type *result;
>  
>  	/*
>  	 * The type offset is the same as instruction offset by default.
> @@ -1753,7 +1829,25 @@ struct annotated_data_type *find_data_type(struct data_loc_info *dloc)
>  	if (find_data_type_die(dloc, &type_die) < 0)
>  		return NULL;
>  
> -	return dso__findnew_data_type(dso, &type_die);
> +	result = dso__findnew_data_type(dso, &type_die);
> +	if (result == NULL)
> +		return NULL;
> +
> +	if (result->flex_array && dloc->type_offset > result->self.size) {

[Severity: Medium]
Since result->self.size (derived from DWARF's DW_AT_byte_size) includes
trailing padding for alignment, won't this check skip flex array elements
that fall within the struct's padded size boundary?

For example, if a struct has an 8-byte aligned member and a trailing flex
array of 2-byte structs, the struct size includes up to 6 bytes of trailing
padding. Accesses to indices 1 through 3 would remain `<= result->self.size`
and erroneously skip this modulo adjustment.

Does this cause fill_member_name() to fail matching the unadjusted offset
against the first element's nested fields, truncating the resolved name to
just the outer array name instead of the specific inner field?

> +		struct annotated_member *flex_array = get_flex_array_member(result);
> +
> +		if (flex_array && flex_array->size > 0) {
> +			int offset = dloc->type_offset;
> +
> +			/* adjust offset in the flex array */
> +			offset -= flex_array->offset;
> +			offset %= flex_array->size;
> +			offset += flex_array->offset;
> +
> +			dloc->type_offset = offset;
> +		}
> +	}
> +	return result;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260919063745.48444-1-namhyung@kernel.org?part=4