[PATCH 5/5] gdb: Coalesce/aggregate (async) vCont packets/actions

Pedro Alves palves@redhat.com
Wed Oct 26 15:35:00 GMT 2016


Hi there.  I've finally got back to looking at this...

On 02/17/2016 01:42 PM, Luis Machado wrote:
> I missed 5/5... Mostly nits, overall it looks sane to me.
> 
> On 02/17/2016 12:44 AM, Pedro Alves wrote:
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index 15210c9..f72937c 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -2357,6 +2357,8 @@ do_target_resume (ptid_t resume_ptid, int step,
>> enum gdb_signal sig)
>>       target_pass_signals ((int) GDB_SIGNAL_LAST, signal_pass);
>>
>>     target_resume (resume_ptid, step, sig);
>> +
>> +  target_do_resume ();
>>   }
>>
> 
> I'm looking at the sequence of function names and they don't look too
> clear.
> 
> do_target_resume/target_resume/target_do_resume.
> 
> Should we have better names for these functions? Ones that make it more
> explicit what each function is doing and the fact that we are
> potentially defering resumptions? Like "queue_resume_actions" for
> target_resume or "commit_resumption_actions" for target_do_resume?

You're right.  I'm not looking forward to changing target_resume,
since that's a target method implemented by all targets.
But I changed target_do_resume to target_commit_resume.


>> +/* Check whether any event pending in the vStopped queue would prevent
>> +   a global or process wildcard vCont action.  Clear
>> +   *may_global_wildcard if we can't do a global wildcard (vCont;c),
>> +   and clear the event inferior's may_wildcard_vcont flag if we can do
>> +   a process-wide wildcard resume (vCont;c:pPID.-1).  */
>> +
> 
> ... inferior's may_wildcard_vcont flag if we can do ...
> 
> Can do or can't do?

Can't.  Fixed.

>> +/* Commit a series of resumption requests previously prepared with
>> +   target_resume calls.
>> +
>> +   GDB always call target_do_resume after calling target_resume or
>> +   more times.  A target may thus use this method in coordination with
> 
> 
> "GDB always calls ... target_resume one or more times."?

Fixed too.  Below's what I pushed to master.

This added a new cleanup, which would better done with a
scoped_restore nowadays.  I'll fix that as a follow up.

>From 85ad3aaf403d2104c82010494d3d4a93a36e2e6f Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 26 Oct 2016 11:08:28 +0100
Subject: [PATCH] gdb: Coalesce/aggregate (async) vCont packets/actions

Currently, with "maint set target-non-stop on", that is, when gdb
connects with the non-stop/asynchronous variant of the remote
protocol, even with "set non-stop off", GDB always sends one vCont
packet per thread resumed.  This patch makes GDB aggregate and
coalesce vCont packets, so we send vCont packets like "vCont;s:p1.1;c"
in non-stop mode too.

Basically, this is done by:

  - Adding a new target method target_commit_resume that is called
    after calling target_resume one or more times.  When resuming a
    batch of threads, we'll only call target_commit_resume once after
    calling target_resume for all threads.

  - Making the remote target defer sending the actual vCont packet to
    target_commit_resume.

Special care must be taken to avoid sending a vCont action with a
"wildcard" thread-id (all threads of process / all threads) when that
would resume threads/processes that should not be resumed.  See
remote_commit_resume comments for details.

Unlike all-stop's remote_resume implementation, this handles the case
of too many actions resulting in a too-big vCont packet, by flushing
the vCont packet and starting a new one.

E.g., imagining that the "c" action in:

  vCont;s:1;c

overflows the packet buffer, we split the actions like:

  vCont;s:1
  vCont;c

Tested on x86_64 Fedora 20, with and without "maint set
target-non-stop on".

Also tested with a hack that makes remote_commit_resume flush the vCont
packet after every action appended (which caught a few bugs).

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

	* inferior.h (ALL_NON_EXITED_INFERIORS): New macro.
	* infrun.c (do_target_resume): Call target_commit_resume.
	(proceed): Defer target_commit_resume while looping over threads,
	resuming them.  Call target_commit_resume at the end.
	* record-btrace.c (record_btrace_commit_resume): New function.
	(init_record_btrace_ops): Install it as to_commit_resume method.
	* record-full.c (record_full_commit_resume): New function.
	(record_full_wait_1): Call the beneath target's to_commit_resume
	method.
	(init_record_full_ops): Install record_full_commit_resume as
	to_commit_resume method.
	* remote.c (struct private_thread_info) <last_resume_step,
	last_resume_sig, vcont_resumed>: New fields.
	(remote_add_thread): Set the new thread's vcont_resumed flag.
	(demand_private_info): Delete.
	(get_private_info_thread, get_private_info_ptid): New functions.
	(remote_update_thread_list): Adjust.
	(process_initial_stop_replies): Clear the thread's vcont_resumed
	flag.
	(remote_resume): If connected in non-stop mode, record the resume
	request and return early.
	(struct private_inferior): New.
	(struct vcont_builder): New.
	(vcont_builder_restart, vcont_builder_flush)
	(vcont_builder_push_action): New functions.
	(MAX_ACTION_SIZE): New macro.
	(remote_commit_resume): New function.
	(thread_pending_fork_status, is_pending_fork_parent_thread): New
	functions.
	(check_pending_event_prevents_wildcard_vcont_callback)
	(check_pending_events_prevent_wildcard_vcont): New functions.
	(process_stop_reply): Adjust.  Clear the thread's vcont_resumed
	flag.
	(init_remote_ops): Install remote_commit_resume.
	* target-delegates.c: Regenerate.
	* target.c (defer_target_commit_resume): New global.
	(target_commit_resume, make_cleanup_defer_target_commit_resume):
	New functions.
	* target.h (struct target_ops) <to_commit_resume>: New field.
	(target_resume): Update comments.
	(target_commit_resume): New declaration.
---
 gdb/ChangeLog          |  44 +++++
 gdb/inferior.h         |   6 +
 gdb/infrun.c           |   8 +
 gdb/record-btrace.c    |  11 ++
 gdb/record-full.c      |  11 ++
 gdb/remote.c           | 452 ++++++++++++++++++++++++++++++++++++++++++++++---
 gdb/target-delegates.c |  26 +++
 gdb/target.c           |  28 +++
 gdb/target.h           |  47 +++--
 9 files changed, 599 insertions(+), 34 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f8296b6..6e24938 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,49 @@
 2016-10-26  Pedro Alves  <palves@redhat.com>
 
+	* inferior.h (ALL_NON_EXITED_INFERIORS): New macro.
+	* infrun.c (do_target_resume): Call target_commit_resume.
+	(proceed): Defer target_commit_resume while looping over threads,
+	resuming them.  Call target_commit_resume at the end.
+	* record-btrace.c (record_btrace_commit_resume): New function.
+	(init_record_btrace_ops): Install it as to_commit_resume method.
+	* record-full.c (record_full_commit_resume): New function.
+	(record_full_wait_1): Call the beneath target's to_commit_resume
+	method.
+	(init_record_full_ops): Install record_full_commit_resume as
+	to_commit_resume method.
+	* remote.c (struct private_thread_info) <last_resume_step,
+	last_resume_sig, vcont_resumed>: New fields.
+	(remote_add_thread): Set the new thread's vcont_resumed flag.
+	(demand_private_info): Delete.
+	(get_private_info_thread, get_private_info_ptid): New functions.
+	(remote_update_thread_list): Adjust.
+	(process_initial_stop_replies): Clear the thread's vcont_resumed
+	flag.
+	(remote_resume): If connected in non-stop mode, record the resume
+	request and return early.
+	(struct private_inferior): New.
+	(struct vcont_builder): New.
+	(vcont_builder_restart, vcont_builder_flush)
+	(vcont_builder_push_action): New functions.
+	(MAX_ACTION_SIZE): New macro.
+	(remote_commit_resume): New function.
+	(thread_pending_fork_status, is_pending_fork_parent_thread): New
+	functions.
+	(check_pending_event_prevents_wildcard_vcont_callback)
+	(check_pending_events_prevent_wildcard_vcont): New functions.
+	(process_stop_reply): Adjust.  Clear the thread's vcont_resumed
+	flag.
+	(init_remote_ops): Install remote_commit_resume.
+	* target-delegates.c: Regenerate.
+	* target.c (defer_target_commit_resume): New global.
+	(target_commit_resume, make_cleanup_defer_target_commit_resume):
+	New functions.
+	* target.h (struct target_ops) <to_commit_resume>: New field.
+	(target_resume): Update comments.
+	(target_commit_resume): New declaration.
+
+2016-10-26  Pedro Alves  <palves@redhat.com>
+
 	* inferior.c (exit_inferior_1): Free 'priv'.
 
 2016-10-26  Pedro Alves  <palves@redhat.com>
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 54c6f65..300cc10 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -513,6 +513,12 @@ extern struct cleanup *save_current_inferior (void);
 #define ALL_INFERIORS(I) \
   for ((I) = inferior_list; (I); (I) = (I)->next)
 
+/* Traverse all non-exited inferiors.  */
+
+#define ALL_NON_EXITED_INFERIORS(I) \
+  ALL_INFERIORS (I)		    \
+    if ((I)->pid != 0)
+
 extern struct inferior *inferior_list;
 
 /* Prune away automatically added inferiors that aren't required
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 00bba16..5e62472 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2366,6 +2366,8 @@ do_target_resume (ptid_t resume_ptid, int step, enum gdb_signal sig)
     target_pass_signals ((int) GDB_SIGNAL_LAST, signal_pass);
 
   target_resume (resume_ptid, step, sig);
+
+  target_commit_resume ();
 }
 
 /* Resume the inferior, but allow a QUIT.  This is useful if the user
@@ -2984,6 +2986,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
   struct execution_control_state ecss;
   struct execution_control_state *ecs = &ecss;
   struct cleanup *old_chain;
+  struct cleanup *defer_resume_cleanup;
   int started;
 
   /* If we're stopped at a fork/vfork, follow the branch set by the
@@ -3125,6 +3128,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
      until the target stops again.  */
   tp->prev_pc = regcache_read_pc (regcache);
 
+  defer_resume_cleanup = make_cleanup_defer_target_commit_resume ();
+
   started = start_step_over ();
 
   if (step_over_info_valid_p ())
@@ -3189,6 +3194,9 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 	error (_("Command aborted."));
     }
 
+  do_cleanups (defer_resume_cleanup);
+  target_commit_resume ();
+
   discard_cleanups (old_chain);
 
   /* Tell the event loop to wait for it to stop.  If the target
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 257d0b0..4808129 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -2150,6 +2150,16 @@ record_btrace_resume (struct target_ops *ops, ptid_t ptid, int step,
     }
 }
 
+/* The to_commit_resume method of target record-btrace.  */
+
+static void
+record_btrace_commit_resume (struct target_ops *ops)
+{
+  if ((execution_direction != EXEC_REVERSE)
+      && !record_btrace_is_replaying (ops, minus_one_ptid))
+    ops->beneath->to_commit_resume (ops->beneath);
+}
+
 /* Cancel resuming TP.  */
 
 static void
@@ -2875,6 +2885,7 @@ init_record_btrace_ops (void)
   ops->to_get_unwinder = &record_btrace_to_get_unwinder;
   ops->to_get_tailcall_unwinder = &record_btrace_to_get_tailcall_unwinder;
   ops->to_resume = record_btrace_resume;
+  ops->to_commit_resume = record_btrace_commit_resume;
   ops->to_wait = record_btrace_wait;
   ops->to_stop = record_btrace_stop;
   ops->to_update_thread_list = record_btrace_update_thread_list;
diff --git a/gdb/record-full.c b/gdb/record-full.c
index e4dd55b..50f235d 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1002,6 +1002,15 @@ record_full_resume (struct target_ops *ops, ptid_t ptid, int step,
     target_async (1);
 }
 
+/* "to_commit_resume" method for process record target.  */
+
+static void
+record_full_commit_resume (struct target_ops *ops)
+{
+  if (!RECORD_FULL_IS_REPLAY)
+    ops->beneath->to_commit_resume (ops->beneath);
+}
+
 static int record_full_get_sig = 0;
 
 /* SIGINT signal handler, registered by "to_wait" method.  */
@@ -1172,6 +1181,7 @@ record_full_wait_1 (struct target_ops *ops,
 					    "target beneath\n");
 		      ops->beneath->to_resume (ops->beneath, ptid, step,
 					       GDB_SIGNAL_0);
+		      ops->beneath->to_commit_resume (ops->beneath);
 		      continue;
 		    }
 		}
@@ -1975,6 +1985,7 @@ init_record_full_ops (void)
   record_full_ops.to_close = record_full_close;
   record_full_ops.to_async = record_full_async;
   record_full_ops.to_resume = record_full_resume;
+  record_full_ops.to_commit_resume = record_full_commit_resume;
   record_full_ops.to_wait = record_full_wait;
   record_full_ops.to_disconnect = record_disconnect;
   record_full_ops.to_detach = record_detach;
diff --git a/gdb/remote.c b/gdb/remote.c
index 96f0ad5..517e36d 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -452,6 +452,24 @@ struct private_thread_info
   /* This is set to the data address of the access causing the target
      to stop for a watchpoint.  */
   CORE_ADDR watch_data_address;
+
+  /* Fields used by the vCont action coalescing implemented in
+     remote_resume / remote_commit_resume.  remote_resume stores each
+     thread's last resume request in these fields, so that a later
+     remote_commit_resume knows which is the proper action for this
+     thread to include in the vCont packet.  */
+
+  /* True if the last target_resume call for this thread was a step
+     request, false if a continue request.  */
+  int last_resume_step;
+
+  /* The signal specified in the last target_resume call for this
+     thread.  */
+  enum gdb_signal last_resume_sig;
+
+  /* Whether this thread was already vCont-resumed on the remote
+     side.  */
+  int vcont_resumed;
 };
 
 static void
@@ -1805,6 +1823,9 @@ remote_add_inferior (int fake_pid_p, int pid, int attached,
   return inf;
 }
 
+static struct private_thread_info *
+  get_private_info_thread (struct thread_info *info);
+
 /* Add thread PTID to GDB's thread list.  Tag it as executing/running
    according to RUNNING.  */
 
@@ -1812,6 +1833,7 @@ static void
 remote_add_thread (ptid_t ptid, int running, int executing)
 {
   struct remote_state *rs = get_remote_state ();
+  struct thread_info *thread;
 
   /* GDB historically didn't pull threads in the initial connection
      setup.  If the remote target doesn't even have a concept of
@@ -1820,10 +1842,11 @@ remote_add_thread (ptid_t ptid, int running, int executing)
      might be confusing to the user.  Be silent then, preserving the
      age old behavior.  */
   if (rs->starting_up)
-    add_thread_silent (ptid);
+    thread = add_thread_silent (ptid);
   else
-    add_thread (ptid);
+    thread = add_thread (ptid);
 
+  get_private_info_thread (thread)->vcont_resumed = executing;
   set_executing (ptid, executing);
   set_running (ptid, running);
 }
@@ -1918,25 +1941,40 @@ remote_notice_new_inferior (ptid_t currthread, int executing)
     }
 }
 
-/* Return the private thread data, creating it if necessary.  */
+/* Return THREAD's private thread data, creating it if necessary.  */
 
 static struct private_thread_info *
-demand_private_info (ptid_t ptid)
+get_private_info_thread (struct thread_info *thread)
 {
-  struct thread_info *info = find_thread_ptid (ptid);
+  gdb_assert (thread != NULL);
 
-  gdb_assert (info);
-
-  if (!info->priv)
+  if (thread->priv == NULL)
     {
-      info->priv = XNEW (struct private_thread_info);
-      info->private_dtor = free_private_thread_info;
-      info->priv->core = -1;
-      info->priv->extra = NULL;
-      info->priv->name = NULL;
+      struct private_thread_info *priv = XNEW (struct private_thread_info);
+
+      thread->private_dtor = free_private_thread_info;
+      thread->priv = priv;
+
+      priv->core = -1;
+      priv->extra = NULL;
+      priv->name = NULL;
+      priv->name = NULL;
+      priv->last_resume_step = 0;
+      priv->last_resume_sig = GDB_SIGNAL_0;
+      priv->vcont_resumed = 0;
     }
 
-  return info->priv;
+  return thread->priv;
+}
+
+/* Return PTID's private thread data, creating it if necessary.  */
+
+static struct private_thread_info *
+get_private_info_ptid (ptid_t ptid)
+{
+  struct thread_info *info = find_thread_ptid (ptid);
+
+  return get_private_info_thread (info);
 }
 
 /* Call this function as a result of
@@ -3280,7 +3318,7 @@ remote_update_thread_list (struct target_ops *ops)
 
 	      remote_notice_new_inferior (item->ptid, executing);
 
-	      info = demand_private_info (item->ptid);
+	      info = get_private_info_ptid (item->ptid);
 	      info->core = item->core;
 	      info->extra = item->extra;
 	      item->extra = NULL;
@@ -3913,6 +3951,7 @@ process_initial_stop_replies (int from_tty)
 
       set_executing (event_ptid, 0);
       set_running (event_ptid, 0);
+      thread->priv->vcont_resumed = 0;
     }
 
   /* "Notice" the new inferiors before anything related to
@@ -5701,6 +5740,26 @@ remote_resume (struct target_ops *ops,
 {
   struct remote_state *rs = get_remote_state ();
 
+  /* When connected in non-stop mode, the core resumes threads
+     individually.  Resuming remote threads directly in target_resume
+     would thus result in sending one packet per thread.  Instead, to
+     minimize roundtrip latency, here we just store the resume
+     request; the actual remote resumption will be done in
+     target_commit_resume / remote_commit_resume, where we'll be able
+     to do vCont action coalescing.  */
+  if (target_is_non_stop_p () && execution_direction != EXEC_REVERSE)
+    {
+      struct private_thread_info *remote_thr;
+
+      if (ptid_equal (minus_one_ptid, ptid) || ptid_is_pid (ptid))
+	remote_thr = get_private_info_ptid (inferior_ptid);
+      else
+	remote_thr = get_private_info_ptid (ptid);
+      remote_thr->last_resume_step = step;
+      remote_thr->last_resume_sig = siggnal;
+      return;
+    }
+
   /* In all-stop, we can't mark REMOTE_ASYNC_GET_PENDING_EVENTS_TOKEN
      (explained in remote-notif.c:handle_notification) so
      remote_notif_process is not called.  We need find a place where
@@ -5736,6 +5795,283 @@ remote_resume (struct target_ops *ops,
   if (!target_is_non_stop_p ())
     rs->waiting_for_stop_reply = 1;
 }
+
+static void check_pending_events_prevent_wildcard_vcont
+  (int *may_global_wildcard_vcont);
+static int is_pending_fork_parent_thread (struct thread_info *thread);
+
+/* Private per-inferior info for target remote processes.  */
+
+struct private_inferior
+{
+  /* Whether we can send a wildcard vCont for this process.  */
+  int may_wildcard_vcont;
+};
+
+/* Structure used to track the construction of a vCont packet in the
+   outgoing packet buffer.  This is used to send multiple vCont
+   packets if we have more actions than would fit a single packet.  */
+
+struct vcont_builder
+{
+  /* Pointer to the first action.  P points here if no action has been
+     appended yet.  */
+  char *first_action;
+
+  /* Where the next action will be appended.  */
+  char *p;
+
+  /* The end of the buffer.  Must never write past this.  */
+  char *endp;
+};
+
+/* Prepare the outgoing buffer for a new vCont packet.  */
+
+static void
+vcont_builder_restart (struct vcont_builder *builder)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  builder->p = rs->buf;
+  builder->endp = rs->buf + get_remote_packet_size ();
+  builder->p += xsnprintf (builder->p, builder->endp - builder->p, "vCont");
+  builder->first_action = builder->p;
+}
+
+/* If the vCont packet being built has any action, send it to the
+   remote end.  */
+
+static void
+vcont_builder_flush (struct vcont_builder *builder)
+{
+  struct remote_state *rs;
+
+  if (builder->p == builder->first_action)
+    return;
+
+  rs = get_remote_state ();
+  putpkt (rs->buf);
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (strcmp (rs->buf, "OK") != 0)
+    error (_("Unexpected vCont reply in non-stop mode: %s"), rs->buf);
+}
+
+/* The largest action is range-stepping, with its two addresses.  This
+   is more than sufficient.  If a new, bigger action is created, it'll
+   quickly trigger a failed assertion in append_resumption (and we'll
+   just bump this).  */
+#define MAX_ACTION_SIZE 200
+
+/* Append a new vCont action in the outgoing packet being built.  If
+   the action doesn't fit the packet along with previous actions, push
+   what we've got so far to the remote end and start over a new vCont
+   packet (with the new action).  */
+
+static void
+vcont_builder_push_action (struct vcont_builder *builder,
+			   ptid_t ptid, int step, enum gdb_signal siggnal)
+{
+  char buf[MAX_ACTION_SIZE + 1];
+  char *endp;
+  size_t rsize;
+
+  endp = append_resumption (buf, buf + sizeof (buf),
+			    ptid, step, siggnal);
+
+  /* Check whether this new action would fit in the vCont packet along
+     with previous actions.  If not, send what we've got so far and
+     start a new vCont packet.  */
+  rsize = endp - buf;
+  if (rsize > builder->endp - builder->p)
+    {
+      vcont_builder_flush (builder);
+      vcont_builder_restart (builder);
+
+      /* Should now fit.  */
+      gdb_assert (rsize <= builder->endp - builder->p);
+    }
+
+  memcpy (builder->p, buf, rsize);
+  builder->p += rsize;
+  *builder->p = '\0';
+}
+
+/* to_commit_resume implementation.  */
+
+static void
+remote_commit_resume (struct target_ops *ops)
+{
+  struct remote_state *rs = get_remote_state ();
+  struct inferior *inf;
+  struct thread_info *tp;
+  int any_process_wildcard;
+  int may_global_wildcard_vcont;
+  struct vcont_builder vcont_builder;
+
+  /* If connected in all-stop mode, we'd send the remote resume
+     request directly from remote_resume.  Likewise if
+     reverse-debugging, as there are no defined vCont actions for
+     reverse execution.  */
+  if (!target_is_non_stop_p () || execution_direction == EXEC_REVERSE)
+    return;
+
+  /* Try to send wildcard actions ("vCont;c" or "vCont;c:pPID.-1")
+     instead of resuming all threads of each process individually.
+     However, if any thread of a process must remain halted, we can't
+     send wildcard resumes and must send one action per thread.
+
+     Care must be taken to not resume threads/processes the server
+     side already told us are stopped, but the core doesn't know about
+     yet, because the events are still in the vStopped notification
+     queue.  For example:
+
+       #1 => vCont s:p1.1;c
+       #2 <= OK
+       #3 <= %Stopped T05 p1.1
+       #4 => vStopped
+       #5 <= T05 p1.2
+       #6 => vStopped
+       #7 <= OK
+       #8 (infrun handles the stop for p1.1 and continues stepping)
+       #9 => vCont s:p1.1;c
+
+     The last vCont above would resume thread p1.2 by mistake, because
+     the server has no idea that the event for p1.2 had not been
+     handled yet.
+
+     The server side must similarly 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 p1.2
+       #4 => vCont s:p1.1;c
+       #5 <= OK
+
+     Above, the server must not resume thread p1.2.  GDB can't know
+     that p1.2 stopped until it acks the %Stopped notification, and
+     since from GDB's perspective all threads should be running, it
+     sends a "c" action.
+
+     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.  */
+
+  /* Start by assuming a global wildcard (vCont;c) is possible.  */
+  may_global_wildcard_vcont = 1;
+
+  /* And assume every process is individually wildcard-able too.  */
+  ALL_NON_EXITED_INFERIORS (inf)
+    {
+      if (inf->priv == NULL)
+	inf->priv = XNEW (struct private_inferior);
+      inf->priv->may_wildcard_vcont = 1;
+    }
+
+  /* Check for any pending events (not reported or processed yet) and
+     disable process and global wildcard resumes appropriately.  */
+  check_pending_events_prevent_wildcard_vcont (&may_global_wildcard_vcont);
+
+  ALL_NON_EXITED_THREADS (tp)
+    {
+      /* If a thread of a process is not meant to be resumed, then we
+	 can't wildcard that process.  */
+      if (!tp->executing)
+	{
+	  tp->inf->priv->may_wildcard_vcont = 0;
+
+	  /* And if we can't wildcard a process, we can't wildcard
+	     everything either.  */
+	  may_global_wildcard_vcont = 0;
+	  continue;
+	}
+
+      /* If a thread is the parent of an unfollowed fork, then we
+	 can't do a global wildcard, as that would resume the fork
+	 child.  */
+      if (is_pending_fork_parent_thread (tp))
+	may_global_wildcard_vcont = 0;
+    }
+
+  /* Now let's build the vCont packet(s).  Actions must be appended
+     from narrower to wider scopes (thread -> process -> global).  If
+     we end up with too many actions for a single packet vcont_builder
+     flushes the current vCont packet to the remote side and starts a
+     new one.  */
+  vcont_builder_restart (&vcont_builder);
+
+  /* Threads first.  */
+  ALL_NON_EXITED_THREADS (tp)
+    {
+      struct private_thread_info *remote_thr = tp->priv;
+
+      if (!tp->executing || remote_thr->vcont_resumed)
+	continue;
+
+      gdb_assert (!thread_is_in_step_over_chain (tp));
+
+      if (!remote_thr->last_resume_step
+	  && remote_thr->last_resume_sig == GDB_SIGNAL_0
+	  && tp->inf->priv->may_wildcard_vcont)
+	{
+	  /* We'll send a wildcard resume instead.  */
+	  remote_thr->vcont_resumed = 1;
+	  continue;
+	}
+
+      vcont_builder_push_action (&vcont_builder, tp->ptid,
+				 remote_thr->last_resume_step,
+				 remote_thr->last_resume_sig);
+      remote_thr->vcont_resumed = 1;
+    }
+
+  /* Now check whether we can send any process-wide wildcard.  This is
+     to avoid sending a global wildcard in the case nothing is
+     supposed to be resumed.  */
+  any_process_wildcard = 0;
+
+  ALL_NON_EXITED_INFERIORS (inf)
+    {
+      if (inf->priv->may_wildcard_vcont)
+	{
+	  any_process_wildcard = 1;
+	  break;
+	}
+    }
+
+  if (any_process_wildcard)
+    {
+      /* If all processes are wildcard-able, then send a single "c"
+	 action, otherwise, send an "all (-1) threads of process"
+	 continue action for each running process, if any.  */
+      if (may_global_wildcard_vcont)
+	{
+	  vcont_builder_push_action (&vcont_builder, minus_one_ptid,
+				     0, GDB_SIGNAL_0);
+	}
+      else
+	{
+	  ALL_NON_EXITED_INFERIORS (inf)
+	    {
+	      if (inf->priv->may_wildcard_vcont)
+		{
+		  vcont_builder_push_action (&vcont_builder,
+					     pid_to_ptid (inf->pid),
+					     0, GDB_SIGNAL_0);
+		}
+	    }
+	}
+    }
+
+  vcont_builder_flush (&vcont_builder);
+}
+
 

 
 /* Non-stop version of target_stop.  Uses `vCont;t' to stop a remote
@@ -6102,7 +6438,7 @@ struct queue_iter_param
   struct stop_reply *output;
 };
 
-/* Determine if THREAD is a pending fork parent thread.  ARG contains
+/* Determine if THREAD_PTID is a pending fork parent thread.  ARG contains
    the pid of the process that owns the threads we want to check, or
    -1 if we want to check all threads.  */
 
@@ -6120,6 +6456,29 @@ is_pending_fork_parent (struct target_waitstatus *ws, int event_pid,
   return 0;
 }
 
+/* Return the thread's pending status used to determine whether the
+   thread is a fork parent stopped at a fork event.  */
+
+static struct target_waitstatus *
+thread_pending_fork_status (struct thread_info *thread)
+{
+  if (thread->suspend.waitstatus_pending_p)
+    return &thread->suspend.waitstatus;
+  else
+    return &thread->pending_follow;
+}
+
+/* Determine if THREAD is a pending fork parent thread.  */
+
+static int
+is_pending_fork_parent_thread (struct thread_info *thread)
+{
+  struct target_waitstatus *ws = thread_pending_fork_status (thread);
+  int pid = -1;
+
+  return is_pending_fork_parent (ws, pid, thread->ptid);
+}
+
 /* Check whether EVENT is a fork event, and if it is, remove the
    fork child from the context list passed in DATA.  */
 
@@ -6159,12 +6518,7 @@ remove_new_fork_children (struct threads_listing_context *context)
      fork child threads from the CONTEXT list.  */
   ALL_NON_EXITED_THREADS (thread)
     {
-      struct target_waitstatus *ws;
-
-      if (thread->suspend.waitstatus_pending_p)
-	ws = &thread->suspend.waitstatus;
-      else
-	ws = &thread->pending_follow;
+      struct target_waitstatus *ws = thread_pending_fork_status (thread);
 
       if (is_pending_fork_parent (ws, pid, thread->ptid))
 	{
@@ -6182,6 +6536,56 @@ remove_new_fork_children (struct threads_listing_context *context)
 		 remove_child_of_pending_fork, &param);
 }
 
+/* Check whether EVENT would prevent a global or process wildcard
+   vCont action.  */
+
+static int
+check_pending_event_prevents_wildcard_vcont_callback
+  (QUEUE (stop_reply_p) *q,
+   QUEUE_ITER (stop_reply_p) *iter,
+   stop_reply_p event,
+   void *data)
+{
+  struct inferior *inf;
+  int *may_global_wildcard_vcont = (int *) data;
+
+  if (event->ws.kind == TARGET_WAITKIND_NO_RESUMED
+      || event->ws.kind == TARGET_WAITKIND_NO_HISTORY)
+    return 1;
+
+  if (event->ws.kind == TARGET_WAITKIND_FORKED
+      || event->ws.kind == TARGET_WAITKIND_VFORKED)
+    *may_global_wildcard_vcont = 0;
+
+  inf = find_inferior_ptid (event->ptid);
+
+  /* This may be the first time we heard about this process.
+     Regardless, we must not do a global wildcard resume, otherwise
+     we'd resume this process too.  */
+  *may_global_wildcard_vcont = 0;
+  if (inf != NULL)
+    inf->priv->may_wildcard_vcont = 0;
+
+  return 1;
+}
+
+/* Check whether any event pending in the vStopped queue would prevent
+   a global or process wildcard vCont action.  Clear
+   *may_global_wildcard if we can't do a global wildcard (vCont;c),
+   and clear the event inferior's may_wildcard_vcont flag if we can't
+   do a process-wide wildcard resume (vCont;c:pPID.-1).  */
+
+static void
+check_pending_events_prevent_wildcard_vcont (int *may_global_wildcard)
+{
+  struct notif_client *notif = &notif_client_stop;
+
+  remote_notif_get_pending_events (notif);
+  QUEUE_iterate (stop_reply_p, stop_reply_queue,
+		 check_pending_event_prevents_wildcard_vcont_callback,
+		 may_global_wildcard);
+}
+
 /* Remove stop replies in the queue if its pid is equal to the given
    inferior's pid.  */
 
@@ -6812,10 +7216,11 @@ process_stop_reply (struct stop_reply *stop_reply,
 	}
 
       remote_notice_new_inferior (ptid, 0);
-      remote_thr = demand_private_info (ptid);
+      remote_thr = get_private_info_ptid (ptid);
       remote_thr->core = stop_reply->core;
       remote_thr->stop_reason = stop_reply->stop_reason;
       remote_thr->watch_data_address = stop_reply->watch_data_address;
+      remote_thr->vcont_resumed = 0;
     }
 
   stop_reply_xfree (stop_reply);
@@ -13114,6 +13519,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_detach = remote_detach;
   remote_ops.to_disconnect = remote_disconnect;
   remote_ops.to_resume = remote_resume;
+  remote_ops.to_commit_resume = remote_commit_resume;
   remote_ops.to_wait = remote_wait;
   remote_ops.to_fetch_registers = remote_fetch_registers;
   remote_ops.to_store_registers = remote_store_registers;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 57e7939..73e45dd 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -109,6 +109,28 @@ debug_resume (struct target_ops *self, ptid_t arg1, int arg2, enum gdb_signal ar
   fputs_unfiltered (")\n", gdb_stdlog);
 }
 
+static void
+delegate_commit_resume (struct target_ops *self)
+{
+  self = self->beneath;
+  self->to_commit_resume (self);
+}
+
+static void
+tdefault_commit_resume (struct target_ops *self)
+{
+}
+
+static void
+debug_commit_resume (struct target_ops *self)
+{
+  fprintf_unfiltered (gdb_stdlog, "-> %s->to_commit_resume (...)\n", debug_target.to_shortname);
+  debug_target.to_commit_resume (&debug_target);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->to_commit_resume (", debug_target.to_shortname);
+  target_debug_print_struct_target_ops_p (&debug_target);
+  fputs_unfiltered (")\n", gdb_stdlog);
+}
+
 static ptid_t
 delegate_wait (struct target_ops *self, ptid_t arg1, struct target_waitstatus *arg2, int arg3)
 {
@@ -4108,6 +4130,8 @@ install_delegators (struct target_ops *ops)
     ops->to_disconnect = delegate_disconnect;
   if (ops->to_resume == NULL)
     ops->to_resume = delegate_resume;
+  if (ops->to_commit_resume == NULL)
+    ops->to_commit_resume = delegate_commit_resume;
   if (ops->to_wait == NULL)
     ops->to_wait = delegate_wait;
   if (ops->to_fetch_registers == NULL)
@@ -4413,6 +4437,7 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_detach = tdefault_detach;
   ops->to_disconnect = tdefault_disconnect;
   ops->to_resume = tdefault_resume;
+  ops->to_commit_resume = tdefault_commit_resume;
   ops->to_wait = default_target_wait;
   ops->to_fetch_registers = tdefault_fetch_registers;
   ops->to_store_registers = tdefault_store_registers;
@@ -4570,6 +4595,7 @@ init_debug_target (struct target_ops *ops)
   ops->to_detach = debug_detach;
   ops->to_disconnect = debug_disconnect;
   ops->to_resume = debug_resume;
+  ops->to_commit_resume = debug_commit_resume;
   ops->to_wait = debug_wait;
   ops->to_fetch_registers = debug_fetch_registers;
   ops->to_store_registers = debug_store_registers;
diff --git a/gdb/target.c b/gdb/target.c
index cb89e75..246d292 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2329,6 +2329,34 @@ target_resume (ptid_t ptid, int step, enum gdb_signal signal)
   clear_inline_frame_state (ptid);
 }
 
+/* If true, target_commit_resume is a nop.  */
+static int defer_target_commit_resume;
+
+/* See target.h.  */
+
+void
+target_commit_resume (void)
+{
+  struct target_ops *t;
+
+  if (defer_target_commit_resume)
+    return;
+
+  current_target.to_commit_resume (&current_target);
+}
+
+/* See target.h.  */
+
+struct cleanup *
+make_cleanup_defer_target_commit_resume (void)
+{
+  struct cleanup *old_chain;
+
+  old_chain = make_cleanup_restore_integer (&defer_target_commit_resume);
+  defer_target_commit_resume = 1;
+  return old_chain;
+}
+
 void
 target_pass_signals (int numsigs, unsigned char *pass_signals)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 176f332..a54b3db 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -461,6 +461,8 @@ struct target_ops
 		       int TARGET_DEBUG_PRINTER (target_debug_print_step),
 		       enum gdb_signal)
       TARGET_DEFAULT_NORETURN (noprocess ());
+    void (*to_commit_resume) (struct target_ops *)
+      TARGET_DEFAULT_IGNORE ();
     ptid_t (*to_wait) (struct target_ops *,
 		       ptid_t, struct target_waitstatus *,
 		       int TARGET_DEBUG_PRINTER (target_debug_print_options))
@@ -1328,19 +1330,42 @@ extern void target_detach (const char *, int);
 
 extern void target_disconnect (const char *, int);
 
-/* Resume execution of the target process PTID (or a group of
-   threads).  STEP says whether to hardware single-step or to run free;
-   SIGGNAL is the signal to be given to the target, or GDB_SIGNAL_0 for no
-   signal.  The caller may not pass GDB_SIGNAL_DEFAULT.  A specific
-   PTID means `step/resume only this process id'.  A wildcard PTID
-   (all threads, or all threads of process) means `step/resume
-   INFERIOR_PTID, and let other threads (for which the wildcard PTID
-   matches) resume with their 'thread->suspend.stop_signal' signal
-   (usually GDB_SIGNAL_0) if it is in "pass" state, or with no signal
-   if in "no pass" state.  */
-
+/* Resume execution (or prepare for execution) of a target thread,
+   process or all processes.  STEP says whether to hardware
+   single-step or to run free; SIGGNAL is the signal to be given to
+   the target, or GDB_SIGNAL_0 for no signal.  The caller may not pass
+   GDB_SIGNAL_DEFAULT.  A specific PTID means `step/resume only this
+   process id'.  A wildcard PTID (all threads, or all threads of
+   process) means `step/resume INFERIOR_PTID, and let other threads
+   (for which the wildcard PTID matches) resume with their
+   'thread->suspend.stop_signal' signal (usually GDB_SIGNAL_0) if it
+   is in "pass" state, or with no signal if in "no pass" state.
+
+   In order to efficiently handle batches of resumption requests,
+   targets may implement this method such that it records the
+   resumption request, but defers the actual resumption to the
+   target_commit_resume method implementation.  See
+   target_commit_resume below.  */
 extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
 
+/* Commit a series of resumption requests previously prepared with
+   target_resume calls.
+
+   GDB always calls target_commit_resume after calling target_resume
+   one or more times.  A target may thus use this method in
+   coordination with the target_resume method to batch target-side
+   resumption requests.  In that case, the target doesn't actually
+   resume in its target_resume implementation.  Instead, it prepares
+   the resumption in target_resume, and defers the actual resumption
+   to target_commit_resume.  E.g., the remote target uses this to
+   coalesce multiple resumption requests in a single vCont packet.  */
+extern void target_commit_resume ();
+
+/* Setup to defer target_commit_resume calls, and return a cleanup
+   that reactivates target_commit_resume, if it was previously
+   active.  */
+struct cleanup *make_cleanup_defer_target_commit_resume ();
+
 /* For target_read_memory see target/target.h.  */
 
 /* The default target_ops::to_wait implementation.  */
-- 
2.5.5




More information about the Gdb-patches mailing list