> So, we should revert this change and send the trap from ptrace_resume(),

What's wrong with ptrace_report_signal doing it?

>       - instead of send_sigtrap() we should use user_single_step_siginfo()
>         + force_sig_info()

Right.

>       - PTRACE_EVENT_SYSCALL_EXIT shouldn't send the trap, this was
>         x86 specific behaviour, and this is fixed in upstream by

Right.

>         (btw, I wrongly thought x86 is right and other machines should
>          be fixed).

In these corner cases it is always debatable what "right" is.
I didn't really think and decide about it until recently when
we decided about the upstream change.

>       - force_sig_info() needs tasklist_lock.
> 
>         we can change force_sig_info() to use lock_task_sighand(),
>         but perhaps we should not worry about this now

It would be much better if we can leave the signals code alone.  (It can
always use more cleanups, but let those happen later on their own.)  I
think ideally we should try to invoke it the way it's invoked now, i.e.
force_sig_info() is called on current.

>       - 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.
(Still we won't ignore the resume signal any more, but that is a
desireable change.)  

Conversely, we could make an upstream change along the lines of
the tracehook_report_syscall_exit() change, so that this uses a
real SIGTRAP instead of ptrace_notify().  That has some logic to
it.  But today this case is consistent across machines, so we
could consider that behavior change separately after utrace merges.

> I agree in advance with everything you prefer, you definetely
> know better.

:-)  Perhaps I do when I have it all in mind at the same moment.
But my brain capacity is feeling a bit low this week.

> Anything else I missed?

I hope not!  This area is not really fresh in my mind right now.


Thanks,
Roland

Reply via email to