Re: RFA[threads]: Fork event updates, part the fifteenth

[Michael Snyder <msnyder at redhat dot com>]

Daniel Jacobowitz wrote:
> 
> And here's the really exciting one for today.  The patch looks a little
> silly because I'm not quite ready to introduce the body of
> linux_record_stopped_pid yet; it will go in linux-nat.c, which means the
> patch to add it will be noisy and generally uninteresting.  Let me explain
> what's going on and why it is necessary.
> 
> When we get a fork event, the kernel reports two things.  The parent is
> stopped and reports a new "extended wait status", indicating that a fork has
> occured.  The child reports a SIGSTOP and the fact that we've never seen its
> PID before is supposed to tip us off.  Unfortunately there was no practical
> way for these two events to be received in a deterministic order, given the
> other design constraints on the interface.  Particularly, by the time that
> the event is reported I wanted to be able to wait for and PTRACE_DETACH from
> the child without having to resume the parent, so the child already had to
> be on the runqueue.
> 
> This SIGSTOP from an unknown PID is the same mechanism used by CLONE_PTRACE.
> 
> This patch provides a hook to just add the PIDs to a list and go back to
> waiting for the fork/vfork/clone event.  Right now, this will further mess
> up debugging of a CLONE_PTRACE application.  However, while there is some
> code lying around to support it I just verified that we can't debug a simple
> CLONE_PTRACE application, so I don't feel bad about that :)  We receive
> events from an unknown process and get confused.

I don't think CLONE_PTRACE is handled yet.  We oughta do that (tm).

> Is this OK?  Would you rather it go in with the real linux_record_stopped_pid?
> I'm just submitting it separately in order to make the patch clearer.

Assuming that I understand the context correctly:
  The process we are debugging has created a new process, presumably by fork.
  The new process is stopped with SIGSTOP.
  We want to debug the new process.

then let me comment:


> 
> 2002-12-15  Daniel Jacobowitz  <drow@mvista.com>
> 
>         * lin-lwp.c (linux_record_stopped_pid): New function and
>         prototype.
>         (child_wait): Call linux_record_stopped_pid.
>         (lin_lwp_wait): Likewise.
> 
> --- lin-lwp.c.orig      2002-12-15 16:44:49.000000000 -0500
> +++ lin-lwp.c   2002-12-15 17:00:27.000000000 -0500
> @@ -156,6 +156,14 @@ static sigset_t blocked_mask;
> 
>  /* Prototypes for local functions.  */
>  static int stop_wait_callback (struct lwp_info *lp, void *data);
> +
> +static int linux_record_stopped_pid (int pid);
> +
> +static int
> +linux_record_stopped_pid (int pid)
> +{
> +  return 0;
> +}
> 
>  /* Convert wait status STATUS to a string.  Used for printing debug
>     messages only.  */
> @@ -972,6 +980,19 @@ child_wait (ptid_t ptid, struct target_w
>           save_errno = EINTR;
>         }
> 
> +      /* If we expect to receive stopped processes after fork, vfork, and
> +        clone events, then just add them to the list and go back to
> +        waiting for the event to be reported.  The stopped process can
> +        be returned from waitpid before or after the event is.
> +        If we want to handle debugging of CLONE_PTRACE processes we need
> +        to do more here, for instance enabling lin_lwp_ops.  */
> +      if (pid != -1 && WIFSTOPPED (status) && pid != GET_PID (inferior_ptid))
> +       if (linux_record_stopped_pid (pid))
> +         {
> +           pid = -1;
> +           save_errno = EINTR;
> +         }
> +

(1) I would think you would check WSTOPSIG (status) == SIGSTOP.
Is there some flag you can check, to determine whether we are
supposed to be debugging forks (or otherwise-unexpected children)?
I'd like to see that check, and some sort of error/warning otherwise.

(2) What's the rationalle for making pid = -1, when it wasn't before?
What's the rationalle for EINTR?


>        clear_sigio_trap ();
>        clear_sigint_trap ();
>      }
> @@ -1110,6 +1131,22 @@ lin_lwp_wait (ptid_t ptid, struct target
>               continue;
>             }
> 
> +         /* If we expect to receive stopped processes after fork, vfork, and
> +            clone events, then just add them to the list and go back to
> +            waiting for the event to be reported.  The stopped process can
> +            be returned from waitpid before or after the event is.  */
> +         if (WIFSTOPPED (status) && ! lp && linux_record_stopped_pid (lwpid))
> +           {
> +             status = 0;
> +             continue;
> +           }

See (1) above.
Otherwise looks good, I think.

> +         /* NOTE drow/2002-12-15: This code seems to be for debugging
> +            CLONE_PTRACE processes which do not use the thread library.
> +            That's currently disabled; if it is enabled then this should
> +            be broken out into a function so that we can also handle
> +            new LWPs if they are reported by the extended events
> +            interface.  */
>           if (! lp)
>             {
>               lp = add_lwp (BUILD_LWP (lwpid, GET_PID (inferior_ptid)));