Re: [PATCH v2 2/4] perf dwarf-aux: Add die_has_flex_array() helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The local variable `last_mb` is accessed uninitialized if a struct contains DWARF children but none of them are data members.
- [Low] Missing explicit inclusion of `<string.h>` for `memcpy` usage, violating the POSIX libc headers subsystem rule.
--

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

perf dwarf-aux: Add die_has_flex_array() helper

The die_has_flex_array() returns true when the given type is a compound
type and contains an array at the end.  To prevent an infinite recursion
add a depth field to the internal function.

> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index d7160f87ac7d7..465824e6513eb 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
[ ... ]
> +static bool die_has_flex_array_recurse(Dwarf_Die *parent_die, int depth)
> +{
> +	Dwarf_Die die_mem, last_mb;
> +	int tag = dwarf_tag(parent_die);
> +
> +	if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type)
> +		return false;
> +
> +	/* prevent infinite recursion */
> +	if (depth > MAX_FLEX_ARRAY_RECURSION)
> +		return false;
> +
> +	if (dwarf_child(parent_die, &die_mem))
> +		return false;
> +
> +	do {
> +		if (dwarf_tag(&die_mem) != DW_TAG_member)
> +			continue;
> +
> +		if (tag == DW_TAG_union_type) {
> +			if (is_flex_array_member(&die_mem))
> +				return true;
> +
> +			if (die_get_real_type(&die_mem, &last_mb) &&
> +			    die_has_flex_array_recurse(&last_mb, depth + 1))
> +				return true;
> +		}
> +
> +		if (tag == DW_TAG_structure_type)
> +			memcpy(&last_mb, &die_mem, sizeof(last_mb));
                        ^^^^^^
[Severity: Low]
This isn't a bug, but this memcpy() call relies on implicit inclusion of
<string.h>. Should we explicitly include <string.h> at the top of the file
to ensure musl compatibility?

> +
> +	} while (dwarf_siblingof(&die_mem, &die_mem) == 0);
> +
> +	if (tag == DW_TAG_structure_type) {
> +		if (is_flex_array_member(&last_mb))
                    ^^^^^^^^^^^^^^^^^^^^
[Severity: High]
If a structure has DWARF children (such as DW_TAG_subprogram or
DW_TAG_typedef), but none of them are DW_TAG_member, the memcpy() above is
skipped due to the continue statement.

Will this cause is_flex_array_member() to access an uninitialized
last_mb variable?

> +			return true;
> +
> +		if (die_get_real_type(&last_mb, &die_mem))
> +			return die_has_flex_array_recurse(&die_mem, depth + 1);
> +	}
> +
> +	return false;
> +}

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