[RFC,3/7] gdb/remote: add vReplayDiversion packet

Message ID x4a6tj_4rjn5sau70r8.8_z5&xobl_&m3mmnxj5wc/s6fpj/-kkc@mail.bob131.so
State New
Headers show
Series
  • gdb: replay diversion support
Related show

Commit Message

Simon Marchi via Gdb-patches June 1, 2021, 7:10 p.m.
This commit implements the replay diversion target interface for the
remote target and adds a remote protocol extension enabling the
control of remote deterministic replay targets.

gdb/ChangeLog:

2021-06-01  George Barrett  <bob@bob131.so>

	* remote.c (remote_state::in_replay_diversion): Define
	variable.
	(remote_state::needs_replay_diversion)
	(remote_state::in_replay_diversion)
	(remote_state::start_replay_diversion)
	(remote_state::end_replay_diversion): Declare overrides and
	add implementation.
	(PACKET_vReplayDiversion): Add declaration.
	(remote_protocol_features): Add PACKET_vReplayDiversion.
	(remote_target::remote_query_supported): Announce support for
	vReplayDiversion packet.
	(remote_target::process_stop_reply): If a remote signalled an
	exit during a replay diversion, change the wait status kind to
	TARGET_WAITKIND_REPLAY_DIVERSION_ENDED and prepare for a stop.
	(_initialize_remote): Add packet configuration command for
	vReplayDiversion.

gdb/doc/ChangeLog:

2021-06-01  George Barrett  <bob@bob131.so>

	* gdb.texinfo (Packets): Document vReplayDiversion packet.
	(General Query Packets): Document vReplayDiversion GDB and
	stub feature for qSupported.
---
 gdb/doc/gdb.texinfo |  40 +++++++++++++++
 gdb/remote.c        | 116 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 153 insertions(+), 3 deletions(-)

-- 
2.31.1

Comments

Simon Marchi via Gdb-patches June 1, 2021, 7:18 p.m. | #1
> Date: Wed, 02 Jun 2021 05:10:39 +1000

> From: George Barrett via Gdb-patches <gdb-patches@sourceware.org>

> Cc: George Barrett <bob@bob131.so>

> 

> This commit implements the replay diversion target interface for the

> remote target and adds a remote protocol extension enabling the

> control of remote deterministic replay targets.

> 

> gdb/ChangeLog:

> 

> 2021-06-01  George Barrett  <bob@bob131.so>

> 

> 	* remote.c (remote_state::in_replay_diversion): Define

> 	variable.

> 	(remote_state::needs_replay_diversion)

> 	(remote_state::in_replay_diversion)

> 	(remote_state::start_replay_diversion)

> 	(remote_state::end_replay_diversion): Declare overrides and

> 	add implementation.

> 	(PACKET_vReplayDiversion): Add declaration.

> 	(remote_protocol_features): Add PACKET_vReplayDiversion.

> 	(remote_target::remote_query_supported): Announce support for

> 	vReplayDiversion packet.

> 	(remote_target::process_stop_reply): If a remote signalled an

> 	exit during a replay diversion, change the wait status kind to

> 	TARGET_WAITKIND_REPLAY_DIVERSION_ENDED and prepare for a stop.

> 	(_initialize_remote): Add packet configuration command for

> 	vReplayDiversion.

> 

> gdb/doc/ChangeLog:

> 

> 2021-06-01  George Barrett  <bob@bob131.so>

> 

> 	* gdb.texinfo (Packets): Document vReplayDiversion packet.

> 	(General Query Packets): Document vReplayDiversion GDB and

> 	stub feature for qSupported.


The gdb.texinfo part is OK, with one comment:

> +Diverted inferiors may exit unexpectedly (for example, due to an

> +inferior call to @code{exit()}).  The remote should send an exit stop

                    ^^^^^^^^^^^^^
Please use @code{exit}, without the parentheses.  The way you wrote
it, it looks like a call to 'exit' with no arguments, which is not
what you mean.

Thanks.
Simon Marchi via Gdb-patches June 1, 2021, 7:27 p.m. | #2
On Tue, Jun 01, 2021 at 10:18:43PM +0300, Eli Zaretskii wrote:
> Please use @code{exit}, without the parentheses.  The way you wrote

> it, it looks like a call to 'exit' with no arguments, which is not

> what you mean.


Ack.

Thanks for the fast review!

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 90d827a50e7..e98530069f9 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -40412,6 +40412,31 @@  If this packet is handled differently to other unknown @samp{v}
 packets then it is possible that @value{GDBN} may run into problems in
 other areas, specifically around use of @samp{vFile:setfs:}.
 
+@item vReplayDiversion:@var{want-diversion}
+@cindex @samp{vReplayDiversion} packet
+Remotes designed to deterministically replay a recorded program trace
+may require a debugger to create temporary diversions before performing
+arbitrary writes to memory or registers of the inferior.  Sending the
+packet with @samp{1} for @var{want-diversion} sets up such a diversion;
+with @samp{0}, destroys the diversion and reverts the inferior state to
+that prior to the diversion setup.
+
+Diverted inferiors may exit unexpectedly (for example, due to an
+inferior call to @code{exit()}).  The remote should send an exit stop
+packet and end the diversion as though it had received
+@samp{vReplayDiversion:0}.
+
+This packet should only be used if both @value{GDBN} and the remote stub
+announce support for it.  @xref{qSupported}.
+
+Reply:
+@table @samp
+@item E.@var{error-reason}
+for an error
+@item OK
+for success
+@end table
+
 @item vRun;@var{filename}@r{[};@var{argument}@r{]}@dots{}
 @cindex @samp{vRun} packet
 Run the program @var{filename}, passing it each @var{argument} on its
@@ -41745,6 +41770,10 @@  including @samp{exec-events+} in its @samp{qSupported} reply.
 @item vContSupported
 This feature indicates whether @value{GDBN} wants to know the
 supported actions in the reply to @samp{vCont?} packet.
+
+@item vReplayDiversion
+This feature indicates whether @value{GDBN} supports replay targets
+needing temporary diversions of execution prior to inferior calls.
 @end table
 
 Stubs should ignore any unknown values for
@@ -42023,6 +42052,11 @@  These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{vReplayDiversion}
+@tab No
+@tab @samp{-}
+@tab No
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -42247,6 +42281,12 @@  For AArch64 GNU/Linux systems, this feature also requires access to the
 @file{/proc/@var{pid}/smaps} file so memory mapping page flags can be inspected.
 This is done via the @samp{vFile} requests.
 
+@item vReplayDiversion
+The remote stub is deterministically replaying a program recording and
+supports temporary diversions from the replay in order to execute
+arbitrary code.  Such targets must receive @code{vReplayDiversion:1}
+before any memory writes or register changes may occur.
+
 @end table
 
 @item qSymbol::
diff --git a/gdb/remote.c b/gdb/remote.c
index 9b465d77343..f38772da525 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -390,6 +390,10 @@  class remote_state
      this can go away.  */
   int wait_forever_enabled_p = 1;
 
+  /* Track whether the remote is currently executing in a replay
+     diversion.  */
+  bool in_replay_diversion = false;
+
 private:
   /* Mapping of remote protocol data for each gdbarch.  Usually there
      is only one entry here, though we may see more with stubs that
@@ -681,6 +685,12 @@  class remote_target : public process_stratum_target
 				 enum btrace_read_type type) override;
 
   const struct btrace_config *btrace_conf (const struct btrace_target_info *) override;
+
+  bool needs_replay_diversion () override;
+  bool in_replay_diversion () override;
+  void start_replay_diversion () override;
+  void end_replay_diversion () override;
+
   bool augmented_libraries_svr4_read () override;
   void follow_fork (bool, bool) override;
   void follow_exec (inferior *, ptid_t, const char *) override;
@@ -2184,6 +2194,9 @@  enum {
      packets and the tag violation stop replies.  */
   PACKET_memory_tagging_feature,
 
+  /* Support for replay diversions.  */
+  PACKET_vReplayDiversion,
+
   PACKET_MAX
 };
 
@@ -5333,6 +5346,8 @@  static const struct protocol_feature remote_protocol_features[] = {
   { "no-resumed", PACKET_DISABLE, remote_supported_packet, PACKET_no_resumed },
   { "memory-tagging", PACKET_DISABLE, remote_supported_packet,
     PACKET_memory_tagging_feature },
+  { "vReplayDiversion", PACKET_DISABLE, remote_supported_packet,
+    PACKET_vReplayDiversion },
 };
 
 static char *remote_support_xml;
@@ -5431,6 +5446,10 @@  remote_target::remote_query_supported ()
 	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "memory-tagging+");
 
+      if (packet_set_cmd_state (PACKET_vReplayDiversion)
+	  != AUTO_BOOLEAN_FALSE)
+	remote_query_supported_append (&q, "vReplayDiversion+");
+
       /* Keep this one last to work around a gdbserver <= 7.10 bug in
 	 the qSupported:xmlRegisters=i386 handling.  */
       if (remote_support_xml != NULL
@@ -8017,9 +8036,19 @@  remote_target::process_stop_reply (struct stop_reply *stop_reply,
     ptid = select_thread_for_ambiguous_stop_reply (status);
   gdb_assert (ptid != null_ptid);
 
-  if (status->kind != TARGET_WAITKIND_EXITED
-      && status->kind != TARGET_WAITKIND_SIGNALLED
-      && status->kind != TARGET_WAITKIND_NO_RESUMED)
+  if (status->kind == TARGET_WAITKIND_EXITED
+      || status->kind == TARGET_WAITKIND_SIGNALLED)
+    {
+      remote_state *rs = get_remote_state ();
+      if (rs->in_replay_diversion)
+	{
+	  status->kind = TARGET_WAITKIND_REPLAY_DIVERSION_ENDED;
+	  rs->in_replay_diversion = false;
+	  for (thread_info *tp : all_non_exited_threads (this))
+	    get_remote_thread_info (tp)->set_not_resumed ();
+	}
+    }
+  else if (status->kind != TARGET_WAITKIND_NO_RESUMED)
     {
       /* Expedited registers.  */
       if (!stop_reply->regcache.empty ())
@@ -14247,6 +14276,84 @@  remote_target::btrace_conf (const struct btrace_target_info *tinfo)
   return &tinfo->conf;
 }
 
+bool
+remote_target::needs_replay_diversion ()
+{
+  return packet_support (PACKET_vReplayDiversion) == PACKET_ENABLE;
+}
+
+bool
+remote_target::in_replay_diversion ()
+{
+  return get_remote_state ()->in_replay_diversion;
+}
+
+void
+remote_target::start_replay_diversion ()
+{
+  REMOTE_SCOPED_DEBUG_ENTER_EXIT;
+
+  if (!needs_replay_diversion ())
+    return;
+
+  remote_state *rs = get_remote_state ();
+  if (rs->in_replay_diversion)
+    error ("Already in diversion");
+
+  remote_debug_printf ("Starting diversion");
+
+  xsnprintf (rs->buf.data (), get_remote_packet_size (), "vReplayDiversion:1");
+  putpkt (rs->buf);
+  getpkt (&rs->buf, 0);
+
+  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vReplayDiversion]))
+    {
+    case PACKET_OK:
+      rs->in_replay_diversion = true;
+      return;
+    case PACKET_ERROR:
+      warning (_("Remote failure reply: %s"), rs->buf.data ());
+      break;
+    case PACKET_UNKNOWN:
+      break;
+    }
+
+  error ("Failed to start diversion");
+}
+
+void
+remote_target::end_replay_diversion ()
+{
+  REMOTE_SCOPED_DEBUG_ENTER_EXIT;
+
+  if (!needs_replay_diversion ())
+    return;
+
+  remote_state *rs = get_remote_state ();
+  if (!rs->in_replay_diversion)
+    error ("Not in diversion");
+
+  remote_debug_printf ("Ending diversion");
+
+  xsnprintf (rs->buf.data (), get_remote_packet_size (), "vReplayDiversion:0");
+  putpkt (rs->buf);
+  getpkt (&rs->buf, 0);
+
+  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vReplayDiversion]))
+    {
+    case PACKET_OK:
+      rs->in_replay_diversion = false;
+      return;
+    case PACKET_ERROR:
+      warning (_("Remote failure reply: %s"), rs->buf.data ());
+      break;
+    case PACKET_UNKNOWN:
+      break;
+    }
+
+  error ("Failed to end diversion");
+}
+
 bool
 remote_target::augmented_libraries_svr4_read ()
 {
@@ -15222,6 +15329,9 @@  Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_memory_tagging_feature],
 			 "memory-tagging-feature", "memory-tagging-feature", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_vReplayDiversion],
+			 "vReplayDiversion", "replay-diversion", 0);
+
   /* Assert that we've registered "set remote foo-packet" commands
      for all packet configs.  */
   {