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

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

[Luis Machado]

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

Hi,

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.

In case the PID being attached to is not the TGID, GDBServer attaches to 
such a PID in the usual way, without stopping any other threads.

Tested without regressions.

Ok?

Luis

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

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"
 #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;
 }
 

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

[Pedro Alves]

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.

-- 
Pedro Alves

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

[Luis Machado]

[-- 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) \