Re: s390 && user_enable_single_step() (Was: odd utrace testing results on s390x)

[Roland McGrath <roland@redhat.com>]

> Hmm, command for tracehook_signal_handler say this for stepping:
> @stepping:           nonzero if debugger single-step or block-step in
> use

Are you saying you would like me to clarify that wording somehow?  It's
meant to be implicit that the arch code is not doing any special fakery
about single-step for signal handlers, only processing real single-step
traps (and faking them for a syscall instruction if the arch requires
that).  No other arch does it, so it didn't occur to me that s390 would.
Before tracehook some had ptrace_notify calls there, and the call to
tracehook_signal_handler replaced that call.

> > In ptrace (including utrace-based ptrace), this winds up with sending a
> > SIGTRAP.  So when we finally do get out of do_signal and TIF_SINGLE_STEP
> > causes a second SIGTRAP, it's already pending and the second one makes no
> > difference.
> 
> So we have been lucky so far.

Actually, Oleg rightly points out:

> Confused again, perhaps I just misunderstood what you mean...
> 
> Without utrace, tracehook_signal_handler() doesn't send SIGTRAP, it
> merely does ptrace_notify(SIGTRAP), this means that
[...]
> even without utrace we can have unexpected SIGTRAP.

That is quite true, and I just misremembered when writing that paragraph.

So indeed we have been lucky, but it's not the luck of the problem not
happening on s390, but the luck of nobody ever caring. :-)

> Ok, so with the full utrace the semantics of tracehook_signal_handler
> is more than just causing a SIGTRAP. It is an indication for a signal
> AND a SIGTRAP if single-stepping is active. 

In short, it is the indication of a signal handler having been set up, just
like its kerneldoc description says.  Whatever that should mean to tracing
(SIGTRAP or otherwise) is in the purview of the generic tracing layer, not
the arch layer.

> To make both cases work we
> should stop setting TIF_SINGLE_STEP in do_signal and pass
> current->thread.per_info.single_step to tracehook_signal_handler
> instead of test_thread_flag(TIF_SINGLE_STEP).

Correct.


Thanks,
Roland