[PATCH v3 3/5] gdb: move commit_resume to process_stratum_target

[Simon Marchi <simon.marchi@polymtl.ca>]

From: Simon Marchi <simon.marchi@efficios.com>

The following patch will change the commit_resume target method to
something stateful.  Because it would be difficult to track a state
replicated in the various targets of a target stack, and since for the
foreseeable future, only process stratum targets are going to use this
concept, this patch makes the commit resume concept specific to process
stratum targets.

So, move the method to process_stratum_target, and move helper functions
to process-stratum-target.h.

gdb/ChangeLog:

	* target.h (struct target_ops) <commit_resume>: New.
	(target_commit_resume): Remove.
	(make_scoped_defer_target_commit_resume): Remove.
	* target.c (defer_target_commit_resume): Remove.
	(target_commit_resume): Remove.
	(make_scoped_defer_target_commit_resume): Remove.
	* process-stratum-target.h (class process_stratum_target)
	<commit_resume>: New.
	(maybe_commit_resume_all_process_targets): New.
	(make_scoped_defer_process_target_commit_resume): New.
	* process-stratum-target.c (defer_process_target_commit_resume):
	New.
	(maybe_commit_resume_process_target): New.
	(make_scoped_defer_process_target_commit_resume): New.
	* infrun.c (do_target_resume): Adjust.
	(commit_resume_all_targets): Rename into...
	(maybe_commit_resume_all_process_targets): ... this, adjust.
	(proceed): Adjust.
	* record-full.c (record_full_wait_1): Adjust.
	* target-delegates.c: Re-generate.

Change-Id: Ifc957817ac5b2303e22760ce3d14740b9598f02c
---
 gdb/infrun.c                 | 28 +++++++++-------------------
 gdb/process-stratum-target.c | 23 +++++++++++++++++++++++
 gdb/process-stratum-target.h | 29 +++++++++++++++++++++++++++++
 gdb/record-full.c            |  8 ++++----
 gdb/target-delegates.c       | 22 ----------------------
 gdb/target.c                 | 22 ----------------------
 gdb/target.h                 | 20 --------------------
 7 files changed, 65 insertions(+), 87 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 45bedf896419..1a27af51b7e9 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2172,7 +2172,7 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig)
 
   target_resume (resume_ptid, step, sig);
 
-  target_commit_resume ();
+  maybe_commit_resume_process_target (tp->inf->process_target ());
 
   if (target_can_async_p ())
     target_async (1);
@@ -2760,28 +2760,17 @@ schedlock_applies (struct thread_info *tp)
 					    execution_direction)));
 }
 
-/* Calls target_commit_resume on all targets.  */
+/* Calls maybe_commit_resume_process_target on all process targets.  */
 
 static void
-commit_resume_all_targets ()
+maybe_commit_resume_all_process_targets ()
 {
   scoped_restore_current_thread restore_thread;
 
-  /* Map between process_target and a representative inferior.  This
-     is to avoid committing a resume in the same target more than
-     once.  Resumptions must be idempotent, so this is an
-     optimization.  */
-  std::unordered_map<process_stratum_target *, inferior *> conn_inf;
-
-  for (inferior *inf : all_non_exited_inferiors ())
-    if (inf->has_execution ())
-      conn_inf[inf->process_target ()] = inf;
-
-  for (const auto &ci : conn_inf)
+  for (process_stratum_target *target : all_non_exited_process_targets ())
     {
-      inferior *inf = ci.second;
-      switch_to_inferior_no_thread (inf);
-      target_commit_resume ();
+      switch_to_target_no_thread (target);
+      maybe_commit_resume_process_target (target);
     }
 }
 
@@ -3005,7 +2994,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
   cur_thr->prev_pc = regcache_read_pc_protected (regcache);
 
   {
-    scoped_restore save_defer_tc = make_scoped_defer_target_commit_resume ();
+    scoped_restore save_defer_tc
+      = make_scoped_defer_process_target_commit_resume ();
 
     started = start_step_over ();
 
@@ -3075,7 +3065,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
       }
   }
 
-  commit_resume_all_targets ();
+  maybe_commit_resume_all_process_targets ();
 
   finish_state.release ();
 
diff --git a/gdb/process-stratum-target.c b/gdb/process-stratum-target.c
index 719167803fff..1436a550ac04 100644
--- a/gdb/process-stratum-target.c
+++ b/gdb/process-stratum-target.c
@@ -108,3 +108,26 @@ switch_to_target_no_thread (process_stratum_target *target)
       break;
     }
 }
+
+/* If true, `maybe_commit_resume_process_target` is a no-op.  */
+
+static bool defer_process_target_commit_resume;
+
+/* See target.h.  */
+
+void
+maybe_commit_resume_process_target (process_stratum_target *proc_target)
+{
+  if (defer_process_target_commit_resume)
+    return;
+
+  proc_target->commit_resume ();
+}
+
+/* See process-stratum-target.h.  */
+
+scoped_restore_tmpl<bool>
+make_scoped_defer_process_target_commit_resume ()
+{
+  return make_scoped_restore (&defer_process_target_commit_resume, true);
+}
diff --git a/gdb/process-stratum-target.h b/gdb/process-stratum-target.h
index b513c26ffc2a..c8060c46be93 100644
--- a/gdb/process-stratum-target.h
+++ b/gdb/process-stratum-target.h
@@ -63,6 +63,20 @@ class process_stratum_target : public target_ops
   bool has_registers () override;
   bool has_execution (inferior *inf) override;
 
+  /* Commit a series of resumption requests previously prepared with
+     resume calls.
+
+     GDB always calls `commit_resume` on the process stratum target after
+     calling `resume` on a target stack.  A process stratum target may thus use
+     this method in coordination with its `resume` method to batch resumption
+     requests.  In that case, the target doesn't actually resume in its
+     `resume` implementation.  Instead, it takes note of resumption intent in
+     `resume`, and defers the actual resumption `commit_resume`.
+
+     E.g., the remote target uses this to coalesce multiple resumption requests
+     in a single vCont packet.  */
+  virtual void commit_resume () {}
+
   /* True if any thread is, or may be executing.  We need to track
      this separately because until we fully sync the thread list, we
      won't know whether the target is fully stopped, even if we see
@@ -92,4 +106,19 @@ extern std::set<process_stratum_target *> all_non_exited_process_targets ();
 
 extern void switch_to_target_no_thread (process_stratum_target *target);
 
+/* Commit a series of resumption requests previously prepared with
+   target_resume calls.
+
+   This function is a no-op if commit resumes are deferred (see
+   `make_scoped_defer_process_target_commit_resume`).  */
+
+extern void maybe_commit_resume_process_target
+  (process_stratum_target *target);
+
+/* Setup to defer `commit_resume` calls, and re-set to the previous status on
+   destruction.  */
+
+extern scoped_restore_tmpl<bool>
+  make_scoped_defer_process_target_commit_resume ();
+
 #endif /* !defined (PROCESS_STRATUM_TARGET_H) */
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 22eaaa4bb1bc..56ab29479874 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1242,11 +1242,11 @@ record_full_wait_1 (struct target_ops *ops,
 			   break;
   			}
 
+		      process_stratum_target *proc_target
+			= current_inferior ()->process_target ();
+
 		      if (gdbarch_software_single_step_p (gdbarch))
 			{
-			  process_stratum_target *proc_target
-			    = current_inferior ()->process_target ();
-
 			  /* Try to insert the software single step breakpoint.
 			     If insert success, set step to 0.  */
 			  set_executing (proc_target, inferior_ptid, false);
@@ -1263,7 +1263,7 @@ record_full_wait_1 (struct target_ops *ops,
 					    "issuing one more step in the "
 					    "target beneath\n");
 		      ops->beneath ()->resume (ptid, step, GDB_SIGNAL_0);
-		      ops->beneath ()->commit_resume ();
+		      proc_target->commit_resume ();
 		      continue;
 		    }
 		}
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 437b19b8581c..8b933fdf82eb 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -14,7 +14,6 @@ struct dummy_target : public target_ops
   void detach (inferior *arg0, int arg1) override;
   void disconnect (const char *arg0, int arg1) override;
   void resume (ptid_t arg0, int arg1, enum gdb_signal arg2) override;
-  void commit_resume () override;
   ptid_t wait (ptid_t arg0, struct target_waitstatus *arg1, target_wait_flags arg2) override;
   void fetch_registers (struct regcache *arg0, int arg1) override;
   void store_registers (struct regcache *arg0, int arg1) override;
@@ -185,7 +184,6 @@ struct debug_target : public target_ops
   void detach (inferior *arg0, int arg1) override;
   void disconnect (const char *arg0, int arg1) override;
   void resume (ptid_t arg0, int arg1, enum gdb_signal arg2) override;
-  void commit_resume () override;
   ptid_t wait (ptid_t arg0, struct target_waitstatus *arg1, target_wait_flags arg2) override;
   void fetch_registers (struct regcache *arg0, int arg1) override;
   void store_registers (struct regcache *arg0, int arg1) override;
@@ -440,26 +438,6 @@ debug_target::resume (ptid_t arg0, int arg1, enum gdb_signal arg2)
   fputs_unfiltered (")\n", gdb_stdlog);
 }
 
-void
-target_ops::commit_resume ()
-{
-  this->beneath ()->commit_resume ();
-}
-
-void
-dummy_target::commit_resume ()
-{
-}
-
-void
-debug_target::commit_resume ()
-{
-  fprintf_unfiltered (gdb_stdlog, "-> %s->commit_resume (...)\n", this->beneath ()->shortname ());
-  this->beneath ()->commit_resume ();
-  fprintf_unfiltered (gdb_stdlog, "<- %s->commit_resume (", this->beneath ()->shortname ());
-  fputs_unfiltered (")\n", gdb_stdlog);
-}
-
 ptid_t
 target_ops::wait (ptid_t arg0, struct target_waitstatus *arg1, target_wait_flags arg2)
 {
diff --git a/gdb/target.c b/gdb/target.c
index 3a03a0ad530e..3a5270e5a416 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2062,28 +2062,6 @@ target_resume (ptid_t ptid, int step, enum gdb_signal signal)
   clear_inline_frame_state (curr_target, ptid);
 }
 
-/* If true, target_commit_resume is a nop.  */
-static int defer_target_commit_resume;
-
-/* See target.h.  */
-
-void
-target_commit_resume (void)
-{
-  if (defer_target_commit_resume)
-    return;
-
-  current_top_target ()->commit_resume ();
-}
-
-/* See target.h.  */
-
-scoped_restore_tmpl<int>
-make_scoped_defer_target_commit_resume ()
-{
-  return make_scoped_restore (&defer_target_commit_resume, 1);
-}
-
 void
 target_pass_signals (gdb::array_view<const unsigned char> pass_signals)
 {
diff --git a/gdb/target.h b/gdb/target.h
index e1a1d7a9226b..a252c29eafb4 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -478,8 +478,6 @@ struct target_ops
 			 int TARGET_DEBUG_PRINTER (target_debug_print_step),
 			 enum gdb_signal)
       TARGET_DEFAULT_NORETURN (noprocess ());
-    virtual void commit_resume ()
-      TARGET_DEFAULT_IGNORE ();
     /* See target_wait's description.  Note that implementations of
        this method must not assume that inferior_ptid on entry is
        pointing at the thread or inferior that ends up reporting an
@@ -1431,24 +1429,6 @@ extern void target_disconnect (const char *, int);
    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 reactivate
-   target_commit_resume on destruction, if it was previously
-   active.  */
-extern scoped_restore_tmpl<int> make_scoped_defer_target_commit_resume ();
-
 /* For target_read_memory see target/target.h.  */
 
 /* The default target_ops::to_wait implementation.  */
-- 
2.29.2