Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The x86 PLT CFA expression handler hardcodes 64-bit DWARF register numbers, breaking the claimed i386 support. --
--- Patch [21]: [RFC PATCH v1 21/25] unwind_user/eh_frame/x86: Handle PLT expressions --- commit 0609783d6303290732b67ec7492d3c753e51d51c Author: Jens Remus <[email protected]> unwind_user/eh_frame/x86: Handle PLT expressions This commit adds x86-specific support for unwinding through GNU linker generated PLT entries by handling the CFA expression found in .eh_frame. It explicitly aims to enable this for both i386 and x86-64 binaries. > diff --git a/arch/x86/include/asm/unwind_user_eh_frame.h > b/arch/x86/include/asm/unwind_user_eh_frame.h > index fdccbda2fe4b1..8268eb1a0ff5a 100644 > --- a/arch/x86/include/asm/unwind_user_eh_frame.h > +++ b/arch/x86/include/asm/unwind_user_eh_frame.h [ ... ] > +static inline int eh_frame_do_def_cfa_expression(const char *expr, > + int size, > + unsigned long ip, > + struct eh_frame_reg_state > *reg_state) > +{ [ ... ] > + sp_reg = DW_OP_breg_register(sp_op); > + ip_reg = DW_OP_breg_register(ip_op); > + if (sp_reg != EH_FRAME_REG_SP || ip_reg != EH_FRAME_REG_RA) > + return -EOPNOTSUPP; [Severity: Medium] Does this unconditionally reject 32-bit i386 PLT expressions? The commit message mentions enabling unwinding for both i386 and x86-64, and the comment above notes the 32-bit DWARF registers 4 (ESP) and 8 (EIP). However, this check requires the parsed DWARF register values to match EH_FRAME_REG_SP (7) and EH_FRAME_REG_RA (16), which are exclusively the x86-64 register mappings. Will this cause valid i386 expressions to fail and return -EOPNOTSUPP? While 32-bit eh_frame unwinding is currently disabled at a higher level for x86-64 tasks, this logic appears to contradict the intended architectural support. > + > + /* CFA = (SP + SP_offset) + (((IP & 0xf) >= N) << shift) */ > + reg_state->cfa_rule = CFA_REG_OFFSET; > + reg_state->cfa_regnum = EH_FRAME_REG_SP; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=21
