Re: [PATCH perf-tools-next v6 1/5] perf trace: Fix error checking in btf_struct_scnprintf()

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

On Mon, Aug 24, 2026 at 09:31:18AM -0400, Aaron Tomlin wrote:
> btf_dump__dump_type_data() returns the positive number of bytes dumped on
> success, or a negative error code (e.g., -EINVAL) on failure.
> 
> Currently, btf_struct_scnprintf() checks if btf_dump__dump_type_data()
> returns 0. When a negative error code is returned on failure, this check
> evaluates to false, causing the function to proceed down the success path
> and advance arg->augmented.args past the unprinted struct data before
> returning 0.
> 
> Consequently, when syscall__scnprintf_args() falls back to the default
> argument beautifier upon receiving 0 from trace__btf_scnprintf(), the
> fallback beautifier or subsequent arguments read from an erroneously
> advanced augmented arguments buffer.
> 
> Fix this by checking for '<= 0' ensuring that on any failure or 0-byte
> dump, 0 is returned without modifying the augmented arguments pointer.
> 
> Fixes: cb32035214b9 ("perf trace: Pretty print augmented struct args using BTF")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
>  tools/perf/builtin-trace.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index c3c7f1f85c53..1bc39f674f11 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -1066,7 +1066,9 @@ static size_t btf_struct_scnprintf(const struct btf_type *type, struct btf *btf,
>  		return 0;
>  
>  	/* pretty print the struct data here */
> -	if (btf_dump__dump_type_data(btf_dump, type_id, arg->augmented.args->value, type->size, &dump_data_opts) == 0)
> +	if (btf_dump__dump_type_data(btf_dump, type_id,
> +				     arg->augmented.args->value,
> +				     type->size, &dump_data_opts) <= 0)
>  		return 0;

Applied the series, only change I made was:

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index c3c7f1f85c530196..4c8f91f0f854b382 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1066,7 +1066,7 @@ static size_t btf_struct_scnprintf(const struct btf_type *type, struct btf *btf,
                return 0;

        /* pretty print the struct data here */
-       if (btf_dump__dump_type_data(btf_dump, type_id, arg->augmented.args->value, type->size, &dump_data_opts) == 0)
+       if (btf_dump__dump_type_data(btf_dump, type_id, arg->augmented.args->value, type->size, &dump_data_opts) <= 0)
                return 0;

        consumed = sizeof(*augmented_arg) + augmented_arg->size;
(END)
acme@number:~/git/pahole$

As this source code allows for longer lines, having it like above helps
in reviewing, we can see straight away what the change was. There was a
clash a few patches later that expected that reflow to be in place, I
adjusted that as well, no code changes in both cases.

Please avoid reflowing things.

Thanks for your work, appreciated!

- Arnaldo