Re: [PATCH V2 5/6] tools/perf: Add powerpc HTM auxtrace event processing support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Implicit truncation of `__u64` to `size_t` on 32-bit architectures.
- [Low] The `auxtrace_type` field in `struct powerpc_htm` is declared but never initialized.
--

--- Patch [5]: [PATCH V2 5/6] tools/perf: Add powerpc HTM auxtrace event processing support ---
commit 87b8929025f59e2555a76806a3d33736b3cb7f18
Author: Athira Rajeev <atrajeev@linux.ibm.com>

tools/perf: Add powerpc HTM auxtrace event processing support
    
This commit adds the PERF_AUXTRACE_POWERPC_HTM enum value to auxtrace.h and
wires the dispatch in perf_event__process_auxtrace_info() to call
powerpc_htm_process_auxtrace_info(). It also sets the auxtrace_info->type
in htm_info_fill().

> diff --git a/tools/perf/util/powerpc-htm.c b/tools/perf/util/powerpc-htm.c
> new file mode 100644
> index 0000000000000..8d28d02031d6e
> --- /dev/null
> +++ b/tools/perf/util/powerpc-htm.c
> @@ -0,0 +1,115 @@
[ ... ]
> +static void powerpc_htm_dump_event(size_t len)
> +{
> +	const char *color = PERF_COLOR_BLUE;
> +
> +	if (dump_trace) {
> +		color_fprintf(stdout, color,
> +			". ... HTM PMU data: size %zu bytes\n", len);
> +	}
> +}
[ ... ]
> +static int powerpc_htm_process_auxtrace_event(struct perf_session *session __maybe_unused,
> +					      union perf_event *event,
> +					      const struct perf_tool *tool __maybe_unused)
> +{
> +	if (dump_trace)
> +		powerpc_htm_dump_event(event->auxtrace.size);

[Severity: Low]
Does this implicitly truncate event->auxtrace.size on 32-bit architectures?

event->auxtrace.size is a __u64, but powerpc_htm_dump_event() accepts a
size_t. This isn't a critical bug since it's only used for debug output,
but could it result in printing a truncated size if the trace exceeds 4GB?

> +
> +	return 0;
> +}
[ ... ]
> +int powerpc_htm_process_auxtrace_info(union perf_event *event,
> +				      struct perf_session *session)
> +{
> +	struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
> +	struct powerpc_htm *htm;
> +	int err;
> +
> +	if (auxtrace_info->header.size < sizeof(struct perf_record_auxtrace_info) +
> +					 HTM_AUXTRACE_PRIV_FIXED)
> +		return -EINVAL;
> +
> +	htm = zalloc(sizeof(struct powerpc_htm));
> +	if (!htm)
> +		return -ENOMEM;
> +
> +	err = auxtrace_queues__init(&htm->queues);
> +	if (err) {
> +		free(htm);
> +		return err;
> +	}
> +
> +	htm->session = session;
> +	htm->machine = &session->machines.host;

[Severity: Low]
Is htm->auxtrace_type intentionally left uninitialized here?

It is declared in struct powerpc_htm, but unlike other fields, it is not
assigned auxtrace_info->type after the allocation.

> +	htm->auxtrace.process_event = powerpc_htm_process_event;
> +	htm->auxtrace.process_auxtrace_event = powerpc_htm_process_auxtrace_event;
> +	htm->auxtrace.flush_events = powerpc_htm_flush;
> +	htm->auxtrace.free_events = powerpc_htm_free_events;
> +	htm->auxtrace.free = powerpc_htm_free;
> +	session->auxtrace = &htm->auxtrace;
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720105218.14277-1-atrajeev@linux.ibm.com?part=5