Re: [discuss] Support for reverse-execution

[Johan Rydberg <jrydberg@virtutech.com>]

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

Dan Shearer wrote:

> Johan Rydberg, a developer at Virtutech, has got some patches against
> 6.3 (for discussion only) that he'll be sending along to this list. I
> want to bring up bookmarks for discussion, a topic that isn't yet
> addressed by his patches.
> 
> First, what is addressed: Johan has implemented the reverse instructions
> rstep, rstepi, rnext and rnexti for PowerPC, Sparc and Sparc64 targets.
 > [...]

Here's a patch against CVS that implements the following commands:

   - rstep, rstepi
   - rnext, rnexti

The obvious commands "rcontinue" and "rfinished" are not yet
implemented, and therefor not included in this patch.  The command
names are of course up for discussion.

Other things that is missing from the patch:

   - Error handling.  GDB will not complain if the remote stub does
     not implement reverse execution.

   - Documentaton of the commands.

   - Some corner cases are not handled yet.

   - Mechanism for working with bookmarks.

What I have done is add a new variable, step_direction, that can either
be STEP_DIR_FORWARD or STEP_DIR_REVERSE.  A new target vector has also
been added; to_reverse.  The remote protocol has been extended with two
new functions: "bs" and "bc"

To implement "rnext" I had to add a new target-specific function that
tries to figure out the address of the call instruction based on the
last executed instruction and the instruction to be executed, IF the
current instruction can be identified as a return insn.

This patch should not been seen as a straight forward contribution, but
as a proof-of-concept implementation.  A lot of things could probably
be done better, and more in the "GDB-way", so any input is will
appreciated.

Currently the patch only works for PowerPC and SPARC targets, though
I plan to add support for a few more targets in the near future.

Obviously it is a bit hard to test without access to a reversible target
but  we are working on changing that in the next month or so (Dan has
more  information on that.)

Thanks,
Johan

[-- Attachment #2: gdb-reverse-1.patch --]
[-- Type: text/x-patch, Size: 19492 bytes --]

Index: gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.361
diff -u -r1.361 gdbarch.sh
--- gdbarch.sh	14 May 2005 06:07:41 -0000	1.361
+++ gdbarch.sh	19 May 2005 12:57:30 -0000
@@ -591,6 +591,10 @@
 v:=:int:frame_red_zone_size
 #
 m::CORE_ADDR:convert_from_func_ptr_addr:CORE_ADDR addr, struct target_ops *targ:addr, targ::convert_from_func_ptr_addr_identity::0
+
+# When running backwards it's needed to calculate the address of the
+# call instruction when the single stepper hits a return insn.
+f:=:int:call_insn_from_return_addr:CORE_ADDR pc, CORE_ADDR prev_pc, CORE_ADDR *call_pc:pc, prev_pc, call_pc::0:
 # On some machines there are bits in addresses which are not really
 # part of the address, but are used by the kernel, the hardware, etc.
 # for special purposes.  ADDR_BITS_REMOVE takes out any such bits so
Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.137
diff -u -r1.137 infcmd.c
--- infcmd.c	12 May 2005 20:21:17 -0000	1.137
+++ infcmd.c	19 May 2005 12:57:30 -0000
@@ -100,7 +100,7 @@
 
 static void jump_command (char *, int);
 
-static void step_1 (int, int, char *);
+static void step_1 (int, int, char *, enum step_direction_kind);
 static void step_once (int skip_subroutines, int single_inst, int count);
 static void step_1_continuation (struct continuation_arg *arg);
 
@@ -579,13 +579,39 @@
 
   proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0);
 }
+
+\f
+static void
+rstep_command (char *count_string, int from_tty)
+{
+  step_1 (0, 0, count_string, STEP_DIR_REVERSE);
+}
+
+static void
+rstepi_command (char *count_string, int from_tty)
+{
+  step_1 (0, 1, count_string, STEP_DIR_REVERSE);
+}
+
+static void
+rnext_command (char *count_string, int from_tty)
+{
+  step_1 (1, 0, count_string, STEP_DIR_REVERSE);
+}
+
+static void
+rnexti_command (char *count_string, int from_tty)
+{
+  step_1 (1, 1, count_string, STEP_DIR_REVERSE);
+}
+
 \f
 /* Step until outside of current statement.  */
 
 static void
 step_command (char *count_string, int from_tty)
 {
-  step_1 (0, 0, count_string);
+  step_1 (0, 0, count_string, STEP_DIR_FORWARD);
 }
 
 /* Likewise, but skip over subroutine calls as if single instructions.  */
@@ -593,7 +619,7 @@
 static void
 next_command (char *count_string, int from_tty)
 {
-  step_1 (1, 0, count_string);
+  step_1 (1, 0, count_string, STEP_DIR_FORWARD);
 }
 
 /* Likewise, but step only one instruction.  */
@@ -601,13 +627,13 @@
 void
 stepi_command (char *count_string, int from_tty)
 {
-  step_1 (0, 1, count_string);
+  step_1 (0, 1, count_string, STEP_DIR_FORWARD);
 }
 
 void
 nexti_command (char *count_string, int from_tty)
 {
-  step_1 (1, 1, count_string);
+  step_1 (1, 1, count_string, STEP_DIR_FORWARD);
 }
 
 static void
@@ -617,7 +643,8 @@
 }
 
 static void
-step_1 (int skip_subroutines, int single_inst, char *count_string)
+step_1 (int skip_subroutines, int single_inst, char *count_string,
+	enum step_direction_kind direction)
 {
   int count = 1;
   struct frame_info *frame;
@@ -659,6 +686,7 @@
       for (; count > 0; count--)
 	{
 	  clear_proceed_status ();
+	  step_direction = direction;
 
 	  frame = get_current_frame ();
 	  if (!frame)		/* Avoid coredump here.  Why tho? */
@@ -712,6 +740,8 @@
      and handle them one at the time, through step_once(). */
   else
     {
+      gdb_assert (0);
+
       if (target_can_async_p ())
 	step_once (skip_subroutines, single_inst, count);
     }
@@ -2082,6 +2112,16 @@
 Argument N means do this N times (or till program stops for another reason)."));
   add_com_alias ("ni", "nexti", class_alias, 0);
 
+  add_com ("rstepi", class_run, rstepi_command,
+ 	   "Reverse one instruction, but proceed through subroutine calls.\n\
+Argument N means do this N times (or till program stops for another reason).");
+  add_com_alias ("rsi", "rstepi", class_alias, 0);
+ 
+  add_com ("rnexti", class_run, rnexti_command,
+ 	   "Reverse one instruction, skipping subroutine calls.\n\
+Argument N means do this N times (or till program stops for another reason).");
+  add_com_alias ("rni", "rnexti", class_run, 1);
+ 
   add_com ("finish", class_run, finish_command, _("\
 Execute until selected stack frame returns.\n\
 Upon return, the value returned is printed and put in the value history."));
@@ -2100,6 +2140,16 @@
 Argument N means do this N times (or till program stops for another reason)."));
   add_com_alias ("s", "step", class_run, 1);
 
+  add_com ("rstep", class_run, rstep_command,
+	   "Reverse program until it reaches a different source line.\n\
+Argument N means do this N times (or till program stops for another reason).");
+  add_com_alias ("rs", "rstep", class_run, 1);
+ 
+  add_com ("rnext", class_run, rnext_command,
+           "Step program until it reaches the previous source line.\n\
+Argument N means do this N times (or till program stops for another reason).");
+   add_com_alias ("rn", "rnext", class_run, 1);
+
   c = add_com ("until", class_run, until_command, _("\
 Execute until the program reaches a source line greater than the current\n\
 or a specified location (same args as break command) within the current frame."));
Index: inferior.h
===================================================================
RCS file: /cvs/src/src/gdb/inferior.h,v
retrieving revision 1.71
diff -u -r1.71 inferior.h
--- inferior.h	14 May 2005 06:07:42 -0000	1.71
+++ inferior.h	19 May 2005 12:57:30 -0000
@@ -373,6 +373,17 @@
 
 extern enum step_over_calls_kind step_over_calls;
 
+/* STEP_DIR_FORWARD means normal execution,
+   STEP_DIR_REVERSE means going back in time.  */
+
+enum step_direction_kind
+  {
+    STEP_DIR_FORWARD,
+    STEP_DIR_REVERSE
+  };
+
+extern enum step_direction_kind step_direction;
+
 /* If stepping, nonzero means step count is > 1
    so don't print frame next time inferior stops
    if it stops due to stepping.  */
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.201
diff -u -r1.201 infrun.c
--- infrun.c	12 May 2005 20:21:17 -0000	1.201
+++ infrun.c	19 May 2005 12:57:32 -0000
@@ -247,6 +247,10 @@
 
 static struct symbol *step_start_function;
 
+/* Direction of which the execution is heading.  */
+
+enum step_direction_kind step_direction;
+
 /* Nonzero if we are expecting a trace trap and should proceed from it.  */
 
 static int trap_expected;
@@ -509,7 +513,6 @@
     }
 }
 
-
 /* Resume the inferior, but allow a QUIT.  This is useful if the user
    wants to interrupt some lengthy single-stepping operation
    (for child processes, the SIGINT goes to the inferior, and so
@@ -620,13 +623,19 @@
 	  if (step && breakpoints_inserted && breakpoint_here_p (read_pc ()))
 	    step = 0;
 	}
-      target_resume (resume_ptid, step, sig);
+
+      if (step_direction == STEP_DIR_FORWARD)
+	target_resume (resume_ptid, step, sig);
+      else
+	{
+	  target_reverse (resume_ptid, step);
+	}
     }
 
   discard_cleanups (old_cleanups);
 }
-\f
 
+\f
 /* Clear out all variables saying what to do when inferior is continued.
    First do this, then set the ones you want, then call `proceed'.  */
 
@@ -638,6 +647,7 @@
   step_range_end = 0;
   step_frame_id = null_frame_id;
   step_over_calls = STEP_OVER_UNDEBUGGABLE;
+  step_direction = STEP_DIR_FORWARD;
   stop_after_trap = 0;
   stop_soon = NO_STOP_QUIETLY;
   proceed_to_finish = 0;
@@ -2099,6 +2109,21 @@
         if (debug_infrun)
 	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTATE_WHAT_STEP_RESUME\n");
 
+        /* If stepping over functions calls while reversing we might
+           have to still continue running even if we hit the
+           breakpoint.  This to enable "prev" to step over recursive
+           calls.  */
+        if (step_over_calls == STEP_OVER_ALL
+            && step_direction == STEP_DIR_REVERSE
+            && (frame_id_inner (frame_unwind_id (get_current_frame ()), 
+                                step_frame_id)
+                || frame_id_eq (frame_unwind_id (get_current_frame ()), 
+                                step_frame_id)))
+          {
+	    keep_going (ecs);
+	    return;
+          }
+
 	if (step_resume_breakpoint == NULL)
 	  {
 	    step_resume_breakpoint =
@@ -2117,6 +2142,17 @@
 	    keep_going (ecs);
 	    return;
 	  }
+        /* If running backwards break out of this step-resume
+           breakpoint was set on the first insn in the line.  */
+
+        if (step_direction == STEP_DIR_REVERSE 
+            && stop_pc == step_range_start)
+          {
+            stop_step = 1;
+            print_stop_reason (END_STEPPING_RANGE, 0);
+            stop_stepping (ecs);
+            return;
+          }
 	break;
 
       case BPSTAT_WHAT_THROUGH_SIGTRAMP:
@@ -2268,7 +2304,7 @@
       return;
     }
 
-  if (step_resume_breakpoint)
+  if (step_resume_breakpoint && step_direction != STEP_DIR_REVERSE)
     {
       if (debug_infrun)
 	 fprintf_unfiltered (gdb_stdlog, "infrun: step-resume breakpoint\n");
@@ -2288,13 +2324,21 @@
       keep_going (ecs);
       return;
     }
-
+  
   /* If stepping through a line, keep going if still within it.
-
+     
      Note that step_range_end is the address of the first instruction
-     beyond the step range, and NOT the address of the last instruction
-     within it! */
-  if (stop_pc >= step_range_start && stop_pc < step_range_end)
+     beyond the step range, and NOT the address of the last
+     instruction within it!  Also, if running backwards, stop if we
+     hit the first instruction in the line. */
+  if (step_direction == STEP_DIR_REVERSE && stop_pc == step_range_start)
+    {
+      stop_step = 1;
+      print_stop_reason (END_STEPPING_RANGE, 0);
+      stop_stepping (ecs);
+      return;
+    }
+  else if (stop_pc >= step_range_start && stop_pc < step_range_end)
     {
       if (debug_infrun)
 	 fprintf_unfiltered (gdb_stdlog, "infrun: stepping inside range [0x%s-0x%s]\n",
@@ -2511,16 +2555,43 @@
 	}
       else
 	{
-	  /* Set a breakpoint at callee's return address (the address
-	     at which the caller will resume).  */
-	  insert_step_resume_breakpoint_at_frame (get_prev_frame (get_current_frame ()));
-	  keep_going (ecs);
-	  return;
+          if (step_direction == STEP_DIR_FORWARD)
+            {
+              /* Set a breakpoint at callee's return address (the
+                 address at which the caller will resume).  */
+              insert_step_resume_breakpoint_at_frame 
+                (get_prev_frame (get_current_frame ()));
+              keep_going (ecs);
+              return;
+            }
+          else
+            {
+              /* FIXME: gdb-reverse */
+              gdb_assert (0);
+            }
 	}
     }
 
   if (step_range_end == 1)
     {
+      CORE_ADDR call_pc;
+
+      /* For previ we must also check so we did not just end up inside
+         a subroutine.  */
+      if (step_direction == STEP_DIR_REVERSE 
+          && step_over_calls == STEP_OVER_ALL
+          && CALL_INSN_FROM_RETURN_ADDR (stop_pc, prev_pc, &call_pc))
+        {
+	  struct symtab_and_line sr_sal;
+
+	  init_sal (&sr_sal);
+	  sr_sal.pc = call_pc;
+
+	  insert_step_resume_breakpoint_at_sal (sr_sal, null_frame_id);
+          keep_going (ecs);
+          return;
+        }
+
       /* It is stepi or nexti.  We always want to stop stepping after
          one instruction.  */
       if (debug_infrun)
@@ -2563,6 +2634,32 @@
       return;
     }
 
+  if (step_direction == STEP_DIR_REVERSE 
+      && step_over_calls == STEP_OVER_ALL)
+    {
+      /* Going backwards we can make the assumption that if stop_pc is
+         not within the stepping range, prev_pc was the target of a
+         branch (insn that stop_pc points to).  If stop_pc is a return
+         instruction, and subroutine calls should be stepped over, do
+         it here.  It is also known that prev_pc was the last executed
+         insn, i.e., the target of the branch.  */
+      /* It has already been proven that stop_pc is not within the
+         stepping range.  */
+      CORE_ADDR call_pc;
+
+      if (CALL_INSN_FROM_RETURN_ADDR (stop_pc, prev_pc, &call_pc))
+        {
+	  struct symtab_and_line sr_sal;
+
+	  init_sal (&sr_sal);
+	  sr_sal.pc = call_pc;
+
+	  insert_step_resume_breakpoint_at_sal (sr_sal, step_frame_id);
+          keep_going (ecs);
+          return;
+        }
+    }
+
   /* We aren't done stepping.
 
      Optimize by setting the stepping range to the line.
@@ -2570,7 +2667,8 @@
      new line in mid-statement, we continue stepping.  This makes
      things like for(;;) statements work better.)  */
 
-  if (ecs->stop_func_end && ecs->sal.end >= ecs->stop_func_end)
+  if (ecs->stop_func_end && ecs->sal.end >= ecs->stop_func_end
+      && step_direction == STEP_DIR_FORWARD)
     {
       /* If this is the last line of the function, don't keep stepping
          (it would probably step us out of the function).
@@ -2621,7 +2719,7 @@
 }
 
 /* Are we in the middle of stepping?  */
-
+ 
 static int
 currently_stepping (struct execution_control_state *ecs)
 {
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.190
diff -u -r1.190 remote.c
--- remote.c	16 May 2005 16:36:24 -0000	1.190
+++ remote.c	19 May 2005 12:57:32 -0000
@@ -2617,6 +2617,26 @@
   if (target_is_async_p ())
     target_executing = 1;
 }
+
+/* Run in reverse.  */
+static int
+remote_reverse (ptid_t ptid, int step)
+{
+  struct remote_state *rs = get_remote_state ();
+  char *buf = alloca (rs->remote_packet_size);
+  int pid = PIDGET (ptid);
+  
+  /* All other supported resume packets do use Hc, so call set_thread.  */
+  if (pid == -1)
+    set_thread (0, 0);		/* run any thread */
+  else
+    set_thread (pid, 0);	/* run this thread */
+
+  strcpy (buf, step ? "bs" : "bc");
+  putpkt (buf);
+
+  return 0;
+}
 \f
 
 /* Set up the signal handler for SIGINT, while the target is
@@ -5413,6 +5433,7 @@
   remote_ops.to_has_registers = 1;
   remote_ops.to_has_execution = 1;
   remote_ops.to_has_thread_control = tc_schedlock;	/* can lock scheduler */
+  remote_ops.to_reverse = remote_reverse;
   remote_ops.to_magic = OPS_MAGIC;
 }
 
Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.240
diff -u -r1.240 rs6000-tdep.c
--- rs6000-tdep.c	1 May 2005 19:58:55 -0000	1.240
+++ rs6000-tdep.c	19 May 2005 12:57:33 -0000
@@ -2824,6 +2824,26 @@
   else
     return print_insn_little_powerpc (memaddr, info);
 }
+
+\f
+static int
+rs6000_call_insn_from_return_addr (CORE_ADDR pc, CORE_ADDR prev_pc, 
+                                   CORE_ADDR *call_pc)
+{
+  char buf[4];
+
+  if (target_read_memory (pc, buf, 4))
+    return 0;
+
+  if (extract_unsigned_integer (buf, 4) == 0x4e800020)
+    {
+      *call_pc = prev_pc - 4;
+      return 1;
+    }
+
+  return 0;
+}
+
 \f
 static CORE_ADDR
 rs6000_unwind_pc (struct gdbarch *gdbarch, struct frame_info *next_frame)
@@ -3302,6 +3322,7 @@
   set_gdbarch_convert_register_p (gdbarch, rs6000_convert_register_p);
   set_gdbarch_register_to_value (gdbarch, rs6000_register_to_value);
   set_gdbarch_value_to_register (gdbarch, rs6000_value_to_register);
+  set_gdbarch_call_insn_from_return_addr (gdbarch, rs6000_call_insn_from_return_addr);
 
   set_gdbarch_stab_reg_to_regnum (gdbarch, rs6000_stab_reg_to_regnum);
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, rs6000_dwarf2_reg_to_regnum);
Index: sparc-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.c,v
retrieving revision 1.163
diff -u -r1.163 sparc-tdep.c
--- sparc-tdep.c	14 May 2005 13:45:22 -0000	1.163
+++ sparc-tdep.c	19 May 2005 12:57:33 -0000
@@ -868,6 +868,57 @@
 }
 \f
 
+static int
+return_insn_p (unsigned int v, int *regno)
+{
+  if (v == 0x81c7e008) /* jmpl %i7+8,%g0 */
+    {
+      *regno = SPARC_I7_REGNUM;
+      return 1;
+    }
+  else if (v == 0x81c3e008) /* jmpl %o7+8,%g0 */
+    {
+      *regno = SPARC_O7_REGNUM;
+      return 1;
+    }
+  else
+    {
+      /* FIXME: more cases? */
+      return 0;
+    }
+}
+
+static int
+sparc_call_insn_from_return_addr (CORE_ADDR pc, CORE_ADDR prev_pc, 
+                                  CORE_ADDR *call_pc)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  int regno;
+
+  /* Calculating the address of the call instruction is quite tricky.
+     First we must check so that pc - 4 is a return insn, and that it
+     has already been executed (i.e., that the insn at PC is really in
+     PC-4's delay slot).  This is done by checking the value of the
+     NPC register.  */
+  
+  if (return_insn_p (sparc_fetch_instruction (pc - 4), &regno))
+    {
+      static CORE_ADDR npc, reg;
+
+      npc = sparc_address_from_register (tdep->npc_regnum);
+      reg = sparc_address_from_register (regno);
+
+      /* Set *call_pc so it points to the delay slot of the call
+         instruction.  This is needed for the "previ" case.  */
+      *call_pc = prev_pc - 4;
+      return ((reg == prev_pc - 8) && npc == prev_pc);
+    }
+
+  return 0;
+}
+
+\f
+
 /* Extract from an array REGBUF containing the (raw) register state, a
    function return value of TYPE, and copy that into VALBUF.  */
 
@@ -1211,6 +1262,8 @@
   set_gdbarch_pc_regnum (gdbarch, SPARC32_PC_REGNUM); /* %pc */
   set_gdbarch_fp0_regnum (gdbarch, SPARC_F0_REGNUM); /* %f0 */
 
+  set_gdbarch_call_insn_from_return_addr (gdbarch, sparc_call_insn_from_return_addr);
+
   /* Call dummy code.  */
   set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
   set_gdbarch_push_dummy_code (gdbarch, sparc32_push_dummy_code);
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.108
diff -u -r1.108 target.c
--- target.c	16 May 2005 16:36:24 -0000	1.108
+++ target.c	19 May 2005 12:57:33 -0000
@@ -457,6 +457,7 @@
       INHERIT (to_make_corefile_notes, t);
       INHERIT (to_get_thread_local_address, t);
       INHERIT (to_magic, t);
+      INHERIT (to_reverse, t);
     }
 #undef INHERIT
 
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.73
diff -u -r1.73 target.h
--- target.h	16 May 2005 04:45:43 -0000	1.73
+++ target.h	19 May 2005 12:57:33 -0000
@@ -425,6 +425,7 @@
     int to_magic;
     /* Need sub-structure for target machine related rather than comm related?
      */
+    int (*to_reverse) (ptid_t, int);
   };
 
 /* Magic number for checking ops size.  If a struct doesn't end with this
@@ -498,6 +499,16 @@
     (*current_target.to_resume) (ptid, step, siggnal);			\
   } while (0)
 
+/* Resume execution of the process TPID, but run in reverse.  STEP
+   says whether to single-step or to run free.  Returns -1 if the
+   target does not support reverse execution.  */
+
+#define target_reverse(ptid, step)					\
+  (current_target.to_reverse 						\
+   ? (*current_target.to_reverse) (ptid, step) : -1)
+
+#define target_reserve_p() (current_target.to_reserve ? 1 : 0)
+
 /* Wait for process pid to do something.  PTID = -1 to wait for any
    pid to do something.  Return pid of child, or -1 in case of error;
    store status through argument pointer STATUS.  Note that it is