Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Enabling `.eh_frame` unwinding on x86_64 breaks user stack traces for 32-bit compat binaries due to incorrect error handling in the generic unwind loop. --
--- Patch [19]: [RFC PATCH v2 19/24] unwind_user/eh_frame/x86: Enable eh_frame unwinding on x86 --- commit 26326eedb731dafef6946d5d887c188233f39043 Author: Jens Remus <[email protected]> unwind_user/eh_frame/x86: Enable eh_frame unwinding on x86 This patch enables .eh_frame user stack unwinding for x86-64 by selecting HAVE_UNWIND_USER_EH_FRAME and providing the necessary register accessors. > diff --git a/arch/x86/include/asm/unwind_user.h > b/arch/x86/include/asm/unwind_user.h > index c96645c824d1b..df1966bcb0f6e 100644 > --- a/arch/x86/include/asm/unwind_user.h > +++ b/arch/x86/include/asm/unwind_user.h [ ... ] > @@ -15,6 +16,45 @@ static inline int unwind_user_word_size(struct pt_regs > *regs) > return user_64bit_mode(regs) ? 8 : 4; > } > > +#ifdef CONFIG_X86_64 > + > +static inline int unwind_user_get_reg(unsigned long *val, unsigned int > regnum) > +{ > + struct pt_regs *regs = task_pt_regs(current); > + > + /* TODO: eh_frame is currently limited to x86-64 */ > + if (!user_64bit_mode(regs)) > + return -EINVAL; [Severity: High] Does returning -EINVAL here break the stack trace fallback mechanism for 32-bit compat tasks? In kernel/unwind/user.c:unwind_user_next(), the generic unwinder only continues to the next method (like the frame pointer unwinder) if it receives -ENOENT: kernel/unwind/user.c:unwind_user_next() { ... case UNWIND_USER_TYPE_EH_FRAME: switch (unwind_user_next_eh_frame(state)) { case 0: return 0; case -ENOENT: continue; /* Try next method. */ default: state->done = true; } break; ... } By returning -EINVAL for 32-bit tasks, the unwinder hits the default case and aborts entirely by setting state->done = true. This truncates the user stack trace instead of safely falling back to the FP unwinder. Would returning -ENOENT be more appropriate here when the current task is not 64-bit to ensure the fallback pipeline continues correctly? > + > + switch (regnum) { > + /* DWARF register numbers 0..15 */ > + case 0: *val = regs->ax; break; [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=19
