Re: [PATCH] Add 'reverse' capability query to remote protocol (qSupported).

[Michael Snyder <msnyder@vmware.com>]

[-- Attachment #1: Type: text/plain, Size: 1611 bytes --]

Michael Snyder wrote:
> Pedro Alves wrote:
>> On Sunday 06 September 2009 04:36:22, Michael Snyder wrote:
>>> Eli Zaretskii wrote:
>>>>> From: Pedro Alves <pedro@codesourcery.com>
>>>>> Date: Tue, 1 Sep 2009 16:44:13 +0100
>>>>> Cc: Michael Snyder <msnyder@vmware.com>,  Jakob Engblom <jakob@virtutech.com>,  Greg Law <glaw@undo-software.com>
>>>>>
>>>>> Your patch also needs docs and NEWS entries, BTW.
>>>> And a patch for the manual documenting the new packets, no?
>>> New diff incorporating comments and adding docs and NEWS.
>> What about the i18n comments?
> 
> Oof, sorry, forgot.  You just mean the two error msgs, right?
> See revised diff.
> 
>> What about the vCont (the one about not sending vcont
>> if requesting a reverse resume) comments?
> 
> Are you sure?  I guess, like you, I hoped it would eventually
> be added.  Works fine as it is, it probes and fails, but if
> you want it, ok...  added below.
> 
> I have one final question to raise.
> 
> You may notice (though nobody has commented), that I made the
> two new supported-probed-packets (bs and bc) default to "enabled".
> 
> This sort of defeats the purpose (if the purpose is that we can
> decide whether to run a testsuite on a remote target) -- but I
> was just reluctant to default them to "disabled", because it
> would mean that anybody with a deployed target that doesn't yet
> support the new "qSupported" probe would have to make his users
> enable them by hand.
> 
> (why I cc:ed Jakob and Greg.)
> 
> So now that I've mentioned it, what do you think?
> Enabled, or disabled by default?

Arrr, patch attached this time.




[-- Attachment #2: qSupported-4.txt --]
[-- Type: text/plain, Size: 5648 bytes --]

2009-09-07  Michael Snyder  <msnyder@vmware.com>

	* remote.c (PACKET_bc, PACKET_bs): New enums.
	(remote_protocol_features): Add ReverseStep, ReverseContinue.
	(remote_resume): Check for reverse capability.
	(_initialize_remote): Add packet config for "bs" and "bc" packets.
	* NEWS (new options): Mention set/show for "bs" and "bc" packets.

2009-09-07  Michael Snyder  <msnyder@vmware.com>

	* gdb.texinfo (qSupported): Mention new ReverseContinue and
	ReverseStep replies to the qSupported query.

Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.370
diff -u -p -r1.370 remote.c
--- remote.c	18 Aug 2009 16:17:16 -0000	1.370
+++ remote.c	7 Sep 2009 22:10:56 -0000
@@ -1000,6 +1000,8 @@ enum {
   PACKET_qXfer_siginfo_write,
   PACKET_qAttached,
   PACKET_ConditionalTracepoints,
+  PACKET_bc,
+  PACKET_bs,
   PACKET_MAX
 };
 
@@ -3051,6 +3053,10 @@ static struct protocol_feature remote_pr
     PACKET_qXfer_siginfo_write },
   { "ConditionalTracepoints", PACKET_DISABLE, remote_cond_tracepoint_feature,
     PACKET_ConditionalTracepoints },
+  { "ReverseContinue", PACKET_DISABLE, remote_supported_packet,
+    PACKET_bc },
+  { "ReverseStep", PACKET_DISABLE, remote_supported_packet,
+    PACKET_bs },
 };
 
 static void
@@ -3801,8 +3807,10 @@ remote_resume (struct target_ops *ops,
   remote_pass_signals ();
 
   /* The vCont packet doesn't need to specify threads via Hc.  */
-  if (remote_vcont_resume (ptid, step, siggnal))
-    goto done;
+  /* No reverse support (yet) for vCont.  */
+  if (execution_direction != EXEC_REVERSE)
+    if (remote_vcont_resume (ptid, step, siggnal))
+      goto done;
 
   /* All other supported resume packets do use Hc, so set the continue
      thread.  */
@@ -3818,6 +3826,14 @@ remote_resume (struct target_ops *ops,
       if (info_verbose && siggnal != TARGET_SIGNAL_0)
 	warning (" - Can't pass signal %d to target in reverse: ignored.\n",
 		 siggnal);
+
+      if (step 
+	  && remote_protocol_packets[PACKET_bs].support == PACKET_DISABLE)
+	error (_("Remote reverse-step not supported."));
+      if (!step
+	  && remote_protocol_packets[PACKET_bc].support == PACKET_DISABLE)
+	error ("_(Remote reverse-continue not supported."));
+
       strcpy (buf, step ? "bs" : "bc");
     }
   else if (siggnal != TARGET_SIGNAL_0)
@@ -8730,12 +8746,14 @@ remote_command (char *args, int from_tty
   help_list (remote_cmdlist, "remote ", -1, gdb_stdout);
 }
 
-static int remote_target_can_reverse = 1;
-
 static int
 remote_can_execute_reverse (void)
 {
-  return remote_target_can_reverse;
+  if (remote_protocol_packets[PACKET_bs].support == PACKET_ENABLE
+      || remote_protocol_packets[PACKET_bc].support == PACKET_ENABLE)
+    return 1;
+  else
+    return 0;
 }
 
 static int
@@ -9165,6 +9183,12 @@ Show the maximum size of the address (in
 			 "qGetTLSAddr", "get-thread-local-storage-address",
 			 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_bc],
+			 "bc", "reverse-continue", 0);
+
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_bs],
+			 "bs", "reverse-step", 0);
+
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qSupported],
 			 "qSupported", "supported-packets", 0);
 
Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.328
diff -u -p -r1.328 NEWS
--- NEWS	31 Aug 2009 20:18:45 -0000	1.328
+++ NEWS	7 Sep 2009 22:10:57 -0000
@@ -309,6 +309,14 @@ show remote write-siginfo-object
   Control use of remote protocol `qXfer:siginfo:write' (write-siginfo-object)
   packet.
 
+set remote reverse-continue
+show remote reverse-continue
+  Control use of remote protocol 'bc' (reverse-continue) packet.
+
+set remote reverse-step
+show remote reverse-step
+  Control use of remote protocol 'bs' (reverse-step) packet.
+
 set displaced-stepping
 show displaced-stepping
   Control displaced stepping mode.  Displaced stepping is a way to
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.620
diff -u -p -r1.620 gdb.texinfo
--- doc/gdb.texinfo	1 Sep 2009 18:48:58 -0000	1.620
+++ doc/gdb.texinfo	7 Sep 2009 22:10:57 -0000
@@ -27493,16 +27493,18 @@ breakpoint at @var{addr}.
 Don't use this packet.  Use the @samp{Z} and @samp{z} packets instead
 (@pxref{insert breakpoint or watchpoint packet}).
 
-@item bc
 @cindex @samp{bc} packet
+@anchor{bc}
+@item bc
 Backward continue.  Execute the target system in reverse.  No parameter.
 @xref{Reverse Execution}, for more information.
 
 Reply:
 @xref{Stop Reply Packets}, for the reply specifications.
 
-@item bs
 @cindex @samp{bs} packet
+@anchor{bs}
+@item bs
 Backward single step.  Execute one instruction in reverse.  No parameter.
 @xref{Reverse Execution}, for more information.
 
@@ -28746,6 +28748,16 @@ These are the currently defined stub fea
 @tab @samp{-}
 @tab No
 
+@item @samp{ReverseContinue}
+@tab No
+@tab @samp{+}
+@tab No
+
+@item @samp{ReverseStep}
+@tab No
+@tab @samp{+}
+@tab No
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -28827,6 +28839,14 @@ The remote stub understands the @samp{qX
 The remote stub accepts and implements conditional expressions defined
 for tracepoints (@pxref{Tracepoint Conditions}).
 
+@item ReverseContinue
+The remote stub accepts and implements the reverse continue packet
+(@pxref{bc}).
+
+@item ReverseStep
+The remote stub accepts and implements the reverse step packet
+(@pxref{bs}).
+
 @end table
 
 @item qSymbol::