Re: [PATCH v2 04/14] perf cs-etm: Use per-CPU queues for per-thread mode

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing memory allocation failure checks for `cs_etm__etmq_get_traceid_queue()` and `machine__findnew_thread()` lead to NULL pointer dereferences.
--

--- Patch [4]: [PATCH v2 04/14] perf cs-etm: Use per-CPU queues for per-thread mode ---
commit 71df2b2d43e19498295801b34c498fd510d905f6
Author: James Clark <[email protected]>

perf cs-etm: Use per-CPU queues for per-thread mode

This commit updates the cs-etm decoder to support multiple threads in
per-thread mode by using a queue for each CPU. It also prevents an
assertion failure caused by all hardware IDs being pushed into queue 0
when a thread migrates across CPUs.

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index d423f1cb408fa..ccf886efff952 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c

[ ... ]

> @@ -2188,6 +2186,27 @@ static int cs_etm__get_data_block(struct cs_etm_queue *etmq)
>  	 */
>  	cs_etm__flush_all_stack(etmq);
>  
> +	/*
> +	 * Per-thread mode still uses a queue for each CPU, but that CPU can run
> +	 * different threads. When the TID from the AUX record on a CPU changes,
> +	 * re-initialize the thread using the AUX record/buffer fragment TID so
> +	 * we can start decoding even if the context ID packet was cropped or
> +	 * they're disabled.
> +	 */
> +	if (etm->per_thread_decoding) {
> +		struct cs_etm_traceid_queue *tidq = cs_etm__etmq_get_traceid_queue(etmq,
> +							CS_ETM_PER_THREAD_TRACEID);
> +
> +		if (thread__tid(tidq->decode_thread) != etmq->buffer->tid) {

[Severity: Medium]
What happens if cs_etm__etmq_get_traceid_queue() returns NULL due to an
allocation failure? Other callers check for a NULL return, but here tidq
is dereferenced immediately. Should there be a check before accessing
tidq->decode_thread?

> +			thread__zput(tidq->frontend_thread);
> +			thread__zput(tidq->decode_thread);
> +			tidq->frontend_thread = machine__findnew_thread(&etm->session->machines.host,
> +									-1, etmq->buffer->tid);
> +			tidq->decode_thread = machine__findnew_thread(&etm->session->machines.host,
> +								      -1, etmq->buffer->tid);

[Severity: Medium]
Can machine__findnew_thread() fail and return NULL? If so, the returned
values are assigned directly to the thread pointers and would be blindly
dereferenced on subsequent block iterations when thread__tid() is called.

Does this code need to handle thread allocation failures?

> +		}
> +	}
> +
>  	return 1;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-james-cs-unformatted-per-thread-fix-v2-0-00c4fd0701b4@linaro.org?part=4