| Message ID | 10b92f2fd065b67e6e3fd473ca145c34ea74b73a.1737511963.git.jpoimboe@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | unwind, perf: sframe user space unwinding | expand |
On Tue, Jan 21, 2025 at 6:32 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > Add optional support for user space frame pointer unwinding. If > supported, the arch needs to enable CONFIG_HAVE_UNWIND_USER_FP and > define ARCH_INIT_USER_FP_FRAME. > > By encoding the frame offsets in struct unwind_user_frame, much of this > code can also be reused for future unwinder implementations like sframe. > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> > --- > arch/Kconfig | 4 +++ > include/asm-generic/unwind_user.h | 9 ++++++ > include/linux/unwind_user_types.h | 1 + > kernel/unwind/user.c | 49 +++++++++++++++++++++++++++++-- > 4 files changed, 60 insertions(+), 3 deletions(-) > create mode 100644 include/asm-generic/unwind_user.h > Do you plan to reuse this logic for stack unwinding done by perf subsystem in perf_callchain_user()? See is_uprobe_at_func_entry() parts and also fixup_uretprobe_trampoline_entries() for some of the quirks that have to be taken into account when doing frame pointer-based unwinding. It would be great not to lose those in this new reimplementation. Not sure what's the best way to avoid duplicating the logic, but I thought I'd bring that up. > diff --git a/arch/Kconfig b/arch/Kconfig > index c6fa2b3ecbc6..cf996cbb8142 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -438,6 +438,10 @@ config HAVE_HARDLOCKUP_DETECTOR_ARCH > config UNWIND_USER > bool > > +config HAVE_UNWIND_USER_FP > + bool > + select UNWIND_USER > + > config AS_SFRAME > def_bool $(as-instr,.cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc) > [...]
On Fri, Jan 24, 2025 at 09:59:37AM -0800, Andrii Nakryiko wrote: > On Tue, Jan 21, 2025 at 6:32 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > > > Add optional support for user space frame pointer unwinding. If > > supported, the arch needs to enable CONFIG_HAVE_UNWIND_USER_FP and > > define ARCH_INIT_USER_FP_FRAME. > > > > By encoding the frame offsets in struct unwind_user_frame, much of this > > code can also be reused for future unwinder implementations like sframe. > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> > > --- > > arch/Kconfig | 4 +++ > > include/asm-generic/unwind_user.h | 9 ++++++ > > include/linux/unwind_user_types.h | 1 + > > kernel/unwind/user.c | 49 +++++++++++++++++++++++++++++-- > > 4 files changed, 60 insertions(+), 3 deletions(-) > > create mode 100644 include/asm-generic/unwind_user.h > > > > Do you plan to reuse this logic for stack unwinding done by perf > subsystem in perf_callchain_user()? See is_uprobe_at_func_entry() > parts and also fixup_uretprobe_trampoline_entries() for some of the > quirks that have to be taken into account when doing frame > pointer-based unwinding. It would be great not to lose those in this > new reimplementation. > > Not sure what's the best way to avoid duplicating the logic, but I > thought I'd bring that up. Indeed! That was on the todo list and somehow evaporated.
diff --git a/arch/Kconfig b/arch/Kconfig index c6fa2b3ecbc6..cf996cbb8142 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -438,6 +438,10 @@ config HAVE_HARDLOCKUP_DETECTOR_ARCH config UNWIND_USER bool +config HAVE_UNWIND_USER_FP + bool + select UNWIND_USER + config AS_SFRAME def_bool $(as-instr,.cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc) diff --git a/include/asm-generic/unwind_user.h b/include/asm-generic/unwind_user.h new file mode 100644 index 000000000000..832425502fb3 --- /dev/null +++ b/include/asm-generic/unwind_user.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_GENERIC_UNWIND_USER_H +#define _ASM_GENERIC_UNWIND_USER_H + +#ifndef ARCH_INIT_USER_FP_FRAME + #define ARCH_INIT_USER_FP_FRAME +#endif + +#endif /* _ASM_GENERIC_UNWIND_USER_H */ diff --git a/include/linux/unwind_user_types.h b/include/linux/unwind_user_types.h index 6ed1b4ae74e1..65bd070eb6b0 100644 --- a/include/linux/unwind_user_types.h +++ b/include/linux/unwind_user_types.h @@ -6,6 +6,7 @@ enum unwind_user_type { UNWIND_USER_TYPE_NONE, + UNWIND_USER_TYPE_FP, }; struct unwind_stacktrace { diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c index 456539635e49..73fd4e150dfd 100644 --- a/kernel/unwind/user.c +++ b/kernel/unwind/user.c @@ -6,6 +6,18 @@ #include <linux/sched.h> #include <linux/sched/task_stack.h> #include <linux/unwind_user.h> +#include <linux/uaccess.h> +#include <asm/unwind_user.h> + +static struct unwind_user_frame fp_frame = { + ARCH_INIT_USER_FP_FRAME +}; + +static inline bool fp_state(struct unwind_user_state *state) +{ + return IS_ENABLED(CONFIG_HAVE_UNWIND_USER_FP) && + state->type == UNWIND_USER_TYPE_FP; +} int unwind_user_next(struct unwind_user_state *state) { @@ -13,8 +25,36 @@ int unwind_user_next(struct unwind_user_state *state) struct unwind_user_frame *frame = &_frame; unsigned long cfa = 0, fp, ra = 0; - /* no implementation yet */ - -EINVAL; + if (state->done) + return -EINVAL; + + if (fp_state(state)) + frame = &fp_frame; + else + goto the_end; + + cfa = (frame->use_fp ? state->fp : state->sp) + frame->cfa_off; + + /* stack going in wrong direction? */ + if (cfa <= state->sp) + goto the_end; + + if (get_user(ra, (unsigned long *)(cfa + frame->ra_off))) + goto the_end; + + if (frame->fp_off && get_user(fp, (unsigned long __user *)(cfa + frame->fp_off))) + goto the_end; + + state->ip = ra; + state->sp = cfa; + if (frame->fp_off) + state->fp = fp; + + return 0; + +the_end: + state->done = true; + return -EINVAL; } int unwind_user_start(struct unwind_user_state *state) @@ -28,7 +68,10 @@ int unwind_user_start(struct unwind_user_state *state) return -EINVAL; } - state->type = UNWIND_USER_TYPE_NONE; + if (IS_ENABLED(CONFIG_HAVE_UNWIND_USER_FP)) + state->type = UNWIND_USER_TYPE_FP; + else + state->type = UNWIND_USER_TYPE_NONE; state->ip = instruction_pointer(regs); state->sp = user_stack_pointer(regs);
Add optional support for user space frame pointer unwinding. If supported, the arch needs to enable CONFIG_HAVE_UNWIND_USER_FP and define ARCH_INIT_USER_FP_FRAME. By encoding the frame offsets in struct unwind_user_frame, much of this code can also be reused for future unwinder implementations like sframe. Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> --- arch/Kconfig | 4 +++ include/asm-generic/unwind_user.h | 9 ++++++ include/linux/unwind_user_types.h | 1 + kernel/unwind/user.c | 49 +++++++++++++++++++++++++++++-- 4 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 include/asm-generic/unwind_user.h