Re: Bug in rev 1.3 of sys/i386/linux/linux_ptrace.c

On Mon, 14 Apr 2003, Marcel Moolenaar wrote:

> On Mon, Apr 14, 2003 at 04:12:06PM -0400, John Baldwin wrote:
> > In the linux_ptrace() function there is the following code:
> *snip*
> >                 /* not currently stopped */
> >                 if ((p->p_flag & (P_TRACED|P_WAITED)) == 0) {
> >                         error = EBUSY;
> >                         goto fail;
> >                 }
> > 
> >                 ...
> > 
> > Now, since we've already checked P_TRACED above, this last
> > check will never fail.  The diff in rev 1.3 was:
> > 
> > -               if (p->p_stat != SSTOP || (p->p_flag & P_WAITED) == 0) {
> > +               if ((p->p_flag & (P_TRACED|P_WAITED)) == 0) {
> > 
> > So should this be (P_STOPPED|P_WAITED) instead?  Or maybe just
> > (P_STOPPED_TRACE|P_WAITED)?
> 
> I don't know the difference between P_STOPPED and P_STOPPED_TRACE


P_STOPPED is actually a set of 3 bits
each for one reason that teh process might be stopped..
i.e 
#define P_STOPPED_SIG           0x20000 /* Stopped due to SIGSTOP/SIGTSTP */
#define P_STOPPED_TRACE         0x40000 /* Stopped because of tracing */
#define P_STOPPED_SINGLE        0x80000 /* Only one thread can continue
*/

#define P_STOPPED \
(P_STOPPED_SIG|P_STOPPED_SINGLE|P_STOPPED_TRACE)

If P_STOPPED but !P_STOPPED_TRACE then that means that the process is
stopped for some other reason and the debugger has not stopped it (yet).
Iwther we find any stoppage acceptible or just P_STOPPED_TRACE
depends upon whether there is a chance that the other stop reason 
may be removed before whatever it is we are testign for gets done.

For example you may be in P_STOPPED_SINGLE mode, 
where all threads are trapped and forced to SUSPENDED state until
only one (doing for example a fork()) is left. THAT thread is allowed to
continue doing it's work in the kernel until it finishes
and then releases the condition. Theoretically this is using the
'STOPPED' checks in the kernel to "mostly stop" the process.
but the bit could be released without intervention from the 
debugger. Also, in a threaded world, 
having any of the STOPPED bits does not mean that all your threads are
yet suspended.. These are flags that 'trip up' threads as they try exit
the kernel and suspend them, (or they suspend themselves) but the
true check for all teh threads being suspended is:

(I think)
if ((P_SHOULDSTOP(p) & P_STOPPED_TRACE) &&
            (p->p_numthreads == p->p_suspcount))


i.e.
Are all the threads now suspended, and is our trace one of the reasons
that they will be held suspended. (i.e this guarantees that no-one else
can allow them to run while we are not looking)

The code above is slightly suboptimal but the compiler should optimise
it.. (one would hope).


> but yes, we should check whether the process is stopped. The
> equivalent in sys/kern/sys_process.c is:

This says that the threads have been requested to stop, but does not
indicate truely whther they have complied yet.

> 
> 	if (!P_SHOULDSTOP(p) || (p->p_flag & P_WAITED) == 0) {
> 
> P_SHOULDSTOP(p) expands to:
> 
> 	((p)->p_flag & P_STOPPED)
> 
> Using P_STOPPED makes us bug-for-bug compatible...
> 
> -- 
>  Marcel Moolenaar	  USPA: A-39004		 marcel_at_xcllnt.net
>