Re: [PATCH, gdbserver] Scan for existing threads during attachment on Linux

[Pedro Alves <pedro at codesourcery dot com>]

On Monday 23 May 2011 16:45:08, Luis Machado wrote:
> 2011-05-23  Luis Machado  <lgustavo@codesourcery.com>
> 
>         * linux-low.c (tgid_of_pid): New function.
>         (linux_attach): Scan for existing threads when attaching to a
>         process that is the tgid.
> 
> --- .pc/stop_threads.diff/gdb/gdbserver/linux-low.c     2011-05-23 12:05:42.447095000 -0300
> +++ gdb/gdbserver/linux-low.c   2011-05-23 12:40:47.735095000 -0300
> @@ -495,6 +495,38 @@ get_stop_pc (struct lwp_info *lwp)
>    return stop_pc;
>  }
>  
> +/* Extract the tgid (thread group leader id) of PID.  Return
> +   the tgid if successful and 0 otherwise.  */
> +
> +static unsigned long
> +tgid_of_pid (unsigned long pid)
> +{
> +  char pathname[128];
> +  FILE *fd;
> +  unsigned long tgid = 0;
> +
> +  sprintf (pathname, "/proc/%ld/stat", pid);
> +
> +  fd = fopen (pathname, "r");
> +
> +  if (fd != NULL)
> +    {
> +      int proc_id, ppid, pgrp;
> +      char comm[NAME_MAX + 1], state;
> +
> +      /* Extract the pgrp field.  */
> +      fscanf (fd, "%d %s %c %d %d", &proc_id, comm, &state, &ppid, &pgrp);

Wait, is that field really the process group?
That is not the same as the thread group.

> +      fclose (fd);
> +      tgid = pgrp;
> +    }
> +  else
> +    {
> +      fprintf (stderr, "Could not open /proc/%ld/stat.\n", pid);
> +      fflush (stderr);
> +    }
> +    return tgid;
> +}
> +
>  static void *
>  add_lwp (ptid_t ptid)
>  {
> @@ -586,7 +618,9 @@ linux_attach_lwp_1 (unsigned long lwpid,
>      }
>  
>    if (initial)
> -    /* NOTE/FIXME: This lwp might have not been the tgid.  */
> +    /* If lwp is the tgid, we handle adding existing threads later.
> +       Otherwise we just add lwp without bothering about any other
> +       threads.  */
>      ptid = ptid_build (lwpid, lwpid, 0);
>    else
>      {
> @@ -621,8 +655,10 @@ linux_attach_lwp_1 (unsigned long lwpid,
>         In this case we want the process thread to stop.
>         This is handled by having linux_attach set last_resume_kind ==
>         resume_stop after we return.
> -       ??? If the process already has several threads we leave the other
> -       threads running.
> +
> +       If the pid we are attaching to is also the tgid, we attach to and
> +       stop all the existing threads.  Otherwise, we attach to pid and
> +       ignore any other threads in the same group as this pid.
>  
>       3) GDB is connecting to gdbserver and is requesting an enumeration of all
>         existing threads.
> @@ -646,22 +682,93 @@ linux_attach_lwp (unsigned long lwpid)
>    linux_attach_lwp_1 (lwpid, 0);
>  }
>  
> +/* Attach to PID.  If PID is the tgdi, attach to it and stop
> +   all of its threads.  */
> +
>  int
>  linux_attach (unsigned long pid)
>  {
> +  struct thread_info *thread;
> +
> +  /* Attach to PID.  We will check for other threads
> +     soon.  */
>    linux_attach_lwp_1 (pid, 1);
>    linux_add_process (pid, 1);
>  
>    if (!non_stop)
>      {
> -      struct thread_info *thread;
> -
> -     /* Don't ignore the initial SIGSTOP if we just attached to this
> -       process.  It will be collected by wait shortly.  */
>        thread = find_thread_ptid (ptid_build (pid, pid, 0));
>        thread->last_resume_kind = resume_stop;
>      }
>  
> +  if (tgid_of_pid (pid) == pid)
> +    {
> +      DIR *dir;
> +      char pathname[128];
> +
> +      sprintf (pathname, "/proc/%ld/task", pid);
> +
> +      dir = opendir (pathname);
> +
> +      if (!dir)
> +       {
> +         fprintf (stderr, "Could not open /proc/%ld/task.\n", pid);
> +         fflush (stderr);
> +       }
> +      else
> +       {
> +         /* At this point we attached to the tgid.  Scan the task for
> +            existing threads.  */
> +         unsigned long lwp;
> +         int new_threads_found;
> +         int iterations = 0;
> +         struct dirent *dp;
> +
> +         while (iterations < 2)
> +           {
> +             new_threads_found = 0;
> +             /* Add all the other threads.  While we go through the
> +                threads, new threads may be spawned.  Cycle through
> +                the list of threads until we have done two iterations without
> +                finding new threads.  */
> +             while ((dp = readdir (dir)) != NULL)
> +               {
> +                 /* Fetch one lwp.  */
> +                 lwp = strtoul (dp->d_name, NULL, 10);
> +
> +                 /* Is this a new thread?  */
> +                 if (lwp && find_thread_ptid (ptid_build (pid, lwp, 0)) == NULL)

Line too long.

> +                   {
> +                     linux_attach_lwp_1 (lwp, 0);
> +                     new_threads_found++;
> +
> +                     /* Mark the threads as stopped.  */
> +                     if (!non_stop)
> +                       {
> +                         thread = find_thread_ptid (ptid_build (pid, lwp, 0));
> +
> +                         /* Check if we actually attached to this thread.  It
> +                            may have failed.  */
> +                         if (thread)
> +                           thread->last_resume_kind = resume_stop;
> +                       }

One more thing... I'm curious to know whether setting resume_stop in
the threads actually made a difference?  server.c does a mywait just
after attaching, to collect the stop of the initial thread, and, given
we're in all-stop mode, that freezes all threads with resume_stop
anyway.

> +
> +                     if (debug_threads)
> +                       fprintf (stderr, "Found and attached to new lwp %ld\n", lwp);

This line too long.

> +                   }
> +               }
> +
> +             if (!new_threads_found)
> +               iterations++;
> +             else
> +               iterations = 0;
> +
> +             rewinddir (dir);
> +           }
> +         closedir (dir);
> +       }
> +    }
> +
>    return 0;
>  }

-- 
Pedro Alves