Q: utrace && ptrace_check_attach()

[Oleg Nesterov]

On 07/27, Roland McGrath wrote:
>
> No, it can't.  That's what "get the bookkeeping right" means.
> It the debugger uses UTRACE_RESUME et al, then that thread moves
> from TASK_TRACED into TASK_STOPPED and still never runs.

Ah. In that case yes, we can make this all consistent.

But I am not sure I understand you right, please see below.

> > > The bookkeeping should ensure that when a TASK_TRACED thread was
> > > counted as stopped,
> >
> > But we can't know if it was already counted or not.
>
> Sure we can.  The group_stop_count is set by the stop-instigator while it
> holds siglock and checks every thread's ->state.  All transitions into or
> out of TASK_TRACED are done holding the siglock.  When you take siglock and
> see there is a stop in progress, then you know the thread's TASK_TRACED was
> counted as stopped in setting up that stop.

Yes, yes, sure.

But currently ptrace can wake up the tracee and then later it can be
ptrace_stop()'ed again, in this case we can decrement ->group_stop_count
twice for the same thread.


OK, forget about mt issues. Do you really mean PTRACE_CONT/etc must _not_
wake up SIGNAL_STOP_STOPPED tracee ? This would be nice perhaps, but this
means a serious user-visible change.

Or I misunderstood you?

> > And note that utrace_stop() doesn't set SIGNAL_STOP_STOPPED and doesn't
> > notify if ->group_stop_count becomes 0.
>
> We can fix this.

sure, this is minor.

> > Or do you think it is better to add tracehook_finish_stop() helper which is
> > called by do_signal_stop() to clear ->stopped ?
>
> Let's do it however makes the code taken all together come out cleanest.
> From what you said before, it sounds like tracehook_finish_stop() would
> help with that.

OK, I'll send the patch.

> > BTW, can't finish_utrace_stop() check utrace->stopped lockless?
>
> You tell me!

I think it can.

finish_utrace_stop() can't miss ->stopped if it was not cleared, it was
set by us. When finish_utrace_stop() takes utrace->lock to clear ->stopped
it can be already cleared by utrace_wakeup() but we don't care.

And, if ->stopped == F, nobody (utrace_do_stop() actually) can set it.
If this was possible we have a bug anyway.

Oleg.