[Crash-utility] Re: [PATCH v3] arm64: fix gdb register feeding for exception frames and stack switching

Sorry for the late reply, for v3, ack. Thanks, Tao Liu On Thu, May 7, 2026 at 12:31 AM 李鹏飞 <ljdlns1987(a)gmail.com&gt; wrote: > > Hi Tao, > > Just a gentle follow-up on this patch. > > Is there anything else needed, or can it be picked up? > > Thanks, > Pengfei > > Li Pengfei <ljdlns1987(a)gmail.com&gt; 于2026年4月1日周三 10:32写道： >> >> From: lipengfei28 <lipengfei28(a)xiaomi.com&gt; >> >> Hi Tao, >> >> Thanks for your review. >> >> This v3 addresses your comment: >> - remove redundant extra_stacks_regs[] allocation in >> arm64_switch_stack_from_overflow() >> - move variable declarations to block start for C89 style >> - add braces to outer if for readability >> >> Thanks, >> Pengfei >> >> --- >> >> On ARM64, crash's gdb bt relies on feeding a register snapshot (PC/SP/FP, >> etc.) into gdb and letting gdb unwind from there. Before this change, the >> register snapshot handling across exception frames / stack switching could be >> broken in several ways, leading to truncated backtraces, garbage substacks, or >> invalid addresses (e.g. -3 / 0xff...fd) showing up in gdb output. >> >> Fixes included in this patch: >> >> Fix out-of-bounds read when populating gdb registers from an exception frame >> In arm64_print_exception_frame() the previous code copied >> sizeof(struct arm64_pt_regs) bytes from a smaller stackframe-derived >> buffer, which could read past valid data and poison the gdb registers. >> Replace this with explicit field/register assignment and initialize the >> bitmap accordingly. >> >> Improve unwinding across IRQ/overflow stack transitions on newer kernels >> When switching stacks (IRQ / overflow), gdb may stop at the trampoline (e.g. >> call_on_irq_stack) because the discontinuity prevents it from recovering >> the caller frame automatically. For UNW_4_14+, "peek" one frame ahead by >> reading the saved FP/PC from the current frame, and register that as the >> next gdb substack, so gdb can continue unwinding on the process stack. >> >> Avoid creating invalid/empty substacks >> Only add a gdb substack when the recovered PC is non-zero, preventing bogus >> threads from being created. >> >> This patch only changes ARM64 unwinding/register setup logic. It does not try >> to reformat or merge gdb output. >> >> Tested on: Android Linux 6.x, arm64 >> >> Changes in v2: >> - Fix duplicate definition of arm64_gdb_add_next_frame_substack() >> - Fix indentation issues in arm64_print_exception_frame() >> - Minor cleanups based on Tao's review >> >> Changes in v3: >> - Remove redundant extra_stacks_regs[] allocation in >> arm64_switch_stack_from_overflow(), as arm64_gdb_add_next_frame_substack() >> already handles its own allocation internally >> >> Signed-off-by: lipengfei28 <lipengfei28(a)xiaomi.com&gt; >> --- >> arm64.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++--------- >> 1 file changed, 87 insertions(+), 17 deletions(-) >> >> diff --git a/arm64.c b/arm64.c >> index c125655..d177092 100644 >> --- a/arm64.c >> +++ b/arm64.c >> @@ -228,7 +228,7 @@ arm64_get_current_task_reg(int regno, const char *name, >> return FALSE; >> >> if (sid && sid <= extra_stacks_idx) { >> - ur_bitmap = extra_stacks_regs[extra_stacks_idx - 1]; >> + ur_bitmap = extra_stacks_regs[sid - 1]; >> goto get_sub; >> } >> >> @@ -3026,6 +3026,9 @@ static char *arm64_exception_functions[] = { >> "do_el0_irq_bp_hardening", >> "do_sp_pc_abort", >> "handle_bad_stack", >> + "el1h_64_sync", >> + "el1h_64_irq", >> + "el1h_64_error", >> NULL >> }; >> >> @@ -3123,6 +3126,11 @@ arm64_print_stackframe_entry(struct bt_info *bt, int level, struct arm64_stackfr >> >> fprintf(ofp, "\n"); >> >> + if (STREQ(name, "el1h_64_irq") || STREQ(name, "el1h_64_sync")) { >> + if (arm64_is_kernel_exception_frame(bt, frame->sp)) >> + arm64_print_exception_frame(bt, frame->sp, KERNEL_MODE, ofp); >> + } >> + >> if (bt->flags & BT_LINE_NUMBERS) { >> get_line_number(branch_pc, buf, FALSE); >> if (strlen(buf)) >> @@ -3775,11 +3783,13 @@ arm64_back_trace_cmd(struct bt_info *bt) >> REG_SEQ(arm64_pt_regs, pc)); >> SET_BIT(extra_stacks_regs[extra_stacks_idx]->bitmap, >> REG_SEQ(arm64_pt_regs, sp)); >> - if (!bt->machdep || >> + if (extra_stacks_regs[extra_stacks_idx]->ur.pc && >> + (bt->task != tt->panic_task) && >> + (!bt->machdep || >> (extra_stacks_regs[extra_stacks_idx]->ur.sp != >> ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.sp && >> extra_stacks_regs[extra_stacks_idx]->ur.pc != >> - ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc)) { >> + ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc))) { >> gdb_add_substack (extra_stacks_idx++); >> } >> } >> @@ -3925,11 +3935,13 @@ arm64_back_trace_cmd_v2(struct bt_info *bt) >> REG_SEQ(arm64_pt_regs, pc)); >> SET_BIT(extra_stacks_regs[extra_stacks_idx]->bitmap, >> REG_SEQ(arm64_pt_regs, sp)); >> - if (!bt->machdep || >> + if (extra_stacks_regs[extra_stacks_idx]->ur.pc && >> + (bt->task != tt->panic_task) && >> + (!bt->machdep || >> (extra_stacks_regs[extra_stacks_idx]->ur.sp != >> ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.sp && >> extra_stacks_regs[extra_stacks_idx]->ur.pc != >> - ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc)) { >> + ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc))) { >> gdb_add_substack (extra_stacks_idx++); >> } >> } >> @@ -4232,6 +4244,46 @@ arm64_in_kdump_text_on_irq_stack(struct bt_info *bt) >> return FALSE; >> } >> >> +static void >> +arm64_gdb_add_next_frame_substack(struct bt_info *bt, const struct arm64_stackframe *frame) >> +{ >> + struct machine_specific *ms = machdep->machspec; >> + struct user_regs_bitmap_struct *ur_ptr; >> + ulong next_fp, next_pc, next_sp; >> + >> + if (!extra_stacks_regs[extra_stacks_idx]) { >> + extra_stacks_regs[extra_stacks_idx] = (struct user_regs_bitmap_struct *) >> + malloc(sizeof(struct user_regs_bitmap_struct)); >> + } >> + >> + memset(extra_stacks_regs[extra_stacks_idx], 0, sizeof(struct user_regs_bitmap_struct)); >> + ur_ptr = extra_stacks_regs[extra_stacks_idx]; >> + >> + next_fp = GET_STACK_ULONG(frame->fp); >> + next_pc = GET_STACK_ULONG(frame->fp + 8); >> + next_sp = frame->fp + 16; >> + >> + if (is_kernel_text(next_pc | ms->CONFIG_ARM64_KERNELPACMASK)) >> + next_pc |= ms->CONFIG_ARM64_KERNELPACMASK; >> + >> + ur_ptr->ur.pc = next_pc; >> + ur_ptr->ur.sp = next_sp; >> + ur_ptr->ur.regs[29] = next_fp; >> + >> + SET_BIT(ur_ptr->bitmap, REG_SEQ(arm64_pt_regs, pc)); >> + SET_BIT(ur_ptr->bitmap, REG_SEQ(arm64_pt_regs, sp)); >> + SET_BIT(ur_ptr->bitmap, REG_SEQ(arm64_pt_regs, regs[0]) + 29); >> + >> + if (ur_ptr->ur.pc && >> + (!bt->machdep || >> + (ur_ptr->ur.sp != >> + ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.sp && >> + ur_ptr->ur.pc != >> + ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc))) { >> + gdb_add_substack(extra_stacks_idx++); >> + } >> +} >> + >> static int >> arm64_switch_stack(struct bt_info *bt, struct arm64_stackframe *frame, FILE *ofp) >> { >> @@ -4263,8 +4315,11 @@ arm64_switch_stack(struct bt_info *bt, struct arm64_stackframe *frame, FILE *ofp >> if (frame->fp == 0) >> return USER_MODE; >> >> - if (!(machdep->flags & UNW_4_14)) >> + if (!(machdep->flags & UNW_4_14)) { >> arm64_print_exception_frame(bt, frame->sp, KERNEL_MODE, ofp); >> + } else { >> + arm64_gdb_add_next_frame_substack(bt, frame); >> + } >> >> return KERNEL_MODE; >> } >> @@ -4300,8 +4355,11 @@ arm64_switch_stack_from_overflow(struct bt_info *bt, struct arm64_stackframe *fr >> if (frame->fp == 0) >> return USER_MODE; >> >> - if (!(machdep->flags & UNW_4_14)) >> + if (!(machdep->flags & UNW_4_14)) { >> arm64_print_exception_frame(bt, frame->sp, KERNEL_MODE, ofp); >> + } else { >> + arm64_gdb_add_next_frame_substack(bt, frame); >> + } >> >> return KERNEL_MODE; >> } >> @@ -4549,6 +4607,9 @@ arm64_print_exception_frame(struct bt_info *bt, ulong pt_regs, int mode, FILE *o >> (ulong)regs->orig_x0, (ulong)regs->syscallno); >> fprintf(ofp, " PSTATE: %08lx\n", (ulong)regs->pstate); >> } else if (!(bt->flags & BT_EFRAME_SEARCH)) { >> + struct user_regs_bitmap_struct *ur_ptr; >> + int i; >> + >> if (!extra_stacks_regs[extra_stacks_idx]) { >> extra_stacks_regs[extra_stacks_idx] = >> (struct user_regs_bitmap_struct *) >> @@ -4556,17 +4617,26 @@ arm64_print_exception_frame(struct bt_info *bt, ulong pt_regs, int mode, FILE *o >> } >> memset(extra_stacks_regs[extra_stacks_idx], 0, >> sizeof(struct user_regs_bitmap_struct)); >> - memcpy(&extra_stacks_regs[extra_stacks_idx]->ur, regs, >> - sizeof(struct arm64_pt_regs)); >> - for (int i = 0; i < sizeof(struct arm64_pt_regs)/sizeof(long); i++) >> - SET_BIT(extra_stacks_regs[extra_stacks_idx]->bitmap, i); >> - if (!bt->machdep || >> - (extra_stacks_regs[extra_stacks_idx]->ur.sp != >> + >> + ur_ptr = extra_stacks_regs[extra_stacks_idx]; >> + >> + ur_ptr->ur.pc = regs->pc; >> + ur_ptr->ur.sp = regs->sp; >> + ur_ptr->ur.pstate = regs->pstate; >> + for (i = 0; i < 31; i++) >> + ur_ptr->ur.regs[i] = regs->regs[i]; >> + >> + for (i = 0; i < 34; i++) >> + SET_BIT(ur_ptr->bitmap, i); >> + >> + if (ur_ptr->ur.pc && >> + (!bt->machdep || >> + (ur_ptr->ur.sp != >> ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.sp && >> - extra_stacks_regs[extra_stacks_idx]->ur.pc != >> - ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc)) { >> - gdb_add_substack (extra_stacks_idx++); >> - } >> + ur_ptr->ur.pc != >> + ((struct user_regs_bitmap_struct *)(bt->machdep))->ur.pc))) { >> + gdb_add_substack(extra_stacks_idx++); >> + } >> } >> } >> >> -- >> 2.34.1 >>