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

[sashiko-bot@kernel.org]

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.

> +
> +			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.

> +		}
> +
> +		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.

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