Re: [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32

[Rui Qi]

Hi Palmer, Paul, Albert, Alexandre,

Gentle ping on this patch. It has been posted for over two weeks
now and I would appreciate any feedback.

This fixes a frame pointer bug in call_on_irq_stack on RV32 where
the unwinder reads from the alignment padding instead of the
actual saved fp/ra, causing broken stack traces on 32-bit RISC-V.

Please let me know if there are any concerns or if further changes
are needed.

+Cc: Conor Dooley -- adding RISC-V subsystem reviewer.


On 6/3/26 11:53 AM, Rui Qi wrote:
> The frame pointer (s0/fp) in call_on_irq_stack is set using
> STACKFRAME_SIZE_ON_STACK, which equals ALIGN(sizeof(struct stackframe),
> STACK_ALIGN). On RV64, sizeof(struct stackframe) is 16 and the aligned
> size is also 16, so there is no difference. However, on RV32,
> sizeof(struct stackframe) is 8 while STACKFRAME_SIZE_ON_STACK is 16 due
> to 8 bytes of alignment padding.
> 
> The stack unwinder does 'frame = (struct stackframe *)fp - 1', which
> reads from 'fp - sizeof(struct stackframe)'. On RV32, with fp set to
> sp + STACKFRAME_SIZE_ON_STACK (sp + 16), the unwinder reads from
> sp + 8, which falls in the alignment padding rather than where the
> saved fp/ra are actually stored (sp + 0 and sp + 4).
> 
> Fix this by introducing STACKFRAME_SIZE (the unaligned sizeof) and using
> it for frame pointer setup and restoration, while keeping
> STACKFRAME_SIZE_ON_STACK for the stack pointer allocation/deallocation
> which must remain 16-byte aligned.
> 
> Signed-off-by: Rui Qi <qirui.001@xxxxxxxxxxxxx>
> ---
>  arch/riscv/kernel/asm-offsets.c | 1 +
>  arch/riscv/kernel/entry.S       | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
> index af827448a609..c1b5f7eb03fd 100644
> --- a/arch/riscv/kernel/asm-offsets.c
> +++ b/arch/riscv/kernel/asm-offsets.c
> @@ -500,6 +500,7 @@ void asm_offsets(void)
>  	OFFSET(SBI_HART_BOOT_TASK_PTR_OFFSET, sbi_hart_boot_data, task_ptr);
>  	OFFSET(SBI_HART_BOOT_STACK_PTR_OFFSET, sbi_hart_boot_data, stack_ptr);
>  
> +	DEFINE(STACKFRAME_SIZE, sizeof(struct stackframe));
>  	DEFINE(STACKFRAME_SIZE_ON_STACK, ALIGN(sizeof(struct stackframe), STACK_ALIGN));
>  	OFFSET(STACKFRAME_FP, stackframe, fp);
>  	OFFSET(STACKFRAME_RA, stackframe, ra);
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index d011fb51c59a..e8b654e2b7b5 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -383,7 +383,7 @@ SYM_FUNC_START(call_on_irq_stack)
>  	addi	sp, sp, -STACKFRAME_SIZE_ON_STACK
>  	REG_S	ra, STACKFRAME_RA(sp)
>  	REG_S	s0, STACKFRAME_FP(sp)
> -	addi	s0, sp, STACKFRAME_SIZE_ON_STACK
> +	addi	s0, sp, STACKFRAME_SIZE
>  
>  	/* Switch to the per-CPU shadow call stack */
>  	scs_save_current
> @@ -399,7 +399,7 @@ SYM_FUNC_START(call_on_irq_stack)
>  	scs_load_current
>  
>  	/* Switch back to the thread stack and restore ra and s0 */
> -	addi	sp, s0, -STACKFRAME_SIZE_ON_STACK
> +	addi	sp, s0, -STACKFRAME_SIZE
>  	REG_L	ra, STACKFRAME_RA(sp)
>  	REG_L	s0, STACKFRAME_FP(sp)
>  	addi	sp, sp, STACKFRAME_SIZE_ON_STACK