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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] perf: flex-array offset adjustment skips elements in tail padding
- [Medium] perf: zero-length flex-arrays bypass element size extraction
- [Medium] perf: null pointer dereference in find_flex_array() on alloc failure
--

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

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

This commit adjusts the type offset for flexible array members in perf's
data annotation. It ensures that the offset is properly adjusted to find
the corresponding element using modulo operations and updates the member
name resolution to correctly identify flex arrays.

> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 1ee2b74f99205..1c19b807c4f21 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -241,7 +241,8 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
>  		return DIE_FIND_CB_END;
>  
>  	strbuf_init(&sb, 32);
> -	die_get_typename(die, &sb);
> +	if (die_get_typename(die, &sb) < 0)
> +		strbuf_add(&sb, "(unknown type)", 14);
>  
>  	__die_get_real_type(die, &member_type);
>  	if (dwarf_tag(&member_type) == DW_TAG_typedef)

[Severity: Medium]
Does this code safely handle zero-length flexible arrays during element size
extraction?

Just below this change in __add_member_cb(), the logic assumes failures
for flexible arrays:

	if (dwarf_aggregate_size(&die_mem, &size) < 0) {
		if (dwarf_tag(&die_mem) == DW_TAG_array_type) {
			die_get_real_type(&die_mem, &die_mem);
			...

Since dwarf_aggregate_size() returns 0 for zero-length arrays, the < 0
condition is skipped. This disables offset adjustment for all GNU zero-length
arrays (e.g., [0]), which results in incorrect field annotations.

[ ... ]
> @@ -355,15 +377,37 @@ static int fill_member_name(char *buf, size_t sz, struct annotated_member *m,
[ ... ]
> +static struct annotated_member *find_flex_array(struct annotated_member *m)
> +{
> +	struct annotated_member *child;
> +
> +	if (list_empty(&m->children))
> +		return NULL;
> +
> +	child = list_last_entry(&m->children, struct annotated_member, node);
> +	if (strstr(child->type_name, "[]"))

[Severity: Medium]
Could child->type_name be NULL here?

If strbuf_init() or strbuf_add() fails due to memory pressure during type
string construction, strbuf_detach() returns NULL. Passing an unchecked NULL
pointer to strstr() in find_flex_array() could cause a segmentation fault in
the perf tool.

> +		return child;
> +
> +	return find_flex_array(child);
> +}
[ ... ]
> @@ -1740,6 +1784,7 @@ struct annotated_data_type *find_data_type(struct data_loc_info *dloc)
[ ... ]
> -	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]
Does this condition skip elements located in the tail padding of the struct?

By checking that dloc->type_offset > result->self.size, the offset adjustment
is skipped if the flexible array element being accessed has an offset less
than or equal to the total padded size of the parent struct. 

The unadjusted offset fails to match children in fill_member_name(),
erroneously falling back to the last member during offline perf data analysis.

> +		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/20260915064035.1970175-1-namhyung@kernel.org?part=3