Re: [PATCH] perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch hardcodes the `__probe_ip` argument name in the output format, ignoring the `trace->show_arg_names` configuration.
--

commit 52fd2385bfe746191bd8b95bc76fff9e349f4450
Author: Aaron Tomlin <atomlin@atomlin.com>

perf trace: Fix noise and signed formatting of __probe_ip in bare dynamic probes

This commit hides the implicit __probe_ip field in bare dynamic probes
by default, and formats it as a hexadecimal kernel address when verbose mode
is enabled.

> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 48615ddccd93a..ca0b2680d531b 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c

[ ... ]

> @@ -3234,6 +3234,23 @@ static size_t trace__fprintf_tp_fields(struct trace *trace, struct evsel *evsel,
>  		if (val == 0 && !trace->show_zeros && !arg->show_zero && arg->strtoul != STUL_BTF_TYPE)
>  			continue;
>  
> +		/*
> +		 * __probe_ip is implicitly added to bare dynamic probes.
> +		 * Suppress it by default to avoid cluttering the output.
> +		 * If verbose mode is enabled, ensure it is formatted as a
> +		 * hexadecimal memory address rather than a signed integer.
> +		 */
> +		if (!strcmp(field->name, "__probe_ip")) {
> +			if (!verbose)
> +				continue;
> +
> +			printed += scnprintf(bf + printed, size - printed,
> +					     "%s%s: %#016llx", printed ?
> +					     ", " : "", field->name,
> +					     (unsigned long long)val);

[Severity: Medium]
Does this hardcoded format string bypass the trace->show_arg_names
configuration?

In trace__fprintf_tp_fields(), other arguments dynamically check if their
names should be printed:

        if (trace->show_arg_names)
                printed += scnprintf(bf + printed, size - printed, "%s: ", field->name);

By explicitly formatting "%s%s: %#016llx" using field->name, it looks like
__probe_ip will always print its argument name here, even if perf trace
is executed with --no-arg-names alongside --verbose.

> +			continue;
> +		}
> +
>  		printed += scnprintf(bf + printed, size - printed, "%s", printed ? ", " : "");
>  
>  		if (trace->show_arg_names)

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