Re: [PATCH perf-tools-next v5 3/5] perf ftrace: Support display of inlined functions in function graph tracer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] perf ftrace: implicit declaration of zalloc due to missing <linux/zalloc.h>
- [Low] perf tests: memory leak of out_buf on test assertion failure
--

commit e3c3c2f995806f10402b965ab958305c3e8ca004
Author: Aaron Tomlin <atomlin@atomlin.com>

perf ftrace: Support display of inlined functions in function graph tracer

This patch introduces the --inline option to perf ftrace for the
function_graph tracer. It resolves return addresses to their inlined
callchains using DWARF debug information and synthesises the intermediate
inlined frames directly into the streamed call graph.

> diff --git a/tools/perf/tests/ftrace.c b/tools/perf/tests/ftrace.c
> index 596cecee844e3..3d0f5748252b5 100644
> --- a/tools/perf/tests/ftrace.c
> +++ b/tools/perf/tests/ftrace.c

[ ... ]

> +static int test_stack_saturation(void)
> +{
> +	struct perf_ftrace ftrace;
> +	char *out_buf = NULL;
> +	size_t out_len = 0;
> +	FILE *out;
> +	int i;

[ ... ]

> +	/* Verify base_func starts at standard 2 spaces indentation without drift */
> +	TEST_ASSERT_VAL("no indentation drift after stack saturation",
> +			strstr(out_buf, "|  base_func() {") != NULL);
> +
> +	free(out_buf);
> +	return TEST_OK;
> +}

[Severity: Low]
Does this code leak out_buf if the assertion fails?

The TEST_ASSERT_VAL macro appears to return early upon failure, which would
cause the execution to bypass the free(out_buf) call at the end of the
function.

> diff --git a/tools/perf/util/ftrace.c b/tools/perf/util/ftrace.c
> index aaf108369c365..09b2fd30a01b7 100644
> --- a/tools/perf/util/ftrace.c
> +++ b/tools/perf/util/ftrace.c

[ ... ]

> +static struct ftrace_cpu_state *get_cpu_state(int cpu)
> +{
> +	if (cpu < 0 || cpu >= FTRACE_MAX_CPUS)
> +		return NULL;
> +
> +	if (!ftrace_cpu_states[cpu])
> +		ftrace_cpu_states[cpu] = zalloc(sizeof(struct ftrace_cpu_state));

[Severity: Medium]
Will this cause a build failure or pointer truncation due to a missing
declaration for zalloc()?

Since the appropriate header providing the prototype for zalloc() does not
seem to be explicitly included, the compiler might implicitly assume it
returns an int. 

On 64-bit architectures, this could truncate the returned pointer.
Alternatively, modern compilers configured to warn about implicit function
declarations may fail the build entirely.

> +
> +	return ftrace_cpu_states[cpu];
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015011155.414667-1-atomlin@atomlin.com?part=3