On Thu, Jul 10, 2025 at 06:35:14PM +0200, Jens Remus wrote:
quoted hunk ↗ jump to hunk
+#ifndef unwind_user_get_reg
+
+/**
+ * generic_unwind_user_get_reg - Get register value.
+ * @val: Register value.
+ * @regnum: DWARF register number to obtain the value from.
+ *
+ * Returns zero if successful. Otherwise -EINVAL.
+ */
+static inline int generic_unwind_user_get_reg(unsigned long *val, int regnum)
+{
+ return -EINVAL;
+}
+
+#define unwind_user_get_reg generic_unwind_user_get_reg
+
+#endif /* !unwind_user_get_reg */
I believe the traditional way to do this is to give the function the
same name as the define:
#ifndef unwind_user_get_reg
static inline int unwind_user_get_reg(unsigned long *val, int regnum)
{
return -EINVAL;
}
#define unwind_user_get_reg unwind_user_get_reg
#endif
quoted hunk ↗ jump to hunk
+/**
+ * generic_sframe_set_frame_reginfo - Populate info to unwind FP/RA register
+ * from SFrame offset.
+ * @reginfo: Unwind info for FP/RA register.
+ * @offset: SFrame offset value.
+ *
+ * A non-zero offset value denotes a stack offset from CFA and indicates
+ * that the register is saved on the stack. A zero offset value indicates
+ * that the register is not saved.
+ */
+static inline void generic_sframe_set_frame_reginfo(
+ struct unwind_user_reginfo *reginfo,
+ s32 offset)
+{
+ if (offset) {
+ reginfo->loc = UNWIND_USER_LOC_STACK;
+ reginfo->frame_off = offset;
+ } else {
+ reginfo->loc = UNWIND_USER_LOC_NONE;
+ }
+}
This just inits the reginfo struct, can we call it sframe_init_reginfo()?
Also the function comment seems completely superfluous as the function
is completely obvious.
Also the signature should match kernel style, something like:
static inline void
sframe_init_reginfo(struct unwind_user_reginfo *reginfo, s32 offset)
quoted hunk ↗ jump to hunk
@@ -98,26 +98,57 @@ static int unwind_user_next(struct unwind_user_state *state)
/* Get the Return Address (RA) */
- if (frame->ra_off) {
+ switch (frame->ra.loc) {
+ case UNWIND_USER_LOC_NONE:
+ if (!IS_ENABLED(CONFIG_HAVE_USER_RA_REG) || !topmost)
+ goto done;
+ ra = user_return_address(task_pt_regs(current));
+ break;
+ case UNWIND_USER_LOC_STACK:
+ if (!frame->ra.frame_off)
+ goto done;
/* Make sure that the address is word aligned */
shift = sizeof(long) == 4 || compat_fp_state(state) ? 2 : 3;
- if ((cfa + frame->ra_off) & ((1 << shift) - 1))
+ if ((cfa + frame->ra.frame_off) & ((1 << shift) - 1))
goto done;
- if (unwind_get_user_long(ra, cfa + frame->ra_off, state))
+ if (unwind_get_user_long(ra, cfa + frame->ra.frame_off, state))
goto done;
- } else {
- if (!IS_ENABLED(CONFIG_HAVE_USER_RA_REG) || !topmost)
+ break;
+ case UNWIND_USER_LOC_REG:
+ if (!IS_ENABLED(CONFIG_HAVE_UNWIND_USER_LOC_REG) || !topmost)
goto done;
- ra = user_return_address(task_pt_regs(current));
+ if (unwind_user_get_reg(&ra, frame->ra.regnum))
+ goto done;
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ goto done;
The default case will never happen, can we make it a BUG()?
quoted hunk ↗ jump to hunk
}
/* Get the Frame Pointer (FP) */
- if (frame->fp_off && unwind_get_user_long(fp, cfa + frame->fp_off, state))
+ switch (frame->fp.loc) {
+ case UNWIND_USER_LOC_NONE:
+ break;
The UNWIND_USER_LOC_NONE behavior is different here compared to above.
Do we also need UNWIND_USER_LOC_PT_REGS?
quoted hunk ↗ jump to hunk
+ case UNWIND_USER_LOC_STACK:
+ if (!frame->fp.frame_off)
+ goto done;
+ if (unwind_get_user_long(fp, cfa + frame->fp.frame_off, state))
+ goto done;
+ break;
+ case UNWIND_USER_LOC_REG:
+ if (!IS_ENABLED(CONFIG_HAVE_UNWIND_USER_LOC_REG) || !topmost)
+ goto done;
The topmost checking is *really* getting cumbersome, I do hope we can
get rid of that.
quoted hunk ↗ jump to hunk
+ if (unwind_user_get_reg(&fp, frame->fp.regnum))
+ goto done;
+ break;
+ default:
+ WARN_ON_ONCE(1);
goto done;
+ }
BUG(1) here as well.
quoted hunk ↗ jump to hunk
state->ip = ra;
state->sp = sp;
- if (frame->fp_off)
+ if (frame->fp.loc != UNWIND_USER_LOC_NONE)
state->fp = fp;
Instead of the extra conditional here, can fp be initialized to zero?
Either at the top of the function or in the RA switch statement?
--
Josh