Re: [PATCH v3 08/14] perf c2c: add HPP list parsing for function view histograms

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Calling `perf_hpp__setup_output_field()` with a local list inadvertently pollutes the global `perf_hpp_list` and fails to populate the local list's output fields with the missing sort keys. This causes a type confusion/out-of-bounds read and completely breaks the newly added function view.
--

--- Patch [8]: [PATCH v3 08/14] perf c2c: add HPP list parsing for function view histograms ---
commit f3ca20deabb9c494811fc6797c416d4130121e41
Author: Jiebin Sun <jiebin.sun@intel.com>

perf c2c: add HPP list parsing for function view histograms

This adds the histogram initialization and column parsing infrastructure
for the function view in perf c2c. It provides functions to parse column
strings and initialize histograms with custom dimensions.

> diff --git a/tools/perf/ui/browsers/c2c-function.c b/tools/perf/ui/browsers/c2c-function.c
> index d79d131b1f18c..487f9a68d4a43 100644
> --- a/tools/perf/ui/browsers/c2c-function.c
> +++ b/tools/perf/ui/browsers/c2c-function.c

[ ... ]

> +static int
> +function_hpp_list__parse(struct perf_hpp_list *hpp_list,
> +			 const char *output_str,
> +			 const char *sort_str,
> +			 struct perf_env *env)
> +{

[ ... ]

> +	ret = function_hpp_list__add_tokens(hpp_list, sort, env,
> +					    c2c_function_hists__init_sort);
> +	if (ret)
> +		goto out;
> +
> +	perf_hpp__setup_output_field(hpp_list);

[Severity: High]
This is a pre-existing issue in the core formatting infrastructure, but
will calling perf_hpp__setup_output_field() here fail to populate the local
list's output fields?

The underlying function in tools/perf/ui/hist.c hardcodes registration to
the global perf_hpp_list. This causes the local list to remain empty, while
polluting the global list with local dynamic formats.

This also triggers a type confusion and out-of-bounds read. When a standard
format is compared to a c2c_fmt in perf_hpp__setup_output_field(),
c2c_fmt_equal() casts the standard format to c2c_fmt and reads the
out-of-bounds dim pointer.

> +out:
> +	if (ret)
> +		perf_hpp__reset_output_field(hpp_list);
> +	free(output);
> +	free(sort);
> +	return ret;
> +}
> +
> +static __maybe_unused int
> +c2c_function_hists__init(struct c2c_hists *hists,
> +			 const char *sort,
> +			 int nr_header_lines,
> +			 struct perf_env *env)
> +{
> +	__hists__init(&hists->hists, &hists->list);
> +
> +	perf_hpp_list__init(&hists->list);
> +
> +	hists->list.nr_header_lines = nr_header_lines;
> +
> +	return function_hpp_list__parse(&hists->list, /*output=*/NULL, sort, env);

[Severity: High]
Because this passes NULL for the output format string and relies entirely on
perf_hpp__setup_output_field() to populate the missing sort keys, won't the
new function view fail to render any columns?

> +}

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