Re: [PATCH] perf trace: Format fields with hex specifiers in print_fmt as hexadecimal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] field_has_hex_fmt() prematurely truncates the format string search when it encounters escaped quotes.
--

--- Patch [1]: [PATCH] perf trace: Format fields with hex specifiers in print_fmt as hexadecimal ---
commit 1d9edf5adf3cf666d7e9393e5db036cd73d6656a
Author: Aaron Tomlin <atomlin@atomlin.com>

perf trace: Format fields with hex specifiers in print_fmt as hexadecimal

This commit introduces field_has_hex_fmt to inspect tracepoint print_fmt
strings. If a field uses hexadecimal specifiers, it assigns SCA_PTR formatting
to display the value in hexadecimal by default.

> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index ba0f8749fc7d7..b0ce9c2a24e57 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
[ ... ]
> @@ -2098,6 +2098,37 @@ static bool is_internal_field(struct tep_format_field *field)
>  	return !strcmp(field->type, "__data_loc char[]");
>  }
>  
> +static bool field_has_hex_fmt(struct tep_format_field *field, int len)
> +{
> +	const char *fmt, *pos, *end;
> +
> +	if (!field || !field->event || !field->event->print_fmt.format)
> +		return false;
> +
> +	fmt = field->event->print_fmt.format;
> +
> +	/* Limit scanning strictly to the quoted printf format string */
> +	if (*fmt == '"') {
> +		fmt++;
> +		end = strchr(fmt, '"');

[Severity: Medium]
Does this parsing correctly handle escaped quotes inside the format string?

If a tracepoint format string contains an escaped quote, this strchr call
will incorrectly match it as the end of the format string, causing the end
pointer to be prematurely truncated.

> +	} else {
> +		end = strchr(fmt, ',');
> +	}
> +
> +	for (pos = strstr(fmt, field->name); pos && (!end || pos < end); pos = strstr(pos + 1, field->name)) {

[Severity: Medium]
Because the end pointer can be truncated by an escaped quote earlier, won't
the pos < end loop condition evaluate to false for any fields positioned after
that escaped quote?

This seems like it would skip hex format detection for those subsequent fields,
causing them to fall back to default decimal formatting.

> +		if (pos == fmt || !(isalnum(pos[-1]) || pos[-1] == '_')) {
> +			const char *after = pos + len;
[ ... ]

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