Re: [PATCH 10/14] perf c2c: add hierarchy entry creation and lookup functions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Raw pointer assignment of reference-counted `thread` and `map_symbol` structs violates subsystem architectural rules.
--

commit 1ca4db3fb5b5554b1481105028f0603bdc285571
Author: Jiebin Sun <jiebin.sun@intel.com>

perf c2c: add hierarchy entry creation and lookup functions

This commit adds functions for creating and finding entries at each level
of the 3-level function view hierarchy for perf c2c. It introduces
c2c_child_entry__alloc() for allocation and other helper functions to build
the hierarchy.

> diff --git a/tools/perf/ui/browsers/c2c-function.c b/tools/perf/ui/browsers/c2c-function.c
> index a42378f395f59..a881839299e37 100644
> --- a/tools/perf/ui/browsers/c2c-function.c
> +++ b/tools/perf/ui/browsers/c2c-function.c

[ ... ]

> +static struct c2c_hist_entry *
> +c2c_child_entry__alloc(struct hist_entry *parent_he, struct hist_entry *src_he,
> +		       int depth, u64 ip)
> +{
> +	struct c2c_hist_entry *child_c2c;
> +	struct hist_entry *child_he;
> +	size_t callchain_size;
> +
> +	callchain_size = symbol_conf.use_callchain ? sizeof(struct callchain_root) : 0;
> +	child_he = c2c_he_zalloc(callchain_size);
> +	if (!child_he)
> +		return NULL;
> +
> +	child_c2c = container_of(child_he, struct c2c_hist_entry, he);
> +	child_he->callchain_size = callchain_size;
> +	if (callchain_size)
> +		callchain_init(child_he->callchain);
> +
> +	memcpy(&child_he->ms, &src_he->ms, sizeof(struct map_symbol));
> +
> +	if (src_he->mem_info) {
> +		child_he->mem_info = mem_info__clone(src_he->mem_info);
> +		if (!child_he->mem_info)
> +			goto out_free;
> +	}
> +
> +	child_he->thread = src_he->thread;

[Severity: Medium]
Does this code bypass the reference tracking helpers for the thread and
map_symbol structs?

Using memcpy() for map_symbol and raw pointer assignment for the thread
handle circumvents the reference count checking safety mechanisms. Should
this use explicit lifecycle helpers like thread__get() and map__get() to
ensure proper reference count balancing?

> +	child_he->cpumode = src_he->cpumode;
> +	child_he->cpu = src_he->cpu;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626070355.1556721-1-jiebin.sun@intel.com?part=10