On 11/12, Roland McGrath wrote: > > > So, we should revert this change and send the trap from ptrace_resume(), > > What's wrong with ptrace_report_signal doing it?
Nothing wrong (I think), but this is more complex and implies more unnecessary work. If the trap is sent by ptrace_resume() everything is simple. The tracee is stopped, we send the signal, and that is all. If ptrace_report_signal() sends the trap, we actually send the signal twice. Firstly ptrace_resume() does utrace_control(UTRACE_INTERRUPT) (ok, this is not the signal sending but sort of), then ->report_signal() notices we need the trap and send the "real" signal, then we return to get_signal_to_deliver() which calls utrace_get_signal() again, it dequeues SIGTRAP and calls ptrace_report_signal() again to finally report SIGTRAP to the tracer. But see below. > I > think ideally we should try to invoke it the way it's invoked now, i.e. > force_sig_info() is called on current. OK. Then we should send it from the callback. But: I am not sure yet, but I think the current code is not optimal for that, it was written to report *info without force_sig_info(). If we are going to send the signal instead, we can send it from ->report_quiesce() as well. So, I am going to revert these changes to move the send_sigtrap() logic back to ptrace_resume(), then I will think how to move this into callback context. > > - what should we do with PTRACE_EVENT_SIGTRAP ? > > > > > So, get rid of PTRACE_EVENT_SIGTRAP. Instead for the case of > > > UTRACE_SIGNAL_HANDLER when stepping, initialize *info to look > > > like a vanilla SIGTRAP and then fall into the default case for > > > real signals. > > > > should UTRACE_SIGNAL_HANDLER use force_sig_info() too or > > it can just deliver this *info as you initially suggested ? > > > > Or we can forget about this change for now? > > We need to make it consistent with the upstream ptrace behavior, > so we can't forget about anything that's needed for that. Today > tracehook_signal_handler() uses ptrace_notify(). So that is more > like an implicit fake signal report than a real queued SIGTRAP. So, lets revert this change for now to be compatible. This change is simple and can be done again at any time, probably along with upstream change. OK? Oleg.
