[PATCH 128] introduce suppress_sigtrap() to prevent unwanted send_sigtrap()

[Roland McGrath]

> > 1. (upstream) add arch hook(s) for single-step SIGTRAP siginfo_t + arch
> >    fields (i.e. x86's thread.error_code + thread.trap_no).
> 
> OK. say, arch_fill_sigtrap_info(siginfo_t *info, ...).

Something like that, right.  Since this is only for single-step in
particular, that should be in the name.  Also something that makes
it not seem incongruous to have the x86 one set those thread.error_code
et al fields, i.e. it's not only just filling in siginfo_t.

> force_sig_info() instead of ptrace_notify() ?
> 
> So, the tracee doesn't stop in tracehook_report_syscall_exit() path
> but sends the arch_fill_sigtrap_info()'ed signal to itself ?

Right, like x86 does today.

> > use that arch hook
> 
> "that" means arch_fill_sigtrap_info() above?

Right.

> >    -> powerpc et al PTRACE_SINGLESTEP-over-syscall goes from
> >    ptrace-swallows-signal to normal signal case
> > 3. (upstream) implement that arch hook for at least powerpc
> >    -> synthetic SIGTRAP gets si_* as a real step would, rather than 0
> > 4. (upstream) implement that arch hook for x86
> 
> again, "that arch hook" means arch_fill_sigtrap_info() ?

Right.

> > 5. (upstream) change x86 syscall_trace_leave to pass flag
> >    tracehook_report_syscall_exit(), not call send_sigtrap()
> 
> you mean, pass TIF_SINGLESTEP flag to tracehook_report_syscall_exit()
> and do not call send_sigtrap() ?

Right, like powerpc et al do today.

> >    -> x86 PTRACE_SINGLESTEP from syscall-exit stop no longer fake-traps
> >       without another insn, but takes a true step trap after next insn
> 
> But. 2. above suggests to use force_sig_info() ? Perhaps, this should
> depend on TIF_SINGLESTEP flag passed to tracehook_report_syscall_exit() ?

Right, x86 would do like powerpc et al do today:

	step = test_thread_flag(TIF_SINGLESTEP);
	if (step || test_thread_flag(TIF_SYSCALL_TRACE))
		tracehook_report_syscall_exit(regs, step);

> In any case, "but takes a true step trap after next insn" means the
> serious user-visible change?

It's a minor user-visible change in a strange corner case, but yes, it's
a change.  As to where you stop next for PTRACE_SINGLESTEP done at the
syscall-exit stop, it makes x86 match powerpc et al.

> >    -> compatible with powerpc et al behavior, makes more sense
> 
> What was the point of send_sigtrap() then?

I'm not sure I follow this question.  send_sigtrap is an x86
implementation detail.  If you are asking about why that function exists
as an implementation detail, it's just consolidating the synthetic call
for faked-step with the main SIGTRAP path in the do_debug trap handler.

But I can't tell if you are instead asking why to make the change to
powerpc et al (i.e. generic tracehook_report_syscall_exit) to send a
SIGTRAP instead of ptrace_notify, and have x86 use that same logic.

> Roland, I am all confused. Could you please explain what should
> syscall_trace_leave(TIF_SINGLESTEP) do, at least on x86 machine?
> 
> What it should do if TIF_SINGLESTEP is set, but TIF_SYSCALL_TRACE
> is not? In this case tracehook_report_syscall_exit() is not called.
> 
> Perhaps you can explain this with some pseudo code?

Real code, even! :-)

	step = test_thread_flag(TIF_SINGLESTEP);
	if (step || test_thread_flag(TIF_SYSCALL_TRACE))
		tracehook_report_syscall_exit(regs, step);

This gives the tracehook call the chance to choose the exact behavior,
so that becomes consistent across x86 and other machines.  So at that
level, the "what should" answer is that it should give the tracehook
layer the chance to implement uniform behavior across machines.


Thanks,
Roland