Re: [PATCH] Stop threads when attaching to a PID that is the tgid

[Luis Machado <lgustavo@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]

Hi,

On 08/23/2011 02:48 PM, Pedro Alves wrote:
> On Tuesday 23 August 2011 14:59:55, Luis Machado wrote:
>> Hi,
>>
>
> Missing explanation as to _why_ we need this.  Here's one:
>
> On linux/ptrace, a ptracer needs to attach to each
> and every thread of a process individually.  Currently, when you
> do `gdbserver --attach PID', GDBserver only attaches to the PID thread,
> leaving all the other threads of the process running free. This is because
> GDBserver relies on thread_db to list the threads of PID, and, in order
> to activate thread_db, GDBserver needs to query GDB about some symbols,
> which obviously can only work once GDB connects..
>
> To fix this, this patch teaches GDBServer to list threads using /proc,
> instead of relying on thread_db, and attach/stop all of them.
>
>> This patch teaches GDBServer how to stop threads from a TID that is also
>> the TGID, without having to rely on GDB to list and stop them upon
>> connection.
>
>
>>    2011-08-23  Luis Machado<lgustavo@codesourcery.com>
>>
>>          * linux-low.c: Include linux-procfs.h.
>>          (linux_attach_lwp_1): Update comments.
>>          (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-08-23 10:38:50.653049001 -0300
>> +++ gdb/gdbserver/linux-low.c   2011-08-23 10:58:02.817049000 -0300
>> @@ -26,6 +26,7 @@
>>   #include<sys/param.h>
>>   #include<sys/ptrace.h>
>>   #include "linux-ptrace.h"
>> +#include "linux-procfs.h"
>
> Sorry I missed this before.  This needs an update to the linux-low.o
> rule in Makefile.in.  gdbserver doesn't do automatic header
> dependencies...  This is okay otherwise.
>

Thanks, i've checked the following in.

[-- Attachment #2: stop_threads.diff --]
[-- Type: text/x-patch, Size: 4645 bytes --]

2011-08-24  Luis Machado  <lgustavo@codesourcery.com>

	* linux-low.c: Include linux-procfs.h.
	(linux_attach_lwp_1): Update comments.
	(linux_attach): Scan for existing threads when attaching to a
	process that is the tgid.
	* Makefile.in: Update dependencies.

--- .pc/stop_threads.diff/gdb/gdbserver/linux-low.c	2011-08-24 09:19:15.757049000 -0300
+++ gdb/gdbserver/linux-low.c	2011-08-24 09:19:33.965049000 -0300
@@ -26,6 +26,7 @@
 #include <sys/param.h>
 #include <sys/ptrace.h>
 #include "linux-ptrace.h"
+#include "linux-procfs.h"
 #include <signal.h>
 #include <sys/ioctl.h>
 #include <fcntl.h>
@@ -586,7 +587,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 +624,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,9 +651,14 @@ linux_attach_lwp (unsigned long lwpid)
   linux_attach_lwp_1 (lwpid, 0);
 }
 
+/* Attach to PID.  If PID is the tgid, attach to it and all
+   of its threads.  */
+
 int
 linux_attach (unsigned long pid)
 {
+  /* Attach to PID.  We will check for other threads
+     soon.  */
   linux_attach_lwp_1 (pid, 1);
   linux_add_process (pid, 1);
 
@@ -662,6 +672,65 @@ linux_attach (unsigned long pid)
       thread->last_resume_kind = resume_stop;
     }
 
+  if (linux_proc_get_tgid (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)
+		    {
+		      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);
+	}
+    }
+
   return 0;
 }
 
--- .pc/stop_threads.diff/gdb/gdbserver/Makefile.in	2011-08-24 09:19:15.709049000 -0300
+++ gdb/gdbserver/Makefile.in	2011-08-24 09:19:33.981049000 -0300
@@ -354,6 +354,8 @@ linux_ptrace_h = $(srcdir)/../common/lin
 
 gdb_thread_db_h = $(srcdir)/../common/gdb_thread_db.h
 
+linux_procfs_h = $(srcdir)/../common/linux-procfs.h
+
 lynx_low_h = $(srcdir)/lynx-low.h $(srcdir)/server.h
 
 nto_low_h = $(srcdir)/nto-low.h
@@ -405,6 +407,9 @@ gdbreplay.o: gdbreplay.c config.h
 signals.o: ../common/signals.c $(server_h) $(signals_def)
 	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
 
+linux-procfs.o: ../common/linux-procfs.c $(server_h)
+	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
+
 common-utils.o: ../common/common-utils.c $(server_h)
 	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
 
@@ -443,7 +448,8 @@ i386-low.o: i386-low.c $(i386_low_h) $(s
 
 i387-fp.o: i387-fp.c $(server_h)
 
-linux-low.o: linux-low.c $(linux_low_h) $(linux_ptrace_h) $(server_h) $(linux_osdata_h)
+linux-low.o: linux-low.c $(linux_low_h) $(linux_ptrace_h) $(linux_procfs_h) \
+	$(server_h) $(linux_osdata_h)
 	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< @USE_THREAD_DB@
 
 linux-arm-low.o: linux-arm-low.c $(linux_low_h) $(server_h) \