Re: [RFC PATCH v1 21/25] unwind_user/eh_frame/x86: Handle PLT expressions

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