* [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32
@ 2026-06-03 3:53 ` Rui Qi
0 siblings, 0 replies; 6+ messages in thread
From: Rui Qi @ 2026-06-03 3:53 UTC (permalink / raw)
To: palmer, pjw, aou, alex; +Cc: Rui Qi, linux-riscv, linux-kernel
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@bytedance.com>
---
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
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32
2026-06-03 3:53 ` Rui Qi
@ 2026-06-22 2:54 ` Rui Qi
-1 siblings, 0 replies; 6+ messages in thread
From: Rui Qi @ 2026-06-22 2:54 UTC (permalink / raw)
To: palmer, pjw, aou, alex, Conor Dooley; +Cc: linux-riscv, linux-kernel
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@bytedance.com>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32
@ 2026-06-22 2:54 ` Rui Qi
0 siblings, 0 replies; 6+ messages in thread
From: Rui Qi @ 2026-06-22 2:54 UTC (permalink / raw)
To: palmer, pjw, aou, alex, Conor Dooley; +Cc: linux-riscv, linux-kernel
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@bytedance.com>
> ---
> 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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32
2026-06-03 3:53 ` Rui Qi
@ 2026-06-22 18:08 ` Nam Cao
-1 siblings, 0 replies; 6+ messages in thread
From: Nam Cao @ 2026-06-22 18:08 UTC (permalink / raw)
To: Rui Qi, palmer, pjw, aou, alex; +Cc: Rui Qi, linux-riscv, linux-kernel
"Rui Qi" <qirui.001@bytedance.com> writes:
> --- 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
Doesn't this break the calling convention? The ABI specifies that "After
the prologue, the frame pointer register will point to [...] the stack
pointer value on entry to the current procedure". The frame pointer is
already correct here.
It is the storage locations of ra and fp that are broken. The ABI says
"This puts the return address at fp - XLEN/8, and the previous frame
pointer at fp - 2 * XLEN/8".
Or am I confused?
Nam
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32
@ 2026-06-22 18:08 ` Nam Cao
0 siblings, 0 replies; 6+ messages in thread
From: Nam Cao @ 2026-06-22 18:08 UTC (permalink / raw)
To: Rui Qi, palmer, pjw, aou, alex; +Cc: Rui Qi, linux-riscv, linux-kernel
"Rui Qi" <qirui.001@bytedance.com> writes:
> --- 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
Doesn't this break the calling convention? The ABI specifies that "After
the prologue, the frame pointer register will point to [...] the stack
pointer value on entry to the current procedure". The frame pointer is
already correct here.
It is the storage locations of ra and fp that are broken. The ABI says
"This puts the return address at fp - XLEN/8, and the previous frame
pointer at fp - 2 * XLEN/8".
Or am I confused?
Nam
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread