> Oh... this adds more naming problems. And this way x86's send_sigtrap()
> can't use this helper easily.
It's true. Another approach would be to genericize send_sigtrap so that
all arch's can use it, and call that from tracehook_report_syscall_exit.
On x86 and powerpc TRAP_TRACE is the right si_code for step. So perhaps
you could just pass that to send_sigtrap from generic code. If another
arch differs, then you could make it use a new macro TRAP_SINGLE_STEP
instead--then define that to TRAP_TRACE when not defined and let other
arch's define it appropriately for their other value.
Currently the only other caller of send_sigtrap is do_debug. AFAICT that
case has already ruled out VM86 mode. So perhaps it would really be OK for
it to be using user_mode() instead of user_mode_vm(). In that case, it
could just use:
info->si_code = TRAP_TRACE;
info->si_addr = user_mode(regs) ? instruction_pointer(regs) : 0;
But you still need another arch hook there anyway for x86's extra
task->thread.foo fields. (And I'm not really sure that the syscall-exit
path has ruled out VM86 mode too, so maybe you do need user_mode_vm() in
there--and that's not a call other arch's define.)
> [PATCH 1/x] introduce user_single_step_siginfo()
I won't bother here with all the potential nit ways to do it differently.
Upstream review will help us decide that.
> perhaps user_single_step_siginfo() should go into ptrace.c ?
Yes, I don't think it needs to be inlined.
> +#ifdef ARCH_HAS_USER_SINGLE_STEP_INFO
> +extern void arch_user_single_step_siginfo(struct task_struct *tsk,
> + struct pt_regs *regs, siginfo_t *info);
> +#else
> +static inline void arch_user_single_step_siginfo(struct task_struct *tsk,
> + struct pt_regs *regs, siginfo_t *info)
> +{
> +}
> +#endif
Another way these things are sometimes done is to have a definition with
__attribute__((weak)) in kernel/ptrace.c and then let arch code give a
non-weak definition to override. (I don't much like that style, but it
does avoid all the #ifdef hair.)
> [PATCH 2/x] change powepc
Looks ok to me.
> [PATCH 3/x] change tracehook_report_syscall_exit() to look at step flag
Right.
> This is clear.
>
> What do you think?
That's all close enough to what I had in mind.
Exact details have to be ironed out with the upstream arch maintainers.
Thanks,
Roland
