[PATCH] PR server/9684: gdbserver, attach to stopped processes

[PATCH] PR server/9684: gdbserver, attach to stopped processes

[Pedro Alves]

This teaches GDBserver to attach to job control stopped processes 'T
(stopped)', using the same trick GDB uses.

Refs:
 http://sourceware.org/ml/gdb-patches/2008-04/msg00241.html
 http://sourceware.org/ml/gdb-patches/2008-04/msg00028.html
 http://sourceware.org/ml/gdb-patches/2007-06/msg00059.html

This patch is simpler than linux-nat's equivalent patch was, given
that GDBserver doesn't block waiting for lwps to report the attach
stop.  Instead all that is handled through the regular target_wait
path.  That and that linux-nat.c's state at the time of that patch was
a little messy.  :-)

I'm a bit ambivalent on this.  I found out that this isn't actually
necessary on recent kernels (I haven't looked for the exact version or
commit that made it so) -- waitpid no longer hangs.  OTOH, we still
need it on systems with not so recent kernels where support is still
well active, so I figure this may be useful to many upstream as well.
Note that once we have PTRACE_SEIZE support this hack will definitely
be unnecessary, so it's likely that the window of kernels where this
is actually unnecessary, but used, will be narrow-ish (assuming I or
someone else gets to PTRACE_SEIZE in reasonable time).

Tested with the extended-remote board on x86_64 Fedora 16 (3.2,
doesn't need patch), and x86_64 RHEL 5.8 (2.6.18, needs patch).

2012-02-24  Pedro Alves  <palves@redhat.com>

	PR server/9684
	* linux-low.c (pid_is_stopped): New.
	(linux_attach_lwp_1): Handle attaching to 'T (stopped)' processes.
---
 0 files changed, 0 insertions(+), 0 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index ab34d84..11c53e6 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -598,6 +598,37 @@ linux_create_inferior (char *program, char **allargs)
   return pid;
 }
 
+/* Detect `T (stopped)' in `/proc/PID/status'.
+   Other states including `T (tracing stop)' are reported as false.  */
+
+static int
+pid_is_stopped (pid_t pid)
+{
+  FILE *status_file;
+  char buf[100];
+  int retval = 0;
+
+  snprintf (buf, sizeof (buf), "/proc/%d/status", (int) pid);
+  status_file = fopen (buf, "r");
+  if (status_file != NULL)
+    {
+      int have_state = 0;
+
+      while (fgets (buf, sizeof (buf), status_file))
+	{
+	  if (strncmp (buf, "State:", 6) == 0)
+	    {
+	      have_state = 1;
+	      break;
+	    }
+	}
+      if (have_state && strstr (buf, "T (stopped)") != NULL)
+	retval = 1;
+      fclose (status_file);
+    }
+  return retval;
+}
+
 /* Attach to an inferior process.  */
 
 static void
@@ -643,6 +674,33 @@ linux_attach_lwp_1 (unsigned long lwpid, int initial)
      ptrace call on this LWP.  */
   new_lwp->must_set_ptrace_flags = 1;
 
+  if (pid_is_stopped (lwpid))
+    {
+      if (debug_threads)
+	fprintf (stderr,
+		 "Attached to a stopped process\n");
+
+      /* The process is definitely stopped.  It is in a job control
+	 stop, unless the kernel predates the TASK_STOPPED /
+	 TASK_TRACED distinction, in which case it might be in a
+	 ptrace stop.  Make sure it is in a ptrace stop; from there we
+	 can kill it, signal it, et cetera.
+
+         First make sure there is a pending SIGSTOP.  Since we are
+	 already attached, the process can not transition from stopped
+	 to running without a PTRACE_CONT; so we know this signal will
+	 go into the queue.  The SIGSTOP generated by PTRACE_ATTACH is
+	 probably already in the queue (unless this kernel is old
+	 enough to use TASK_STOPPED for ptrace stops); but since
+	 SIGSTOP is not an RT signal, it can only be queued once.  */
+      kill_lwp (lwpid, SIGSTOP);
+
+      /* Finally, resume the stopped process.  This will deliver the
+	 SIGSTOP (or a higher priority signal, just like normal
+	 PTRACE_ATTACH), which we'll catch later on.  */
+      ptrace (PTRACE_CONT, lwpid, 0, 0);
+    }
+
   /* The next time we wait for this LWP we'll see a SIGSTOP as PTRACE_ATTACH
      brings it to a halt.
 

Re: [PATCH] PR server/9684: gdbserver, attach to stopped processes

[Joel Brobecker]

> I'm a bit ambivalent on this.  I found out that this isn't actually
> necessary on recent kernels (I haven't looked for the exact version or
> commit that made it so) -- waitpid no longer hangs.  OTOH, we still
> need it on systems with not so recent kernels where support is still
> well active, so I figure this may be useful to many upstream as well.

FWIW, I tend to agree. If it is possibly useful to others, and it's
not intrusive, it is all good reasons for the patch to go in. I took
a look just to have an idea of what it's about, and it seems like
a reasonably uninvasive patch...

As always, I always marvel at the beautiful and informative comments
that explain why we have to do all these horrible things :-).

-- 
Joel

Re: [PATCH] PR server/9684: gdbserver, attach to stopped processes

[Jan Kratochvil]

On Fri, 24 Feb 2012 23:21:27 +0100, Pedro Alves wrote:
> Tested with the extended-remote board on x86_64 Fedora 16 (3.2,
> doesn't need patch), and x86_64 RHEL 5.8 (2.6.18, needs patch).

Looks OK to me, thanks.

> +      /* The process is definitely stopped.  It is in a job control
> +	 stop, unless the kernel predates the TASK_STOPPED /
+
> +         First make sure there is a pending SIGSTOP.  Since we are
> +	 already attached, the process can not transition from stopped
+
> +      /* Finally, resume the stopped process.  This will deliver the
> +	 SIGSTOP (or a higher priority signal, just like normal

There are spaces instead of tabs.


Jan

Re: [PATCH] PR server/9684: gdbserver, attach to stopped processes

[Yao Qi]

On 02/25/2012 06:21 AM, Pedro Alves wrote:
> +/* Detect `T (stopped)' in `/proc/PID/status'.
> +   Other states including `T (tracing stop)' are reported as false.  */
> +
> +static int
> +pid_is_stopped (pid_t pid)
> +{
> +  FILE *status_file;
> +  char buf[100];
> +  int retval = 0;
> +
> +  snprintf (buf, sizeof (buf), "/proc/%d/status", (int) pid);
> +  status_file = fopen (buf, "r");
> +  if (status_file != NULL)
> +    {
> +      int have_state = 0;
> +
> +      while (fgets (buf, sizeof (buf), status_file))
> +	{
> +	  if (strncmp (buf, "State:", 6) == 0)
> +	    {
> +	      have_state = 1;
> +	      break;
> +	    }
> +	}
> +      if (have_state && strstr (buf, "T (stopped)") != NULL)
> +	retval = 1;
> +      fclose (status_file);
> +    }
> +  return retval;
> +}

This function is exactly the same as linux-nat.c:pid_is_stopped, so
why don't we move them to common/linux-linuxprocfs.c?

-- 
Yao (齐尧)

Re: [PATCH] PR server/9684: gdbserver, attach to stopped processes

[Pedro Alves]

On 02/25/2012 03:57 AM, Jan Kratochvil wrote:
> On Fri, 24 Feb 2012 23:21:27 +0100, Pedro Alves wrote:
>> Tested with the extended-remote board on x86_64 Fedora 16 (3.2,
>> doesn't need patch), and x86_64 RHEL 5.8 (2.6.18, needs patch).
> 
> Looks OK to me, thanks.
> 
>> +      /* The process is definitely stopped.  It is in a job control
>> +	 stop, unless the kernel predates the TASK_STOPPED /
> +
>> +         First make sure there is a pending SIGSTOP.  Since we are
>> +	 already attached, the process can not transition from stopped
> +
>> +      /* Finally, resume the stopped process.  This will deliver the
>> +	 SIGSTOP (or a higher priority signal, just like normal
> 
> There are spaces instead of tabs.

Indeed, thanks.  Fixed that and applied.

-- 
Pedro Alves

move pid_is_stopped to common/

[Pedro Alves]

On 02/25/2012 05:56 AM, Yao Qi wrote:

> This function is exactly the same as linux-nat.c:pid_is_stopped, so
> why don't we move them to common/linux-linuxprocfs.c?

I've applied this as follow up.

Thanks,
-- 
Pedro Alves

2012-02-27  Pedro Alves  <palves@redhat.com>

	gdb/gdbserver/
	* linux-low.c (pid_is_stopped): Delete, moved to common/.
	(linux_attach_lwp_1): Adjust to use linux_proc_pid_is_stopped.

	gdb/
	* linux-nat.c (pid_is_stopped): Delete, moved to common/.
	(linux_nat_post_attach_wait): Adjust to use
	linux_proc_pid_is_stopped.

	* common/linux-procfs.h (linux_proc_pid_is_stopped): Declare.
	* common/linux-procfs.c (linux_proc_pid_is_stopped): New function,
	based on pid_is_stopped from both linux-nat.c and
	gdbserver/linux-low.c, and renamed.
---
 gdb/common/linux-procfs.c |   31 +++++++++++++++++++++++++++++++
 gdb/common/linux-procfs.h |    5 +++++
 gdb/gdbserver/linux-low.c |   33 +--------------------------------
 gdb/linux-nat.c           |   33 +--------------------------------
 4 files changed, 38 insertions(+), 64 deletions(-)

diff --git a/gdb/common/linux-procfs.c b/gdb/common/linux-procfs.c
index 421f36e..165383e 100644
--- a/gdb/common/linux-procfs.c
+++ b/gdb/common/linux-procfs.c
@@ -53,3 +53,34 @@ linux_proc_get_tgid (int lwpid)

   return tgid;
 }
+
+/* Detect `T (stopped)' in `/proc/PID/status'.
+   Other states including `T (tracing stop)' are reported as false.  */
+
+int
+linux_proc_pid_is_stopped (pid_t pid)
+{
+  FILE *status_file;
+  char buf[100];
+  int retval = 0;
+
+  snprintf (buf, sizeof (buf), "/proc/%d/status", (int) pid);
+  status_file = fopen (buf, "r");
+  if (status_file != NULL)
+    {
+      int have_state = 0;
+
+      while (fgets (buf, sizeof (buf), status_file))
+	{
+	  if (strncmp (buf, "State:", 6) == 0)
+	    {
+	      have_state = 1;
+	      break;
+	    }
+	}
+      if (have_state && strstr (buf, "T (stopped)") != NULL)
+	retval = 1;
+      fclose (status_file);
+    }
+  return retval;
+}
diff --git a/gdb/common/linux-procfs.h b/gdb/common/linux-procfs.h
index a4ba4a1..c1e5547 100644
--- a/gdb/common/linux-procfs.h
+++ b/gdb/common/linux-procfs.h
@@ -26,4 +26,9 @@

 extern int linux_proc_get_tgid (int lwpid);

+/* Detect `T (stopped)' in `/proc/PID/status'.
+   Other states including `T (tracing stop)' are reported as false.  */
+
+extern int linux_proc_pid_is_stopped (pid_t pid);
+
 #endif /* COMMON_LINUX_PROCFS_H */
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index f2887e6..8f57ee3 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -598,37 +598,6 @@ linux_create_inferior (char *program, char **allargs)
   return pid;
 }

-/* Detect `T (stopped)' in `/proc/PID/status'.
-   Other states including `T (tracing stop)' are reported as false.  */
-
-static int
-pid_is_stopped (pid_t pid)
-{
-  FILE *status_file;
-  char buf[100];
-  int retval = 0;
-
-  snprintf (buf, sizeof (buf), "/proc/%d/status", (int) pid);
-  status_file = fopen (buf, "r");
-  if (status_file != NULL)
-    {
-      int have_state = 0;
-
-      while (fgets (buf, sizeof (buf), status_file))
-	{
-	  if (strncmp (buf, "State:", 6) == 0)
-	    {
-	      have_state = 1;
-	      break;
-	    }
-	}
-      if (have_state && strstr (buf, "T (stopped)") != NULL)
-	retval = 1;
-      fclose (status_file);
-    }
-  return retval;
-}
-
 /* Attach to an inferior process.  */

 static void
@@ -674,7 +643,7 @@ linux_attach_lwp_1 (unsigned long lwpid, int initial)
      ptrace call on this LWP.  */
   new_lwp->must_set_ptrace_flags = 1;

-  if (pid_is_stopped (lwpid))
+  if (linux_proc_pid_is_stopped (lwpid))
     {
       if (debug_threads)
 	fprintf (stderr,
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 3731096..e426387 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1356,37 +1356,6 @@ exit_lwp (struct lwp_info *lp)
   delete_lwp (lp->ptid);
 }

-/* Detect `T (stopped)' in `/proc/PID/status'.
-   Other states including `T (tracing stop)' are reported as false.  */
-
-static int
-pid_is_stopped (pid_t pid)
-{
-  FILE *status_file;
-  char buf[100];
-  int retval = 0;
-
-  snprintf (buf, sizeof (buf), "/proc/%d/status", (int) pid);
-  status_file = fopen (buf, "r");
-  if (status_file != NULL)
-    {
-      int have_state = 0;
-
-      while (fgets (buf, sizeof (buf), status_file))
-	{
-	  if (strncmp (buf, "State:", 6) == 0)
-	    {
-	      have_state = 1;
-	      break;
-	    }
-	}
-      if (have_state && strstr (buf, "T (stopped)") != NULL)
-	retval = 1;
-      fclose (status_file);
-    }
-  return retval;
-}
-
 /* Wait for the LWP specified by LP, which we have just attached to.
    Returns a wait status for that LWP, to cache.  */

@@ -1397,7 +1366,7 @@ linux_nat_post_attach_wait (ptid_t ptid, int first, int *cloned,
   pid_t new_pid, pid = GET_LWP (ptid);
   int status;

-  if (pid_is_stopped (pid))
+  if (linux_proc_pid_is_stopped (pid))
     {
       if (debug_linux_nat)
 	fprintf_unfiltered (gdb_stdlog,

Re: [PATCH] PR server/9684: gdbserver, attach to stopped processes

[Pedro Alves]

On 02/24/2012 11:00 PM, Joel Brobecker wrote:
>> I'm a bit ambivalent on this.  I found out that this isn't actually
>> necessary on recent kernels (I haven't looked for the exact version or
>> commit that made it so) -- waitpid no longer hangs.  OTOH, we still
>> need it on systems with not so recent kernels where support is still
>> well active, so I figure this may be useful to many upstream as well.
> 
> FWIW, I tend to agree. If it is possibly useful to others, and it's
> not intrusive, it is all good reasons for the patch to go in. I took
> a look just to have an idea of what it's about, and it seems like
> a reasonably uninvasive patch...

Thanks for weighing in.

> As always, I always marvel at the beautiful and informative comments
> that explain why we have to do all these horrible things :-).

I can't take credit for this; I'm just copying Dan's comment
from linux-nat.c.

-- 
Pedro Alves