Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 4/5] gdbserver: Leave already-vCont-resumed threads as they were
Date: Wed, 17 Feb 2016 02:51:00 -0000	[thread overview]
Message-ID: <1455677091-13683-5-git-send-email-palves@redhat.com> (raw)
In-Reply-To: <1455677091-13683-1-git-send-email-palves@redhat.com>

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-02-16  Pedro Alves  <palves@redhat.com>

	* linux-low.c (linux_set_resume_request): Ignore resume requests
	for already-resumed threads.
	* server.c (in_queued_stop_replies_ptid, in_queued_stop_replies):
	New functions.
	* server.h (in_queued_stop_replies): New declaration.
---
 gdb/gdbserver/linux-low.c | 27 +++++++++++++++++++++++++++
 gdb/gdbserver/server.c    | 33 ++++++++++++++++++++++++++++++++-
 gdb/gdbserver/server.h    |  4 ++++
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 8b025bd..2cac4c0 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4465,6 +4465,33 @@ linux_set_resume_request (struct inferior_list_entry *entry, void *arg)
 	      continue;
 	    }
 
+	  /* Ignore (wildcard) resume requests for already-resumed
+	     requests.  */
+	  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;
+	    }
+
+	  /* 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/server.c b/gdb/gdbserver/server.c
index ef715e7..660ee5b 100644
--- 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,
@@ -2949,7 +2981,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 3d78fb3..68a3670 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -119,6 +119,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"
-- 
1.9.3


  parent reply	other threads:[~2016-02-17  2:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-17  2:44 [PATCH 0/5] Coalesce/aggregate (async) vCont packets/actions Pedro Alves
2016-02-17  2:44 ` [PATCH 1/5] gdb: Clean up remote.c:remote_resume Pedro Alves
2016-02-17 11:45   ` Luis Machado
2016-02-17 12:32     ` Pedro Alves
2016-02-17 13:44       ` Luis Machado
2016-02-17  2:44 ` [PATCH 2/5] gdb: Free inferior->priv when inferior exits Pedro Alves
2016-02-17  2:44 ` [PATCH 3/5] gdb/doc: Clarify vCont packet description Pedro Alves
2016-02-17 15:56   ` Eli Zaretskii
2016-02-17  2:51 ` Pedro Alves [this message]
2016-02-17 11:46   ` [PATCH 4/5] gdbserver: Leave already-vCont-resumed threads as they were Luis Machado
2016-02-17 12:32     ` Pedro Alves
2016-10-26 15:39       ` Pedro Alves
2016-02-17  2:51 ` [PATCH 5/5] gdb: Coalesce/aggregate (async) vCont packets/actions Pedro Alves
2016-02-17 13:42   ` Luis Machado
2016-10-26 15:35     ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1455677091-13683-5-git-send-email-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox