>       ptrace_detach(sig) checks valid_signal(sig) to detect the
>       explicit detach and passes "bool voluntary" to ptrace_wake_up().

ptrace_detach_task() does.  ptrace_detach() has already bailed out
if !valid_signal(sig) was really pass in from userland.

valid_signal(0) => true, so this is checking specifically for the
exit_ptrace() call that passes -1 to ptrace_do_detach()->ptrace_detach_task().

That is all rather nonobvious.  But once you do figure it all out, the
"voluntary" variable name stops being confusing. :-)

It is strange to use valid_signal() at all inside ptrace_detach_task(),
where the possible values for sig are -1 from exit_ptrace(), 0, or a valid
signal number.  

Passing 0 to send_sig_info looks like it will get through and then confuse
the lower layers.  Make that condition sig > 0.

If sig==0, remind me why we're not just doing action=UTRACE_DETACH anyway?

> both upstream and utrace-ptrace are racy (but these races are
> different), hopefully utrace-ptrace is not worse. And of course
> this all is not right and should be fixed, but we seem to agree
> this should be fixed later, when utrace-ptrace is merged.

Ok.

> In my opinion, from now utrace-ptrace is close enough to upstream
> wrt stop/wakeup oddities. It passes all tests which upstream passes,
> plus it passes detach-stopped which fails with upstream kernel.

Ok, that sounds good.  But remember that ptrace-tests has only the problem
cases we've noticed before, not necessarily everything that matters.  We
should feel comfortable when running the gdb testsuite has no regressions
vs the vanilla kernel, for a variety of past and present gdb versions.
(That suite tends to have many failures, but all we need to do is match the
vanilla kernel's behavior in which ones pass or fail.)


Thanks,
Roland

Reply via email to