Re: [PATCH v6 5/9] perf c2c: add function view stats merge and memory management

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Pruning level-2 writers leaves dangling pointers in `hroot_in` tree.
--

--- Patch [5]: [PATCH v6 5/9] perf c2c: add function view stats merge and memory management ---
commit 0e7c26167764aee56c787657f5359beaaecb5f75
Author: Jiebin Sun <jiebin.sun@intel.com>

perf c2c: add function view stats merge and memory management

Add the per-entry stats/cstats aggregation helpers and hierarchy teardown.
Child common fields are released through hist_entry__delete(), while the
function-view free callback handles the private child tree and containing
allocation. Also add a helper for pruning writer entries with no stores or
cacheline children.

> diff --git a/tools/perf/ui/browsers/c2c-function.c b/tools/perf/ui/browsers/c2c-function.c
> index 7e26c4d1147f4..fc9a4c4bcb03f 100644
> --- a/tools/perf/ui/browsers/c2c-function.c
> +++ b/tools/perf/ui/browsers/c2c-function.c

[ ... ]

> @@ -683,6 +683,168 @@ c2c_function_hists__reinit(struct c2c_hists *c2c_hists,

[ ... ]

> +static int __maybe_unused c2c_he__prune_empty_writers(struct hist_entry *l1_he)
> +{
> +	struct rb_node *nd;
> +	int surviving = 0;
> +
> +	if (!l1_he->has_children)
> +		return 0;
> +
> +	nd = rb_first_cached(&l1_he->hroot_out);
> +	while (nd) {
> +		struct rb_node *next = rb_next(nd);
> +		struct hist_entry *l2_he = rb_entry(nd, struct hist_entry, rb_node);
> +
> +		if (l2_he->has_children && hist_entry__displayed_stores(l2_he) > 0) {
> +			surviving++;
> +		} else {
> +			rb_erase_cached(&l2_he->rb_node, &l1_he->hroot_out);
> +			hist_entry__delete(l2_he);

[Severity: Medium]
Will this leave a dangling pointer in the parent's hroot_in tree?

When hist_entry__delete() frees l2_he in c2c_he__prune_empty_writers(), it
appears to only be unlinked from l1_he->hroot_out. If l1_he->hroot_in is
subsequently traversed (for example, during a TUI resort operation like
hists__hierarchy_output_resort()), could this lead to a use-after-free
crash when dereferencing the freed l2_he pointer?

> +		}
> +		nd = next;
> +	}
> +
> +	if (!surviving) {
> +		l1_he->hroot_out = RB_ROOT_CACHED;
> +		l1_he->has_children = false;
> +		l1_he->unfolded = false;
> +	}
> +	return surviving;
> +}

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