This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

gdbserver patch for PR server/10048


Marc is also tripping on the race described on PR server/10048:

 http://sourceware.org/ml/gdb/2009-05/msg00001.html

 http://sourceware.org/bugzilla/show_bug.cgi?id=10048

This patch fixes this particular race.  It doesn't make an attempt at fully
fixing the other attach-should-wait-for-SIGSTOP races Doug described in
linux_attach_lwp_1 (e.g., case 3), and I'm not currently planning on working on
those.  Unless someone else is (working on this), are there any objections
to this patch?  In any case, with multi-process support, this flag needs to be made
per-lwp, otherwise, the ptrace flags could be applied to the wrong lwp.

-- 
Pedro Alves

2009-05-04  Pedro Alves  <pedro@codesourcery.com>

       	PR server/10048

	* linux-low.c (must_set_ptrace_flags): Delete.
	(linux_create_inferior): Set `lwp->must_set_ptrace_flags' instead
	of the global.
	(linux_attach_lwp_1): Don't set PTRACE_SETOPTIONS here.  Set
	`lwp->must_set_ptrace_flags' instead.
	(linux_wait_for_event_1): If ptrace options here.
	(linux_wait_1): ... not here.

---
 gdb/gdbserver/linux-low.c |   27 ++++++++++++++-------------
 gdb/gdbserver/linux-low.h |    4 ++++
 2 files changed, 18 insertions(+), 13 deletions(-)

Index: src/gdb/gdbserver/linux-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-low.c	2009-04-30 23:58:04.000000000 +0100
+++ src/gdb/gdbserver/linux-low.c	2009-05-04 10:56:01.000000000 +0100
@@ -109,8 +109,6 @@ int stopping_threads;
 /* FIXME make into a target method?  */
 int using_threads = 1;
 
-static int must_set_ptrace_flags;
-
 /* This flag is true iff we've just created or attached to our first
    inferior but it has not stopped yet.  As soon as it does, we need
    to call the low target's arch_setup callback.  Doing this only on
@@ -319,7 +317,7 @@ add_lwp (ptid_t ptid)
 static int
 linux_create_inferior (char *program, char **allargs)
 {
-  void *new_lwp;
+  struct lwp_info *new_lwp;
   int pid;
   ptid_t ptid;
 
@@ -354,7 +352,7 @@ linux_create_inferior (char *program, ch
   ptid = ptid_build (pid, pid, 0);
   new_lwp = add_lwp (ptid);
   add_thread (ptid, new_lwp);
-  must_set_ptrace_flags = 1;
+  new_lwp->must_set_ptrace_flags = 1;
 
   return pid;
 }
@@ -383,10 +381,6 @@ linux_attach_lwp_1 (unsigned long lwpid,
 	       strerror (errno), errno);
     }
 
-  /* FIXME: This intermittently fails.
-     We need to wait for SIGSTOP first.  */
-  ptrace (PTRACE_SETOPTIONS, lwpid, 0, PTRACE_O_TRACECLONE);
-
   if (initial)
     /* NOTE/FIXME: This lwp might have not been the tgid.  */
     ptid = ptid_build (lwpid, lwpid, 0);
@@ -402,6 +396,11 @@ linux_attach_lwp_1 (unsigned long lwpid,
   new_lwp = (struct lwp_info *) add_lwp (ptid);
   add_thread (ptid, new_lwp);
 
+
+  /* We need to wait for SIGSTOP before being able to make the next
+     ptrace call on this LWP.  */
+  new_lwp->must_set_ptrace_flags = 1;
+
   /* The next time we wait for this LWP we'll see a SIGSTOP as PTRACE_ATTACH
      brings it to a halt.
 
@@ -996,6 +995,13 @@ linux_wait_for_event_1 (ptid_t ptid, int
 	  continue;
 	}
 
+      if (event_child->must_set_ptrace_flags)
+	{
+	  ptrace (PTRACE_SETOPTIONS, lwpid_of (event_child),
+		  0, PTRACE_O_TRACECLONE);
+	  event_child->must_set_ptrace_flags = 0;
+	}
+
       if (WIFSTOPPED (*wstat)
 	  && WSTOPSIG (*wstat) == SIGSTOP
 	  && event_child->stop_expected)
@@ -1258,11 +1264,6 @@ retry:
 
   lwp = get_thread_lwp (current_inferior);
 
-  if (must_set_ptrace_flags)
-    {
-      ptrace (PTRACE_SETOPTIONS, lwpid_of (lwp), 0, PTRACE_O_TRACECLONE);
-      must_set_ptrace_flags = 0;
-    }
   /* If we are waiting for a particular child, and it exited,
      linux_wait_for_event will return its exit status.  Similarly if
      the last child exited.  If this is not the last child, however,
Index: src/gdb/gdbserver/linux-low.h
===================================================================
--- src.orig/gdb/gdbserver/linux-low.h	2009-04-30 23:20:50.000000000 +0100
+++ src/gdb/gdbserver/linux-low.h	2009-05-04 10:56:01.000000000 +0100
@@ -146,6 +146,10 @@ struct lwp_info
      was a single-step.  */
   int stepping;
 
+  /* If this flag is set, we need to set the event request flags the
+     next time we see this LWP stop.  */
+  int must_set_ptrace_flags;
+
   /* If this is non-zero, it points to a chain of signals which need to
      be delivered to this process.  */
   struct pending_signals *pending_signals;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]