Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch claims to support i386 PLT unwinding but fundamentally breaks it by hardcoding x86-64 DWARF register macros.
--
--- Patch [20]: [RFC PATCH v2 20/24] unwind_user/eh_frame/x86: Handle PLT expressions ---
commit 75d2af7b6df62f61cc44905f96f37d91b80e466a
Author: Jens Remus [off-list ref]
unwind_user/eh_frame/x86: Handle PLT expressions
This commit adds x86-specific support for handling the CFA expression
found in GNU linker generated .eh_frame for PLT. It intends to enable
unwinding through GNU linker generated PLT entries on both i386 and
x86-64.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -15,7 +17,71 @@
#define EH_FRAME_MIN_DATA_ALIGN -8
#define EH_FRAME_MAX_DATA_ALIGN -1
-#endif
+#endif /* CONFIG_X86_64 */
[ ... ]
quoted hunk ↗ jump to hunk
+static inline int eh_frame_do_def_cfa_expression(const char *expr,
+ int size,
+ unsigned long ip,
+ struct eh_frame_reg_state *reg_state)
+{
[ ... ]
quoted hunk ↗ jump to hunk
+ if (!DW_OP_is_breg(sp_op) || !DW_OP_is_breg(ip_op))
+ return -EOPNOTSUPP;
+
+ 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]
The commit message mentions enabling unwinding for both i386 and x86-64,
but does this check inadvertently break i386 support?
EH_FRAME_REG_SP and EH_FRAME_REG_RA are defined under CONFIG_X86_64 and
correspond to x86-64 DWARF register numbers (7 and 16). For i386, the DWARF
register numbers are different (ESP is 4, EIP is 8).
When unwinding an i386 PLT expression, such as for a 32-bit compat process
running under a 64-bit kernel, will this check unconditionally reject the
expression and return -EOPNOTSUPP because 4 does not equal 7?
Also, if this code is compiled natively for 32-bit, would this cause a build
failure because the EH_FRAME_REG_SP and EH_FRAME_REG_RA macros are not
defined outside of the CONFIG_X86_64 block?
quoted hunk ↗ jump to hunk
+
+ /* CFA = (SP + SP_offset) + (((IP & 0xf) >= N) << shift) */
+ reg_state->cfa_rule = CFA_REG_OFFSET;
+ reg_state->cfa_regnum = EH_FRAME_REG_SP;
+ reg_state->cfa_offset = sp_offset + (((ip & 15) >= n) << shift);
+ return 0;
+ }
+
+ return -EOPNOTSUPP;
+}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260821195259.2688377-1-jremus@linux.ibm.com?part=20