Hello Dylan and Weinan!
On 4/6/2026 8:50 PM, Dylan Hatch wrote:
> Add unwind_next_frame_sframe() function to unwind by sframe info if
> present. Use this method at exception boundaries, falling back to
> frame-pointer unwind only on failure. In such failure cases, the
> stacktrace is considered unreliable.
>
> During normal unwind, prefer frame pointer unwind (for better
> performance) with sframe as a backup.
>
> This change restores the LR behavior originally introduced in commit
> c2c6b27b5aa14fa2 ("arm64: stacktrace: unwind exception boundaries"),
> But later removed in commit 32ed1205682e ("arm64: stacktrace: Skip
> reporting LR at exception boundaries")
>
> This can be done because the sframe data can be used to determine
> whether the LR is current for the PC value recovered from pt_regs at the
> exception boundary.
>
> Signed-off-by: Weinan Liu <[email protected]>
> Signed-off-by: Dylan Hatch <[email protected]>
> Reviewed-by: Prasanna Kumar T S M <[email protected]>
> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> +/*
> + * Unwind to the next frame according to sframe.
> + */
> +static __always_inline int
> +unwind_next_frame_sframe(struct kunwind_state *state)
> +{
> + struct unwind_frame frame;
> + unsigned long cfa, fp, ra;
> + enum kunwind_source source = KUNWIND_SOURCE_FRAME;
> + struct pt_regs *regs = state->regs;
> +
> + int err;
> +
> + /* FP/SP alignment 8 bytes */
> + if (state->common.fp & 0x7 || state->common.sp & 0x7)
> + return -EINVAL;
> +
> + /*
> + * Most/all outermost functions are not visible to sframe. So, check for
> + * a meta frame record if the sframe lookup fails.
> + */
> + err = sframe_find_kernel(state->common.pc, &frame);
> + if (err)
> + return kunwind_next_frame_record_meta(state);
> +
> + if (frame.outermost)
> + return -ENOENT;
> +
> + /* Get the Canonical Frame Address (CFA) */
> + switch (frame.cfa.rule) {
> + case UNWIND_CFA_RULE_SP_OFFSET:
> + cfa = state->common.sp;
> + break;
> + case UNWIND_CFA_RULE_FP_OFFSET:
> + if (state->common.fp < state->common.sp)
> + return -EINVAL;
I wonder whether that check is valid in kernel? Looking at
call_on_irq_stack() saving SP in FP and then loading SP with the IRQ SP.
Is that condition always true then?
> + cfa = state->common.fp;
> + break;
> + case UNWIND_CFA_RULE_REG_OFFSET:
> + case UNWIND_CFA_RULE_REG_OFFSET_DEREF:
> + if (!regs)
if (!regs || frame.cfa.regnum > 30)
> + return -EINVAL;
> + cfa = regs->regs[frame.cfa.regnum];
In unwind user this is guarded by a topmost frame check, as arbitrary
registers are otherwise not available. Isn't this necessary in the
kernel case?
> + break;
> + default:
> + WARN_ON_ONCE(1);
> + return -EINVAL;
> + }
> + cfa += frame.cfa.offset;
> +
> + /*
> + * CFA typically points to a higher address than RA or FP, so don't
> + * consume from the stack when we read it.
> + */
> + if (frame.cfa.rule & UNWIND_RULE_DEREF &&
> + !get_word(&state->common, &cfa))
> + return -EINVAL;
> +
> + /* CFA alignment 8 bytes */
> + if (cfa & 0x7)
> + return -EINVAL;
> +
> + /* Get the Return Address (RA) */
> + switch (frame.ra.rule) {
> + case UNWIND_RULE_RETAIN:
> + if (!regs)
> + return -EINVAL;
> + ra = regs->regs[30];
Likewise: Topmost frame check not required to access arbitrary registers
(including RA/LR)? Furthermore, provided don't have a thinko, LR may
only be in LR in the topmost frame. In any other frame it must have
been saved. Otherwise there would be an endless return loop.
> + source = KUNWIND_SOURCE_REGS_LR;
> + break;
> + /* UNWIND_USER_RULE_CFA_OFFSET not implemented on purpose */
> + case UNWIND_RULE_CFA_OFFSET_DEREF:
> + ra = cfa + frame.ra.offset;
> + break;
> + case UNWIND_RULE_REG_OFFSET:
> + case UNWIND_RULE_REG_OFFSET_DEREF:
> + if (!regs)
if (!regs || frame.cfa.regnum > 30)
> + return -EINVAL;
> + ra = regs->regs[frame.cfa.regnum];
Likewise: Topmost frame check not required to access arbitrary registers?
> + ra += frame.ra.offset;
> + break;
> + default:
> + WARN_ON_ONCE(1);
> + return -EINVAL;
> + }
> +
> + /* Get the Frame Pointer (FP) */
> + switch (frame.fp.rule) {
> + case UNWIND_RULE_RETAIN:
> + fp = state->common.fp;
> + break;
> + /* UNWIND_USER_RULE_CFA_OFFSET not implemented on purpose */
> + case UNWIND_RULE_CFA_OFFSET_DEREF:
> + fp = cfa + frame.fp.offset;
> + break;
> + case UNWIND_RULE_REG_OFFSET:
> + case UNWIND_RULE_REG_OFFSET_DEREF:
> + if (!regs)
if (!regs || frame.cfa.regnum > 30)
> + return -EINVAL;
> + fp = regs->regs[frame.fp.regnum];
Likewise: Topmost frame check not required to access arbitrary registers?
> + fp += frame.fp.offset;
> + break;
> + default:
> + WARN_ON_ONCE(1);
> + return -EINVAL;
> + }
> +
> + /*
> + * Consume RA and FP from the stack. The frame record puts FP at a lower
> + * address than RA, so we always read FP first.
> + */
> + if (frame.fp.rule & UNWIND_RULE_DEREF &&
> + !get_word(&state->common, &fp))
> + return -EINVAL;
> +
> + if (frame.ra.rule & UNWIND_RULE_DEREF &&
> + get_consume_word(&state->common, &ra))
> + return -EINVAL;
> +
> + state->common.pc = ra;
> + state->common.sp = cfa;
> + state->common.fp = fp;
> +
> + state->source = source;
> +
> + return 0;
> +}
Thanks and regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
[email protected] / [email protected]
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats:
Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft:
Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
