On 05/23/2011 07:37 AM, Pedro Alves wrote:
> On Friday 20 May 2011 19:42:02, Luis Machado wrote:
>
>
>> +/* Attach to PID.  If PID is the tgdi, attach to it and stop
>> +   all of its threads.  */
>> +
>>   int
>>   linux_attach (unsigned long pid)
>>   {
>> +  DIR *dir;
>> +  char pathname[128];
>> +  int is_tgid = 0;
>> +  FILE *fd;
>> +
>> +  /* Attach to PID.  We will check for other threads
>> +     soon.  */
>>     linux_attach_lwp_1 (pid, 1);
>> -  linux_add_process (pid, 1);
>>
>> -  if (!non_stop)
>> +  /* Find out what is the tgid of this lwp.  */
>> +  sprintf (pathname, "/proc/%ld/stat", pid);
>
> Can you factor this into a standalone function, something
> like
>
> static int
> tgid_of_pid (int pid)
> {
> ...
> }
>
> please?
>

Done.

>> +
>> +  fd = fopen (pathname, "r");
>> +
>> +  if (fd != NULL)
>>       {
>> -      struct thread_info *thread;
>> +      int proc_id, ppid, pgrp;
>> +      char comm[NAME_MAX + 1], state;
>>
>> -     /* 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;
>> +      fscanf (fd, "%d %s %c %d %d",&proc_id, comm,&state,&ppid,&pgrp);
>> +      fclose (fd);
>> +
>> +      if (pgrp == pid)
>> +       is_tgid = 1;
>> +    }
>> +  else
>> +    {
>> +      fprintf (stderr, "Could not open /proc/%ld/stat.\n", pid);
>> +      fflush (stderr);
>>       }
>>
>> +  sprintf (pathname, "/proc/%ld/task", pid);
>> +
>> +  dir = opendir (pathname);
>> +
>> +  if (!dir)
>> +    {
>> +      fprintf (stderr, "Could not open /proc/%ld/task.\n", pid);
>> +      fflush (stderr);
>> +    }
>
> Why even try to open the dir if is_tgid is false?
>

Doesn't make sense really. Fixed.

>> +  else if (is_tgid)
>> +    {
>> +      /* 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)
>> +               {
>> +                 linux_attach_lwp_1 (lwp, 0);
>> +                 new_threads_found++;
>> +
>> +                 if (debug_threads)
>> +                   fprintf (stderr, "Found and attached to new lwp %ld\n", lwp);
>> +               }
>> +           }
>> +
>> +         if (!new_threads_found)
>> +           iterations++;
>> +         else
>> +           iterations = 0;
>> +
>> +         rewinddir (dir);
>> +       }
>> +      closedir (dir);
>> +    }
>> +
>> +  linux_add_process (pid, 1);
>> +
>> +  /* Mark the threads as stopped.  */
>> +  if (!non_stop)
>> +    for_each_inferior (&all_threads, set_resume_kind_stop);
>> +
>>     return 0;
>>   }
>>
>
> Otherwise okay.
>

I've also addressed the two comments from the other reply. We're 
stopping only the threads from the PID we're attaching to.

The stale comments were updated to reflect the current situation. We can 
still have a lwp that isn't the tgid, but we handle the case of pid == 
tgid correctly now. The new comments reflect that.

Luis