gdbserver: Leave already-vCont-resumed threads as they were

Currently GDB never sends more than one action per vCont packet, when
connected in non-stop mode.  A follow up patch will change that, and
it exposed a gdbserver problem with the vCont handling.

For example, this in non-stop mode:

  => vCont;s:p1.1;c
  <= OK

Should be equivalent to:

  => vCont;s:p1.1
  <= OK
  => vCont;c
  <= OK

But gdbserver currently doesn't handle this.  In the latter case,
"vCont;c" makes gdbserver clobber the previous step request.  This
patch fixes that.

Note the server side must ignore resume actions for the thread that
has a pending %Stopped notification (and any other threads with events
pending), until GDB acks the notification with vStopped.  Otherwise,
e.g., the following case is mishandled:

 #1 => g  (or any other packet)
 #2 <= [registers]
 #3 <= %Stopped T05 thread:p1.2
 #4 => vCont s:p1.1;c
 #5 <= OK

Above, the server must not resume thread p1.2 when it processes the
vCont.  GDB can't know that p1.2 stopped until it acks the %Stopped
notification.  (Otherwise it wouldn't send a default "c" action.)

(The vCont documentation already specifies this.)

Finally, special care must also be given to handling fork/vfork
events.  A (v)fork event actually tells us that two processes stopped
-- the parent and the child.  Until we follow the fork, we must not
resume the child.  Therefore, if we have a pending fork follow, we
must not send a global wildcard resume action (vCont;c).  We can still
send process-wide wildcards though.

(The comments above will be added as code comments to gdb in a follow
up patch.)

gdb/gdbserver/ChangeLog:
2016-10-26  Pedro Alves  <palves@redhat.com>

	* linux-low.c (handle_extended_wait): Link parent/child fork
	threads.
	(linux_wait_1): Unlink them.
	(linux_set_resume_request): Ignore resume requests for
	already-resumed and unhandled fork child threads.
	* linux-low.h (struct lwp_info) <fork_relative>: New field.
	* server.c (in_queued_stop_replies_ptid, in_queued_stop_replies):
	New functions.
	(handle_v_requests) <vCont>: Don't call require_running.
	* server.h (in_queued_stop_replies): New declaration.

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 437bb4cd53c0ef539d9abfef70aceaee4868e737..1a9c4e5f9d5b70b15efc0257c2b7016b5a01c59c 100644 (file)
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,16 @@
+2016-10-26  Pedro Alves  <palves@redhat.com>
+
+       * linux-low.c (handle_extended_wait): Link parent/child fork
+       threads.
+       (linux_wait_1): Unlink them.
+       (linux_set_resume_request): Ignore resume requests for
+       already-resumed and unhandled fork child threads.
+       * linux-low.h (struct lwp_info) <fork_relative>: New field.
+       * server.c (in_queued_stop_replies_ptid, in_queued_stop_replies):
+       New functions.
+       (handle_v_requests) <vCont>: Don't call require_running.
+       * server.h (in_queued_stop_replies): New declaration.
+
 2016-10-24  Yao Qi  <yao.qi@linaro.org>
 
        PR server/20733

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 94c5bbee0ae3f88fcd16e40dbbffda7b130c56e6..f43ce7eccf67478cca5fd761e452b61053cd887a 100644 (file)
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -589,6 +589,11 @@ handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat)
          event_lwp->status_pending_p = 1;
          event_lwp->status_pending = wstat;
 
+         /* Link the threads until the parent event is passed on to
+            higher layers.  */
+         event_lwp->fork_relative = child_lwp;
+         child_lwp->fork_relative = event_lwp;
+
          /* If the parent thread is doing step-over with single-step
             breakpoints, the list of single-step breakpoints are cloned
             from the parent's.  Remove them from the child process.
@@ -3853,6 +3858,15 @@ linux_wait_1 (ptid_t ptid,
     {
       /* If the reported event is an exit, fork, vfork or exec, let
         GDB know.  */
+
+      /* Break the unreported fork relationship chain.  */
+      if (event_child->waitstatus.kind == TARGET_WAITKIND_FORKED
+         || event_child->waitstatus.kind == TARGET_WAITKIND_VFORKED)
+       {
+         event_child->fork_relative->fork_relative = NULL;
+         event_child->fork_relative = NULL;
+       }
+
       *ourstatus = event_child->waitstatus;
       /* Clear the event lwp's waitstatus since we handled it already.  */
       event_child->waitstatus.kind = TARGET_WAITKIND_IGNORE;
@@ -4654,6 +4668,51 @@ linux_set_resume_request (struct inferior_list_entry *entry, void *arg)
              continue;
            }
 
+         /* Ignore (wildcard) resume requests for already-resumed
+            threads.  */
+         if (r->resume[ndx].kind != resume_stop
+             && thread->last_resume_kind != resume_stop)
+           {
+             if (debug_threads)
+               debug_printf ("already %s LWP %ld at GDB's request\n",
+                             (thread->last_resume_kind
+                              == resume_step)
+                             ? "stepping"
+                             : "continuing",
+                             lwpid_of (thread));
+             continue;
+           }
+
+         /* Don't let wildcard resumes resume fork children that GDB
+            does not yet know are new fork children.  */
+         if (lwp->fork_relative != NULL)
+           {
+             struct inferior_list_entry *inf, *tmp;
+             struct lwp_info *rel = lwp->fork_relative;
+
+             if (rel->status_pending_p
+                 && (rel->waitstatus.kind == TARGET_WAITKIND_FORKED
+                     || rel->waitstatus.kind == TARGET_WAITKIND_VFORKED))
+               {
+                 if (debug_threads)
+                   debug_printf ("not resuming LWP %ld: has queued stop reply\n",
+                                 lwpid_of (thread));
+                 continue;
+               }
+           }
+
+         /* If the thread has a pending event that has already been
+            reported to GDBserver core, but GDB has not pulled the
+            event out of the vStopped queue yet, likewise, ignore the
+            (wildcard) resume request.  */
+         if (in_queued_stop_replies (entry->id))
+           {
+             if (debug_threads)
+               debug_printf ("not resuming LWP %ld: has queued stop reply\n",
+                             lwpid_of (thread));
+             continue;
+           }
+
          lwp->resume = &r->resume[ndx];
          thread->last_resume_kind = lwp->resume->kind;
 

diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 5057e661406bf997d556fd48dd6cb0d043581371..476816db4821f3ef9a49b1afd407fea2586c7394 100644 (file)
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -301,6 +301,12 @@ struct lwp_info
      information or exit status until it can be reported to GDB.  */
   struct target_waitstatus waitstatus;
 
+  /* A pointer to the fork child/parent relative.  Valid only while
+     the parent fork event is not reported to higher layers.  Used to
+     avoid wildcard vCont actions resuming a fork child before GDB is
+     notified about the parent's fork event.  */
+  struct lwp_info *fork_relative;
+
   /* When stopped is set, this is where the lwp last stopped, with
      decr_pc_after_break already accounted for.  If the LWP is
      running, this is the address at which the lwp was resumed.  */

diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 2996e19be29c3d4ab04d5c8d6f966e46b68c6ef6..3f9ff2bbf49658b3f330b29ceba26d9736c51297 100644 (file)
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -193,6 +193,38 @@ vstop_notif_reply (struct notif_event *event, char *own_buf)
   prepare_resume_reply (own_buf, vstop->ptid, &vstop->status);
 }
 
+/* QUEUE_iterate callback helper for in_queued_stop_replies.  */
+
+static int
+in_queued_stop_replies_ptid (QUEUE (notif_event_p) *q,
+                            QUEUE_ITER (notif_event_p) *iter,
+                            struct notif_event *event,
+                            void *data)
+{
+  ptid_t filter_ptid = *(ptid_t *) data;
+  struct vstop_notif *vstop_event = (struct vstop_notif *) event;
+
+  if (ptid_match (vstop_event->ptid, filter_ptid))
+    return 0;
+
+  /* Don't resume fork children that GDB does not know about yet.  */
+  if ((vstop_event->status.kind == TARGET_WAITKIND_FORKED
+       || vstop_event->status.kind == TARGET_WAITKIND_VFORKED)
+      && ptid_match (vstop_event->status.value.related_pid, filter_ptid))
+    return 0;
+
+  return 1;
+}
+
+/* See server.h.  */
+
+int
+in_queued_stop_replies (ptid_t ptid)
+{
+  return !QUEUE_iterate (notif_event_p, notif_stop.queue,
+                        in_queued_stop_replies_ptid, &ptid);
+}
+
 struct notif_server notif_stop =
 {
   "vStopped", "Stop", NULL, vstop_notif_reply,
@@ -2947,7 +2979,6 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
 
       if (startswith (own_buf, "vCont;"))
        {
-         require_running (own_buf);
          handle_v_cont (own_buf);
          return;
        }

diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 51b219153f565568016e145c02dcd74ac8347b12..f56c0f5eca5a4a4bcba7789dde1dc41aa329fdfa 100644 (file)
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -123,6 +123,10 @@ extern int handle_target_event (int err, gdb_client_data client_data);
 /* Get rid of the currently pending stop replies that match PTID.  */
 extern void discard_queued_stop_replies (ptid_t ptid);
 
+/* Returns true if there's a pending stop reply that matches PTID in
+   the vStopped notifications queue.  */
+extern int in_queued_stop_replies (ptid_t ptid);
+
 #include "remote-utils.h"
 
 #include "utils.h"