[PATCH 0/3] perf cs-etm: Fix GCC 15 build failure

[PATCH 0/3] perf cs-etm: Fix GCC 15 build failure

[Leo Yan]

This series fixes the GCC 15 build failure reported by Ian Rogers [1]
(thanks, Ian!).

Two follow-up refactoring patches use the packet’s recorded instruction
size for stack events and centralize error checking for frontend
instruction reads.

[1] https://lore.kernel.org/linux-perf-users/20261030-perf_cs_etm_fix_non_taken-v3-0-7629b52ab047@arm.com/T/#m64fb81c665214b79883d9be815b40ac5f6669628

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
Leo Yan (3):
      perf cs-etm: Initialize the Thumb instruction buffer
      perf cs-etm: Use the packet instruction size for stack events
      perf cs-etm: Check frontend instruction reads

 tools/perf/util/cs-etm.c | 83 +++++++++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 32 deletions(-)
---
base-commit: 4af65363b11fc4fe65fcb497f68bbe4f3e3a4d2c
change-id: 20261113-perf_fix_gcc15_failure-1e31233be802

Best regards,
-- 
Leo Yan <leo.yan@arm.com>

[PATCH 1/3] perf cs-etm: Initialize the Thumb instruction buffer

[Leo Yan]

cs_etm__frontend_mem_access() can return zero or a short read without
fully initializing instrBytes. cs_etm__t32_instr_size() nevertheless
inspects its second byte. With CORESIGHT=1 and -O3, GCC 15.2.0 reports:

  error: 'instrBytes' may be used uninitialized

Zero-initialize the buffer to avoid reading uninitialized stack data
when instruction memory is unavailable and fix the build failure.

Reported-by: Ian Rogers <irogers@google.com>
Closes: https://lore.kernel.org/linux-perf-users/20261030-perf_cs_etm_fix_non_taken-v3-0-7629b52ab047@arm.com/T/#m64fb81c665214b79883d9be815b40ac5f6669628
Fixes: a7ee4d625ede ("perf cs-etm: Support for ARM A32/T32 instruction sets in CoreSight trace")
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/cs-etm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 3e01b401ca6f3a08f44bb69204317efaf2fa18d0..25f12bd110934a204d79244f45317085b7a81666 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1369,7 +1369,7 @@ static inline int cs_etm__t32_instr_size(struct cs_etm_queue *etmq,
 					 struct cs_etm_traceid_queue *tidq,
 					 struct cs_etm_packet *packet, u64 addr)
 {
-	u8 instrBytes[2];
+	u8 instrBytes[2] = { 0 };
 
 	cs_etm__frontend_mem_access(etmq, tidq, packet, addr,
 				    ARRAY_SIZE(instrBytes), instrBytes);

-- 
2.34.1

[PATCH 2/3] perf cs-etm: Use the packet instruction size for stack events

[Leo Yan]

cs_etm__add_stack_event() uses the last executed instruction of the
previous packet as the branch source. The decoder already records this
instruction's size in last_instr_size, including for T32 instructions.

Use the recorded size directly instead of reading instruction memory
again. This removes an unnecessary read and the dependency on DSO data
when updating the thread stack.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/cs-etm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 25f12bd110934a204d79244f45317085b7a81666..312d4a734e84faa0d24c4ac1fee16f4dae4c2398 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1626,7 +1626,6 @@ static void cs_etm__add_stack_event(struct cs_etm_queue *etmq,
 {
 	struct cs_etm_auxtrace *etm = etmq->etm;
 	u64 from, to;
-	int size;
 
 	if (!etm->synth_opts.branches && !etm->synth_opts.instructions)
 		return;
@@ -1638,11 +1637,10 @@ static void cs_etm__add_stack_event(struct cs_etm_queue *etmq,
 		from = cs_etm__last_executed_instr(tidq->prev_packet);
 		to = cs_etm__first_executed_instr(tidq->packet);
 
-		size = cs_etm__instr_size(etmq, tidq, tidq->prev_packet, from);
-
 		/* Enable callchain so thread stack entry can be allocated */
 		thread_stack__event(tidq->frontend_thread, tidq->prev_packet->cpu,
-				    tidq->prev_packet->flags, from, to, size,
+				    tidq->prev_packet->flags, from, to,
+				    tidq->prev_packet->last_instr_size,
 				    etmq->buffer->buffer_nr + 1,
 				    etmq->etm->use_callchain,
 				    tidq->br_stack_sz, 0);

-- 
2.34.1

[PATCH 3/3] perf cs-etm: Check frontend instruction reads

[Leo Yan]

Frontend instruction decoding needs a complete memory read rather than
a byte count. Refactor cs_etm__frontend_mem_access() to return zero for
a complete read and -EIO otherwise, while retaining the decoder memory
callback's byte-count interface.

Propagate read errors through Thumb instruction size and address
calculation. Skip samples whose addresses cannot be determined while
preserving instruction period accounting, branch synthesis and packet
processing.

Initialize the copied instruction length to zero, validate the size,
and set the length only after reading all instruction bytes. Also check
read errors before recognizing SVC instructions.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/cs-etm.c | 75 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 27 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 312d4a734e84faa0d24c4ac1fee16f4dae4c2398..303e16a54445bc62ce1c7d0e4ffeb1819211fd40 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1218,13 +1218,16 @@ static u32 __cs_etm__mem_access(struct cs_etm_queue *etmq,
 	return ret;
 }
 
-static u32 cs_etm__frontend_mem_access(struct cs_etm_queue *etmq,
+static int cs_etm__frontend_mem_access(struct cs_etm_queue *etmq,
 				       struct cs_etm_traceid_queue *tidq,
 				       struct cs_etm_packet *packet,
 				       u64 address, size_t size, u8 *buffer)
 {
-	return __cs_etm__mem_access(etmq, address, size, buffer, 0, packet->el,
-				    tidq->frontend_thread);
+	if (__cs_etm__mem_access(etmq, address, size, buffer, 0, packet->el,
+				 tidq->frontend_thread) != size)
+		return -EIO;
+
+	return 0;
 }
 
 static u32 cs_etm__decoder_mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id,
@@ -1370,9 +1373,12 @@ static inline int cs_etm__t32_instr_size(struct cs_etm_queue *etmq,
 					 struct cs_etm_packet *packet, u64 addr)
 {
 	u8 instrBytes[2] = { 0 };
+	int ret;
 
-	cs_etm__frontend_mem_access(etmq, tidq, packet, addr,
-				    ARRAY_SIZE(instrBytes), instrBytes);
+	ret = cs_etm__frontend_mem_access(etmq, tidq, packet, addr,
+					  ARRAY_SIZE(instrBytes), instrBytes);
+	if (ret)
+		return ret;
 	/*
 	 * T32 instruction size is indicated by bits[15:11] of the first
 	 * 16-bit word of the instruction: 0b11101, 0b11110 and 0b11111
@@ -1429,7 +1435,11 @@ static inline u64 cs_etm__instr_addr(struct cs_etm_queue *etmq,
 		return addr + offset * 4;
 
 	while (offset) {
-		addr += cs_etm__instr_size(etmq, tidq, packet, addr);
+		int size = cs_etm__instr_size(etmq, tidq, packet, addr);
+
+		if (size < 0)
+			return CS_ETM_INVAL_ADDR;
+		addr += size;
 		offset--;
 	}
 	return addr;
@@ -1560,31 +1570,31 @@ static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
 			      struct cs_etm_packet *packet,
 			      struct perf_sample *sample)
 {
+	int size;
+
+	sample->insn_len = 0;
+
 	/*
 	 * It's pointless to read instructions for the CS_ETM_DISCONTINUITY
 	 * packet, so directly bail out with 'insn_len' = 0.
 	 */
-	if (packet->sample_type == CS_ETM_DISCONTINUITY) {
-		sample->insn_len = 0;
+	if (packet->sample_type == CS_ETM_DISCONTINUITY)
 		return;
-	}
 
-	sample->insn_len = cs_etm__instr_size(etmq, tidq, packet, sample->ip);
+	size = cs_etm__instr_size(etmq, tidq, packet, sample->ip);
 
 	/*
 	 * The instruction length should always fit within sample->insn,
 	 * but check it to prevent an out-of-bounds write.
 	 */
-	if (sample->insn_len > sizeof(sample->insn)) {
-		sample->insn_len = 0;
+	if (size <= 0 || (size_t)size > sizeof(sample->insn))
 		return;
-	}
 
-	if (packet->sample_type == CS_ETM_EXCEPTION && !sample->insn_len)
+	if (cs_etm__frontend_mem_access(etmq, tidq, packet, sample->ip,
+					size, (void *)sample->insn))
 		return;
 
-	cs_etm__frontend_mem_access(etmq, tidq, packet, sample->ip,
-				    sample->insn_len, (void *)sample->insn);
+	sample->insn_len = size;
 }
 
 u64 cs_etm__convert_sample_time(struct cs_etm_queue *etmq, u64 cs_timestamp)
@@ -2011,11 +2021,13 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
 			 */
 			addr = cs_etm__instr_addr(etmq, tidq, tidq->packet,
 						  offset - 1);
-			ret = cs_etm__synth_instruction_sample(
-				etmq, tidq, tidq->packet, addr,
-				etm->instructions_sample_period);
-			if (ret)
-				return ret;
+			if (addr != CS_ETM_INVAL_ADDR) {
+				ret = cs_etm__synth_instruction_sample(
+					etmq, tidq, tidq->packet, addr,
+					etm->instructions_sample_period);
+				if (ret)
+					return ret;
+			}
 
 			offset += etm->instructions_sample_period;
 			tidq->period_instructions -=
@@ -2227,8 +2239,11 @@ static bool cs_etm__is_svc_instr(struct cs_etm_queue *etmq,
 		 * so below only read 2 bytes as instruction size for T32.
 		 */
 		addr = ret_addr - 2;
-		cs_etm__frontend_mem_access(etmq, tidq, packet, addr,
-					    sizeof(instr16), (u8 *)&instr16);
+
+		if (cs_etm__frontend_mem_access(etmq, tidq, packet, addr,
+						sizeof(instr16), (u8 *)&instr16))
+			return false;
+
 		if ((instr16 & 0xFF00) == 0xDF00)
 			return true;
 
@@ -2243,8 +2258,11 @@ static bool cs_etm__is_svc_instr(struct cs_etm_queue *etmq,
 		 * +---------+---------+-------------------------+
 		 */
 		addr = ret_addr - 4;
-		cs_etm__frontend_mem_access(etmq, tidq, packet, addr,
-					    sizeof(instr32), (u8 *)&instr32);
+
+		if (cs_etm__frontend_mem_access(etmq, tidq, packet, addr,
+						sizeof(instr32), (u8 *)&instr32))
+			return false;
+
 		if ((instr32 & 0x0F000000) == 0x0F000000 &&
 		    (instr32 & 0xF0000000) != 0xF0000000)
 			return true;
@@ -2260,8 +2278,11 @@ static bool cs_etm__is_svc_instr(struct cs_etm_queue *etmq,
 		 * +-----------------------+---------+-----------+
 		 */
 		addr = ret_addr - 4;
-		cs_etm__frontend_mem_access(etmq, tidq, packet, addr,
-					    sizeof(instr32), (u8 *)&instr32);
+
+		if (cs_etm__frontend_mem_access(etmq, tidq, packet, addr,
+						sizeof(instr32), (u8 *)&instr32))
+			return false;
+
 		if ((instr32 & 0xFFE0001F) == 0xd4000001)
 			return true;
 

-- 
2.34.1

Re: [PATCH 1/3] perf cs-etm: Initialize the Thumb instruction buffer

[sashiko-bot]

> cs_etm__frontend_mem_access() can return zero or a short read without
> fully initializing instrBytes. cs_etm__t32_instr_size() nevertheless
> inspects its second byte. With CORESIGHT=1 and -O3, GCC 15.2.0 reports:
> 
>   error: 'instrBytes' may be used uninitialized
> 
> Zero-initialize the buffer to avoid reading uninitialized stack data
> when instruction memory is unavailable and fix the build failure.
> 
> Reported-by: Ian Rogers <irogers@google.com>
> Closes: https://lore.kernel.org/linux-perf-users/20261030-perf_cs_etm_fix_non_taken-v3-0-7629b52ab047@arm.com/T/#m64fb81c665214b79883d9be815b40ac5f6669628
> Fixes: a7ee4d625ede ("perf cs-etm: Support for ARM A32/T32 instruction sets in CoreSight trace")
> Signed-off-by: Leo Yan <leo.yan@arm.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-perf_fix_gcc15_failure-v1-0-eae592c5ff64@arm.com?part=1

Re: [PATCH 2/3] perf cs-etm: Use the packet instruction size for stack events

[sashiko-bot]

> cs_etm__add_stack_event() uses the last executed instruction of the
> previous packet as the branch source. The decoder already records this
> instruction's size in last_instr_size, including for T32 instructions.
> 
> Use the recorded size directly instead of reading instruction memory
> again. This removes an unnecessary read and the dependency on DSO data
> when updating the thread stack.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-perf_fix_gcc15_failure-v1-0-eae592c5ff64@arm.com?part=2

Re: [PATCH 3/3] perf cs-etm: Check frontend instruction reads

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 2 related pre-existing issues it does not
introduce:
- [Medium] perf cs-etm: missing perf_sample__exit() in sample synthesis
  error paths [1]
- [Medium] perf cs-etm: infinite loop in cs_etm__sample() when period is
  0 [2]
--

commit 1c4da6763b4506cd44f0609ef42fba0e4bd574ea
Author: Leo Yan <leo.yan@arm.com>

perf cs-etm: Check frontend instruction reads

This commit updates cs_etm__frontend_mem_access() to return 0 on a complete
read and -EIO otherwise, propagating read errors through instruction size,
address calculation, instruction copying, and SVC detection.

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 312d4a734e84f..303e16a54445b 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c

[ ... ]

> @@ -2011,11 +2021,13 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
>  			 */
>  			addr = cs_etm__instr_addr(etmq, tidq, tidq->packet,
>  						  offset - 1);
> -			ret = cs_etm__synth_instruction_sample(
> -				etmq, tidq, tidq->packet, addr,
> -				etm->instructions_sample_period);
> -			if (ret)
> -				return ret;
> +			if (addr != CS_ETM_INVAL_ADDR) {
> +				ret = cs_etm__synth_instruction_sample(
> +					etmq, tidq, tidq->packet, addr,
> +					etm->instructions_sample_period);
> +				if (ret)
> +					return ret;
> +			}

[Severity: Medium]
This problem wasn't introduced by this patch, but can
cs_etm__synth_instruction_sample() and cs_etm__synth_branch_sample() leak
sample resources when event injection fails [1]?

Both functions initialize sample with perf_sample__init(&sample, true),
which allocates sub-structures, and then return early before
perf_sample__exit(&sample) if cs_etm__inject_event() returns an error:

tools/perf/util/cs-etm.c:cs_etm__synth_instruction_sample() {
	...
	perf_sample__init(&sample, /*all=*/true);
	...
	if (etm->synth_opts.inject) {
		ret = cs_etm__inject_event(etm, event, &sample,
					   etm->instructions_sample_type);
		if (ret)
			return ret;
	}
	...
	perf_sample__exit(&sample);
	return ret;
}

>  
>  			offset += etm->instructions_sample_period;
>  			tidq->period_instructions -=

[Severity: Medium]
This problem wasn't introduced by this patch, but can cs_etm__sample() spin
forever when the user specifies an instruction sample period of 0 (for
example, --itrace=i0i) [2]?

The loop condition is:

tools/perf/util/cs-etm.c:cs_etm__sample() {
	...
		while (tidq->period_instructions >=
				etm->instructions_sample_period) {
			...
			offset += etm->instructions_sample_period;
			tidq->period_instructions -=
				etm->instructions_sample_period;
		}
}

Because both operands are unsigned, tidq->period_instructions >= 0 is
always true and subtracting 0 never reduces tidq->period_instructions.

[1] https://sashiko.dev/#/bug/linux-313ce3d0-12d6-4671-8007-6e3925af60fc
[2] https://sashiko.dev/#/bug/linux-88c200a9-e322-487b-904a-28d581ccee8b

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-perf_fix_gcc15_failure-v1-0-eae592c5ff64@arm.com?part=3