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

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Fri, Jun 12, 2026 at 10:16:01PM +0000, sashiko-bot@kernel.org wrote:
> 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.

I think this is a valid concern, please make it follow the
trace->show_arg_names for consistency.

This is a nice improvement, thanks for working on it,

- Arnaldo
 
> > +			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