[reverse] PATCH: Several interface changes

[reverse] PATCH: Several interface changes

[Pedro Alves]

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

On Monday 06 October 2008 23:11:14, Michael Snyder wrote:
> Pedro Alves wrote:
> > A per-target property may seems to make sense on
> > single-threaded,single-inferior targets, but when you add support
> > for multi-inferiors per target (e.g., extended-remote has some of it now,
> > and I'm going to push more of it), or multi-threaded support, the
> > per-target setting may not make sense anymore --- explicit requests
> > at the target resume interface (just like your new packets) may make
> > more sense.  Imagine forward execution non-stop debugging in all threads
> > but one, which the user is examining in reverse.  What's the target
> > direction in this case?
> 
> Yakkk!!!

:-)  Here's an alternative interface I was considering and envisioning
when I mentioned the above.  Consider this just a suggestion.  If it
looks bad, let's quickly forget about it.

> > The question to me is --- when/why does the target (as in, the debug
> > API abstraction) ever need to know about the current direction that
> > it couldn't get from the core's request?

So, after last night's discussion, I came up with the attached to
see how it would look like.  The major change is that I consider the
reverse/forward-direction thing a property or the command the user
requested, and as such, belongs together with the other thread
stepping state we keep in struct thread_info, and the
target_ops implementation, adjusts itself to the direction GDB
requests with target_resume.  I've extended target_resume's interface
to accept this instead of a `step' boolean:

 enum target_resume_kind
   {
     /* Continue forward.  */
     rk_continue,

     /* Step forward.  */
     rk_step,

     /* Continue in the reverse direction.  */
     rk_continue_reverse,

     /* Step in the reverse direction.  */
     rk_step_reverse,
   };

(notice that the order of the things in the enum allows me to
miss some conversions --- I'm lazy).

  I can't say if I like this new target_resume interface or
not.  I just tried doing it to see how it would look.

(I can imagine that we're in the future going to extend the
target_resume interface to be more like gdbserver's, but, well,
that's another issue.)

So, the interface at the command level implementation is just
like it was before:

 1)  call clear_proceed_status ();

 2)  /* construct the step/continue request */

 3)  call proceed (...);

Where in #2, you can set the thread to go backwards by
doing:

     thread->reverse = 1;

The attached patch applies against the reverse-20080930-branch.

Other things I've done in the patch:

   * Added support for a "Tnn nohistory" stop reply that translates
    to TARGET_WAITKIND_NO_HISTORY.  When going multi-threaded, or
    multi-process this will allow things like "T05;thread:pPID.TID;nohistory"
    for free.  I absolutelly didn't test this.  I've no reverse aware target
    at hand.

   * At places, error out if async + reverse or non-stop + reverse
     was requested.

   * Added a target_can_reverse_p method, so infcmd.c can check if the
     target supports reverse execution before calling into the target.  Not
     strictly necessary, though, but I thought this was nicer this way.

I checked that I can use the record target on x86 (actually x86_64
with -m32) as good as without the patch, but it's quite possible I
broke things badly.

-- 
Pedro Alves

[-- Attachment #2: per-thread-reverse.diff --]
[-- Type: text/x-diff, Size: 44011 bytes --]

2008-10-07  Pedro Alves  <pedro@codesourcery.com>

	* gdbthread.h (struct thread_info) <reverse>: New field.
	* inf-ptrace.c (inf_ptrace_resume): Adjust.
	* infcmd.c (continue_1): Rename to ...
	(continue_1_1): ... this.  Add `reverse' parameter.  Adjust.
	(continue_command): Error out in reverse + async.  Call
	continue_1_1 instead of continue_1.
	(step_1): Error out in reverse + async or if the target doesn't
	support reverse execution.  Set the current thread's execution
	direction.  Adjust calls to step_once.
	(struct step_1_continuation_args) <reverse>: New field.
	(step_1_continuation): Adjust.
	(step_once): Add new `reverse' parameter.  Adjust.
	(finish_command): Adjust.
	* inferior.h (set_execution_direction, execution_direction):
	Declare.
	* infrun.c (displaced_step_fixup): Adjust.
	(clear_proceed_status): Clear tp->reverse.
	(proceed, handle_inferior_event): Adjust.
	* linux-nat.c (resume_callback, linux_nat_resume): Adjust.
	* record.c (record_beneath_to_resume): Adjust.
	(record_is_replay): New.
	(record_resume): Adjust.
	(record_wait, record_store_registers, record_xfer_partial)
	(record_insert_breakpoint, record_remove_breakpoint): Use
	record_is_replay instead of RECORD_IS_REPLAY.
	(record_get_exec_direction, record_set_exec_direction): Delete.
	(record_can_reverse_p): New.
	(init_record_ops): Register record_can_reverse_p.  Don't register
	record_get_exec_direction or record_set_exec_direction.
	(cmd_record_delete): Adjust.
	* record.h (RECORD_IS_REPLAY): Delete.
	(record_beneath_to_resume): Adjust.
	* remote.c (remote_vcont_resume, remote_resume): Adjust.
	(remote_wait): Accept a "nohistory" special register in T stop
	replies, and translate that to TARGET_WAITKIND_NO_HISTORY.
	(remote_can_reverse_p): New.
	(remote_get_exec_direction, remote_set_exec_direction): Delete.
	(init_remote_ops): Don't register remote_get_exec_direction or
	remote_set_exec_direction.  Register remote_can_reverse_p.
	* reverse.c: Include "inferior.h".
	(current_execution_direction): New global.
	(set_execution_direction, execution_direction): New.
	(set_exec_direction_func, show_exec_direction_func): Rewrite,
	don't throw errors.
	(exec_direction_default): Rename to ...
	(exec_direction_forward): ... this.
	(exec_reverse_once): Don't check for target support here.
	Adjust.
	* target.c (update_current_target): Don't inherit
	to_get_exec_direction or to_set_exec_direction.  Inherit and
	de_fault to_can_reverse_p.  Adjust.
	(target_resume, debug_to_resume): Adjust.
	* target.h (enum target_resume_kind): New.
	(struct target_ops) <to_resume>: Adjust to take it.
	<to_set_exec_direction, exec_direction_kind>: Delete fields.
	<to_can_reverse_p>: New field.
	(target_resume): Adjust.
	(target_can_reverse_p): New.
	(target_get_execution_direction, target_set_execution_direction): Delete.
	* fork-child.c (startup_inferior): Adjust.
	* i386-linux-nat.c (i386_linux_resume): Adjust.
	
---
 gdb/fork-child.c     |    4 +-
 gdb/gdbthread.h      |    3 +
 gdb/i386-linux-nat.c |    4 +-
 gdb/inf-ptrace.c     |    4 +-
 gdb/infcmd.c         |   68 ++++++++++++++++++++++++++++++++------
 gdb/inferior.h       |    6 +++
 gdb/infrun.c         |   36 ++++++++++++--------
 gdb/linux-nat.c      |    8 ++--
 gdb/record.c         |   82 ++++++++++++++++++++++-----------------------
 gdb/record.h         |    5 +-
 gdb/remote.c         |   72 +++++++++++++++++++++-------------------
 gdb/reverse.c        |   91 +++++++++++++++++++++++++++++++--------------------
 gdb/target.c         |   41 +++++++++++++++++-----
 gdb/target.h         |   57 +++++++++++++++++++------------
 14 files changed, 307 insertions(+), 174 deletions(-)

Index: src/gdb/gdbthread.h
===================================================================
--- src.orig/gdb/gdbthread.h	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/gdbthread.h	2008-10-07 16:20:31.000000000 +0100
@@ -168,6 +168,9 @@ struct thread_info
      at.  */
   bpstat stop_bpstat;
 
+  /* True if this thread is stepping/continuing in reverse.  */
+  int reverse;
+
   /* Private data used by the target vector implementation.  */
   struct private_thread_info *private;
 };
Index: src/gdb/inf-ptrace.c
===================================================================
--- src.orig/gdb/inf-ptrace.c	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/inf-ptrace.c	2008-10-07 16:20:31.000000000 +0100
@@ -352,10 +352,12 @@ inf_ptrace_stop (ptid_t ptid)
    that signal.  */
 
 static void
-inf_ptrace_resume (ptid_t ptid, int step, enum target_signal signal)
+inf_ptrace_resume (ptid_t ptid, enum target_resume_kind how,
+		   enum target_signal signal)
 {
   pid_t pid = ptid_get_pid (ptid);
   int request = PT_CONTINUE;
+  int step = (how == rk_step);
 
   if (pid == -1)
     /* Resume all threads.  Traditionally ptrace() only supports
Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/infcmd.c	2008-10-07 16:20:31.000000000 +0100
@@ -105,7 +105,8 @@ static void signal_command (char *, int)
 static void jump_command (char *, int);
 
 static void step_1 (int, int, char *);
-static void step_once (int skip_subroutines, int single_inst, int count, int thread);
+static void step_once (int skip_subroutines, int single_inst, int count, int reverse,
+		       int thread);
 
 static void next_command (char *, int);
 
@@ -586,11 +587,19 @@ proceed_thread_callback (struct thread_i
   return 0;
 }
 
-void
-continue_1 (int all_threads)
+/* Helper for continue_1.  */
+
+/* If ALL_THREADS, continue all threads forward.  If REVERSE, do so in
+   reverse execution mode.  */
+
+static void
+continue_1_1 (int all_threads, int reverse)
 {
   ERROR_NO_INFERIOR;
 
+  if (non_stop && reverse)
+    error (_("Non-stop mode not supported with reverse execution."));
+
   if (non_stop && all_threads)
     {
       /* Don't error out if the current thread is running, because
@@ -609,16 +618,31 @@ continue_1 (int all_threads)
     {
       ensure_not_running ();
       clear_proceed_status ();
+
+      inferior_thread ()->reverse = reverse;
+
       proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0);
     }
 }
 
+/* This should be renamed as it's a public function --- it is used by
+   MI.  */
+
+/* If ALL_THREADS, continue all threads forward.  */
+
+void
+continue_1 (int all_threads)
+{
+  continue_1_1 (all_threads, 0);
+}
+
 /* continue [-a] [proceed-count] [&]  */
 void
 continue_command (char *args, int from_tty)
 {
   int async_exec = 0;
   int all_threads = 0;
+  int reverse_exec = (execution_direction () == EXEC_REVERSE);
   ERROR_NO_INFERIOR;
 
   /* Find out whether we must run in the background. */
@@ -630,6 +654,13 @@ continue_command (char *args, int from_t
   if (async_exec && !target_can_async_p ())
     error (_("Asynchronous execution not supported on this target."));
 
+  if (reverse_exec && !target_can_reverse_p ())
+    error (_("Reverse execution not supported on this target."));
+
+  if (async_exec && reverse_exec)
+    error (_("\
+Asynchronous execution not supported with reverse execution."));
+
   /* If we are not asked to run in the bg, then prepare to run in the
      foreground, synchronously. */
   if (!async_exec && target_can_async_p ())
@@ -701,7 +732,7 @@ Can't resume all threads and specify pro
   if (from_tty)
     printf_filtered (_("Continuing.\n"));
 
-  continue_1 (all_threads);
+  continue_1_1 (all_threads, reverse_exec);
 }
 \f
 /* Step until outside of current statement.  */
@@ -748,6 +779,7 @@ step_1 (int skip_subroutines, int single
   struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
   int async_exec = 0;
   int thread = -1;
+  int reverse_exec = (execution_direction () == EXEC_REVERSE);
 
   ERROR_NO_INFERIOR;
   ensure_not_running ();
@@ -760,6 +792,13 @@ step_1 (int skip_subroutines, int single
   if (async_exec && !target_can_async_p ())
     error (_("Asynchronous execution not supported on this target."));
 
+  if (reverse_exec && !target_can_reverse_p ())
+    error (_("Reverse execution not supported on this target."));
+
+  if (async_exec && reverse_exec)
+    error (_("\
+Asynchronous execution not supported with reverse execution."));
+
   /* If we don't get a request of running in the bg, then we need
      to simulate synchronous (fg) execution. */
   if (!async_exec && target_can_async_p ())
@@ -788,6 +827,7 @@ step_1 (int skip_subroutines, int single
 	  struct thread_info *tp = inferior_thread ();
 	  clear_proceed_status ();
 	  tp->step_frame_id = get_frame_id (get_current_frame ());
+	  tp->reverse = reverse_exec;
 
 	  if (!single_inst)
 	    {
@@ -838,7 +878,7 @@ which has no line number information.\n"
      and handle them one at the time, through step_once(). */
   else
     {
-      step_once (skip_subroutines, single_inst, count, thread);
+      step_once (skip_subroutines, single_inst, count, reverse_exec, thread);
       /* We are running, and the continuation is installed.  It will
 	 disable the longjmp breakpoint as appropriate.  */
       discard_cleanups (cleanups);
@@ -850,6 +890,7 @@ struct step_1_continuation_args
   int count;
   int skip_subroutines;
   int single_inst;
+  int reverse;
   int thread;
 };
 
@@ -872,7 +913,8 @@ step_1_continuation (void *args)
 	{
 	  /* There are more steps to make, and we did stop due to
 	     ending a stepping range.  Do another step.  */
-	  step_once (a->skip_subroutines, a->single_inst, a->count - 1, a->thread);
+	  step_once (a->skip_subroutines, a->single_inst, a->count - 1,
+		     a->reverse, a->thread);
 	  return;
 	}
       tp->step_multi = 0;
@@ -892,7 +934,8 @@ step_1_continuation (void *args)
    called in case of step n with n>1, after the first step operation has
    been completed.*/
 static void 
-step_once (int skip_subroutines, int single_inst, int count, int thread)
+step_once (int skip_subroutines, int single_inst, int count, int reverse,
+	   int thread)
 {
   struct frame_info *frame;
   struct step_1_continuation_args *args;
@@ -906,6 +949,8 @@ step_once (int skip_subroutines, int sin
       struct thread_info *tp = inferior_thread ();
       clear_proceed_status ();
 
+      tp->reverse = reverse;
+
       frame = get_current_frame ();
       if (!frame)		/* Avoid coredump here.  Why tho? */
 	error (_("No current frame"));
@@ -956,6 +1001,7 @@ which has no line number information.\n"
       args->skip_subroutines = skip_subroutines;
       args->single_inst = single_inst;
       args->count = count;
+      args->reverse = reverse;
       args->thread = thread;
       add_intermediate_continuation (tp, step_1_continuation, args, xfree);
     }
@@ -1454,7 +1500,7 @@ finish_command (char *arg, int from_tty)
     error (_("Asynchronous execution not supported on this target."));
 
   /* Don't try to async in reverse.  */
-  if (async_exec && target_get_execution_direction () == EXEC_REVERSE)
+  if (async_exec && execution_direction () == EXEC_REVERSE)
     error (_("Asynchronous 'finish' not supported in reverse."));
 
   /* If we are not asked to run in the bg, then prepare to run in the
@@ -1478,6 +1524,8 @@ finish_command (char *arg, int from_tty)
 
   clear_proceed_status ();
 
+  tp->reverse = (execution_direction () == EXEC_REVERSE);
+
   /* Find the function we will return from.  */
 
   function = find_pc_function (get_frame_pc (get_selected_frame (NULL)));
@@ -1486,7 +1534,7 @@ finish_command (char *arg, int from_tty)
      source.  */
   if (from_tty)
     {
-      if (target_get_execution_direction () == EXEC_REVERSE)
+      if (tp->reverse)
 	printf_filtered (_("Run back to call of "));
       else
 	printf_filtered (_("Run till exit from "));
@@ -1494,7 +1542,7 @@ finish_command (char *arg, int from_tty)
       print_stack_frame (get_selected_frame (NULL), 1, LOCATION);
     }
 
-  if (target_get_execution_direction () == EXEC_REVERSE)
+  if (tp->reverse)
     {
       /* Split off at this point.  */
       finish_backward (function, tp);
Index: src/gdb/inferior.h
===================================================================
--- src.orig/gdb/inferior.h	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/inferior.h	2008-10-07 16:20:31.000000000 +0100
@@ -250,6 +250,12 @@ extern void error_is_running (void);
 /* Calls error_is_running if the current thread is running.  */
 extern void ensure_not_running (void);
 
+/* From reverse.c */
+
+extern void set_execution_direction (enum exec_direction_kind dir);
+
+extern enum exec_direction_kind execution_direction (void);
+
 /* From infcmd.c */
 
 extern void tty_command (char *, int);
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/infrun.c	2008-10-07 16:20:31.000000000 +0100
@@ -807,7 +807,7 @@ displaced_step_fixup (ptid_t event_ptid,
 
       displaced_step_ptid = null_ptid;
       displaced_step_prepare (ptid);
-      target_resume (ptid, 1, TARGET_SIGNAL_0);
+      target_resume (ptid, rk_step, TARGET_SIGNAL_0);
     }
 }
 
@@ -1071,7 +1071,12 @@ a command like `return' or `jump' to con
           displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf));
         }
 
-      target_resume (resume_ptid, step, sig);
+      if (step)
+	target_resume (resume_ptid,
+		       tp->reverse ? rk_step_reverse : rk_step, sig);
+      else
+	target_resume (resume_ptid,
+		       tp->reverse ? rk_continue_reverse : rk_continue, sig);
 
       /* Avoid confusing the next resume, if the next stop/resume
 	 happens to apply to another thread.  */
@@ -1101,6 +1106,7 @@ clear_proceed_status (void)
       tp->step_range_end = 0;
       tp->step_frame_id = null_frame_id;
       tp->step_over_calls = STEP_OVER_UNDEBUGGABLE;
+      tp->reverse = 0;
 
       tp->stop_step = 0;
 
@@ -1202,8 +1208,9 @@ proceed (CORE_ADDR addr, enum target_sig
 
   if (addr == (CORE_ADDR) -1)
     {
+      tp = inferior_thread ();
       if (pc == stop_pc && breakpoint_here_p (pc) 
-	  && target_get_execution_direction () != EXEC_REVERSE)
+	  && !tp->reverse)
 	/* There is a breakpoint at the address we will resume at,
 	   step one instruction before inserting breakpoints so that
 	   we do not stop right away (and report a second hit at this
@@ -2149,7 +2156,9 @@ handle_inferior_event (struct execution_
     case TARGET_WAITKIND_SYSCALL_RETURN:
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_SYSCALL_RETURN\n");
-      target_resume (ecs->ptid, 1, TARGET_SIGNAL_0);
+      target_resume (ecs->ptid,
+		     ecs->event_thread->reverse ? rk_step_reverse : rk_step,
+		     TARGET_SIGNAL_0);
       prepare_to_wait (ecs);
       return;
 
@@ -2197,7 +2206,9 @@ targets should add new threads to the th
 	 in either the OS or the native code).  Therefore we need to
 	 continue all threads in order to make progress.  */
 
-      target_resume (RESUME_ALL, 0, TARGET_SIGNAL_0);
+      /* Can we step backwards into a threads deletion?  Ah, right, we
+	 don't support multi-threading in reverse yet.  */
+      target_resume (RESUME_ALL, rk_continue, TARGET_SIGNAL_0);
       prepare_to_wait (ecs);
       return;
     }
@@ -2501,7 +2512,7 @@ targets should add new threads to the th
       if (!HAVE_STEPPABLE_WATCHPOINT)
 	remove_breakpoints ();
       registers_changed ();
-      target_resume (ecs->ptid, 1, TARGET_SIGNAL_0);	/* Single step */
+      target_resume (ecs->ptid, rk_step, TARGET_SIGNAL_0);	/* Single step */
       waiton_ptid = ecs->ptid;
       if (HAVE_STEPPABLE_WATCHPOINT)
 	infwait_state = infwait_step_watch_state;
@@ -2886,7 +2897,7 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
 	    return;
 	  }
 	if (stop_pc == ecs->stop_func_start
-	    && target_get_execution_direction () == EXEC_REVERSE)
+	    && ecs->event_thread->reverse)
 	  {
 	    /* We are stepping over a function call in reverse, and
 	       just hit the step-resume breakpoint at the start
@@ -3070,7 +3081,7 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
 	 keep going back to the call point).  */
       if (stop_pc == ecs->event_thread->step_range_start
 	  && stop_pc != ecs->stop_func_start
-	  && target_get_execution_direction () == EXEC_REVERSE)
+	  && ecs->event_thread->reverse)
 	{
 	  ecs->event_thread->stop_step = 1;
 	  print_stop_reason (END_STEPPING_RANGE, 0);
@@ -3140,7 +3151,7 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
 		    ecs->event_thread->step_frame_id)
       && (frame_id_eq (frame_unwind_id (get_current_frame ()),
 		       ecs->event_thread->step_frame_id)
-	  || target_get_execution_direction () == EXEC_REVERSE))
+	  || ecs->event_thread->reverse))
     {
       CORE_ADDR real_stop_pc;
 
@@ -3178,7 +3189,7 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
 	     get there, we'll need to single-step back to the
 	     caller.  */
 
-	  if (target_get_execution_direction () == EXEC_REVERSE)
+	  if (ecs->event_thread->reverse)
 	    {
 	      struct symtab_and_line sr_sal;
 	      init_sal (&sr_sal);
@@ -3230,7 +3241,7 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
 	    /* Find start of appropriate source line (either first or
 	       last line in callee, depending on execution
 	       direction).  */	      
-	    if (target_get_execution_direction () == EXEC_REVERSE)
+	    if (ecs->event_thread->reverse)
 	      stepped_into_function_backward (ecs);
 	    else
 	      stepped_into_function (ecs);
@@ -3250,7 +3261,7 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
 	  return;
 	}
 
-      if (target_get_execution_direction () == EXEC_REVERSE)
+      if (ecs->event_thread->reverse)
 	{
 	  /* Set a breakpoint at callee's start address.
 	     From there we can step once and be back in the caller.  */
@@ -4800,7 +4811,6 @@ show_non_stop (struct ui_file *file, int
 		    value);
 }
 
-
 void
 _initialize_infrun (void)
 {
Index: src/gdb/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/linux-nat.c	2008-10-07 16:20:31.000000000 +0100
@@ -1714,7 +1714,7 @@ resume_callback (struct lwp_info *lp, vo
   if (lp->stopped && lp->status == 0)
     {
       linux_ops->to_resume (pid_to_ptid (GET_LWP (lp->ptid)),
-			    0, TARGET_SIGNAL_0);
+			    rk_continue, TARGET_SIGNAL_0);
       if (debug_linux_nat)
 	fprintf_unfiltered (gdb_stdlog,
 			    "RC:  PTRACE_CONT %s, 0, 0 (resume sibling)\n",
@@ -1748,10 +1748,12 @@ resume_set_callback (struct lwp_info *lp
 }
 
 static void
-linux_nat_resume (ptid_t ptid, int step, enum target_signal signo)
+linux_nat_resume (ptid_t ptid, enum target_resume_kind how,
+		  enum target_signal signo)
 {
   struct lwp_info *lp;
   int resume_all;
+  int step = (how == rk_step);
 
   if (debug_linux_nat)
     fprintf_unfiltered (gdb_stdlog,
@@ -1859,7 +1861,7 @@ linux_nat_resume (ptid_t ptid, int step,
   if (resume_all)
     iterate_over_lwps (resume_callback, NULL);
 
-  linux_ops->to_resume (ptid, step, signo);
+  linux_ops->to_resume (ptid, how, signo);
   memset (&lp->siginfo, 0, sizeof (lp->siginfo));
 
   if (debug_linux_nat)
Index: src/gdb/record.c
===================================================================
--- src.orig/gdb/record.c	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/record.c	2008-10-07 16:20:31.000000000 +0100
@@ -53,7 +53,8 @@ int record_will_store_registers = 0;
 extern struct bp_location *bp_location_chain;
 
 /* The real beneath function pointers.  */
-void (*record_beneath_to_resume) (ptid_t, int, enum target_signal);
+void (*record_beneath_to_resume) (ptid_t, enum target_resume_kind,
+				  enum target_signal);
 ptid_t (*record_beneath_to_wait) (ptid_t, struct target_waitstatus *);
 void (*record_beneath_to_store_registers) (struct regcache *, int regno);
 LONGEST (*record_beneath_to_xfer_partial) (struct target_ops * ops,
@@ -432,15 +433,35 @@ record_close (int quitting)
   record_list_release (record_list);
 }
 
+static int record_is_replay;
+
 static void
-record_resume (ptid_t ptid, int step, enum target_signal siggnal)
+record_resume (ptid_t ptid, enum target_resume_kind how,
+	       enum target_signal siggnal)
 {
-  record_resume_step = step;
+  record_resume_step = (how == rk_step || how == rk_step_reverse);
+
+  switch (how)
+    {
+    case rk_step_reverse:
+    case rk_continue_reverse:
+      record_exec_direction = EXEC_REVERSE;
+      break;
+    case rk_step:
+    case rk_continue:
+      record_exec_direction = EXEC_FORWARD;
+      break;
+    }
 
-  if (!RECORD_IS_REPLAY)
+  record_is_replay = (record_list->next != NULL
+		      || how == rk_step_reverse
+		      || how == rk_continue_reverse);
+
+  if (!record_is_replay)
     {
       record_message (current_gdbarch);
-      record_beneath_to_resume (ptid, 1, siggnal);
+
+      record_beneath_to_resume (ptid, rk_step, siggnal);
     }
 }
 
@@ -487,10 +508,8 @@ record_wait (ptid_t ptid, struct target_
 			  record_resume_step);
     }
 
-  if (!RECORD_IS_REPLAY)
-    {
-      return record_beneath_to_wait (ptid, status);
-    }
+  if (!record_is_replay)
+    return record_beneath_to_wait (ptid, status);
   else
     {
       struct sigaction act, old_act;
@@ -597,8 +616,8 @@ record_wait (ptid_t ptid, struct target_
 	      if (record_debug > 1)
 		{
 		  fprintf_unfiltered (gdb_stdlog,
-				      "Process record: record_end 0x%s to inferior need_dasm = %d.\n",
-				      paddr_nz ((CORE_ADDR)record_list),
+				      "Process record: record_end %p to inferior need_dasm = %d.\n",
+				      record_list,
 				      record_list->u.need_dasm);
 		}
 
@@ -809,7 +828,7 @@ record_store_registers (struct regcache 
 {
   if (!record_not_record)
     {
-      if (RECORD_IS_REPLAY)
+      if (record_is_replay)
 	{
 	  int n;
 	  struct cleanup *old_cleanups;
@@ -874,7 +893,7 @@ record_xfer_partial (struct target_ops *
       && (object == TARGET_OBJECT_MEMORY
 	  || object == TARGET_OBJECT_RAW_MEMORY) && writebuf)
     {
-      if (RECORD_IS_REPLAY)
+      if (record_is_replay)
 	{
 	  /* Let user choice if he want to write memory or not.  */
 	  if (!nquery (_("Because GDB is in replay mode, writing to memory will destroy the record from this point forward.  Write memory at address 0x%s?"),
@@ -931,7 +950,7 @@ record_xfer_partial (struct target_ops *
 static int
 record_insert_breakpoint (struct bp_target_info *bp_tgt)
 {
-  if (!RECORD_IS_REPLAY)
+  if (!record_is_replay)
     {
       return record_beneath_to_insert_breakpoint (bp_tgt);
     }
@@ -942,7 +961,7 @@ record_insert_breakpoint (struct bp_targ
 static int
 record_remove_breakpoint (struct bp_target_info *bp_tgt)
 {
-  if (!RECORD_IS_REPLAY)
+  if (!record_is_replay)
     {
       return record_beneath_to_remove_breakpoint (bp_tgt);
     }
@@ -950,31 +969,13 @@ record_remove_breakpoint (struct bp_targ
   return 0;
 }
 
-static enum exec_direction_kind
-record_get_exec_direction (void)
-{
-  if (record_debug > 1)
-    printf_filtered ("Process record: exec direction is %s\n",
-		     record_exec_direction == EXEC_FORWARD ? "forward" :
-		     record_exec_direction == EXEC_REVERSE ? "reverse" : 
-		     "unknown");
-  return record_exec_direction;
-}
-
 static int
-record_set_exec_direction (enum exec_direction_kind dir)
+record_can_reverse_p (void)
 {
-  if (record_debug)
-    printf_filtered ("Process record: set exec direction: %s\n",
-		     dir == EXEC_FORWARD ? "forward" :
-		     dir == EXEC_REVERSE ? "reverse" : 
-		     "bad direction");
-
-  /* FIXME: check target for capability.  */
-  if (dir == EXEC_FORWARD || dir == EXEC_REVERSE)
-    return (record_exec_direction = dir);
-  else
-    return EXEC_ERROR;
+  /* Sure we can!  */
+
+  /* Actually, we should check for architecture support here.  */
+  return 1;
 }
 
 static void
@@ -997,8 +998,7 @@ init_record_ops (void)
   record_ops.to_xfer_partial = record_xfer_partial;
   record_ops.to_insert_breakpoint = record_insert_breakpoint;
   record_ops.to_remove_breakpoint = record_remove_breakpoint;
-  record_ops.to_get_exec_direction = record_get_exec_direction;
-  record_ops.to_set_exec_direction = record_set_exec_direction;
+  record_ops.to_can_reverse_p = record_can_reverse_p;
   record_ops.to_stratum = record_stratum;
   record_ops.to_magic = OPS_MAGIC;
 }
@@ -1026,7 +1026,7 @@ cmd_record_delete (char *args, int from_
 {
   if (RECORD_IS_USED)
     {
-      if (RECORD_IS_REPLAY)
+      if (record_is_replay)
 	{
 	  if (!from_tty || query (_("Process record: delete the log from this point forward and begin to record the running message at current PC?")))
 	    {
Index: src/gdb/record.h
===================================================================
--- src.orig/gdb/record.h	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/record.h	2008-10-07 16:20:31.000000000 +0100
@@ -22,8 +22,6 @@
 
 #define RECORD_IS_USED   \
      (current_target.beneath == &record_ops)
-#define RECORD_IS_REPLAY \
-     (record_list->next || record_exec_direction == EXEC_REVERSE)
 #define RECORD_TARGET_SUPPORT_RECORD_WAIT	(record_ops.beneath->to_support_record_wait)
 
 typedef struct record_reg_s
@@ -85,7 +83,8 @@ extern int record_arch_list_add_end (int
 extern void record_message (struct gdbarch *gdbarch);
 extern void record_not_record_set (void);
 
-extern void (*record_beneath_to_resume) (ptid_t, int, enum target_signal);
+extern void (*record_beneath_to_resume) (ptid_t, enum target_resume_kind,
+					 enum target_signal);
 extern ptid_t (*record_beneath_to_wait) (ptid_t, struct target_waitstatus *);
 extern void (*record_beneath_to_store_registers) (struct regcache *, int regno);
 extern LONGEST (*record_beneath_to_xfer_partial) (struct target_ops * ops,
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/remote.c	2008-10-07 16:20:31.000000000 +0100
@@ -91,8 +91,8 @@ static void remote_prepare_to_store (str
 
 static void remote_fetch_registers (struct regcache *regcache, int regno);
 
-static void remote_resume (ptid_t ptid, int step,
-                           enum target_signal siggnal);
+static void remote_resume (ptid_t ptid, enum target_resume_kind how,
+			   enum target_signal siggnal);
 static void remote_open (char *name, int from_tty);
 
 static void extended_remote_open (char *name, int from_tty);
@@ -3256,11 +3256,18 @@ remote_vcont_probe (struct remote_state 
    moment.  */
 
 static int
-remote_vcont_resume (ptid_t ptid, int step, enum target_signal siggnal)
+remote_vcont_resume (ptid_t ptid, enum target_resume_kind how,
+		     enum target_signal siggnal)
 {
   struct remote_state *rs = get_remote_state ();
   char *p;
   char *endp;
+  int step = rk_step;
+
+  /* There are no vCont packets supporting reverse execution
+     (yet).  */
+  if (how == rk_step_reverse || rk_continue_reverse)
+    return 0;
 
   if (remote_protocol_packets[PACKET_vCont].support == PACKET_SUPPORT_UNKNOWN)
     remote_vcont_probe (rs);
@@ -3362,10 +3369,12 @@ static enum target_signal last_sent_sign
 static int last_sent_step;
 
 static void
-remote_resume (ptid_t ptid, int step, enum target_signal siggnal)
+remote_resume (ptid_t ptid, enum target_resume_kind how,
+	       enum target_signal siggnal)
 {
   struct remote_state *rs = get_remote_state ();
   char *buf;
+  int step = (how == rk_step);
 
   last_sent_signal = siggnal;
   last_sent_step = step;
@@ -3374,7 +3383,7 @@ remote_resume (ptid_t ptid, int step, en
   remote_pass_signals ();
 
   /* The vCont packet doesn't need to specify threads via Hc.  */
-  if (remote_vcont_resume (ptid, step, siggnal))
+  if (remote_vcont_resume (ptid, how, siggnal))
     goto done;
 
   /* All other supported resume packets do use Hc, so set the continue
@@ -3385,7 +3394,7 @@ remote_resume (ptid_t ptid, int step, en
     set_continue_thread (ptid);
 
   buf = rs->buf;
-  if (target_get_execution_direction () == EXEC_REVERSE)
+  if (how == rk_step_reverse || how == rk_continue_reverse)
     {
       /* We don't pass signals to the target in reverse exec mode.  */
       if (info_verbose && siggnal != TARGET_SIGNAL_0)
@@ -3616,6 +3625,7 @@ remote_wait (ptid_t ptid, struct target_
   ptid_t event_ptid = null_ptid;
   ULONGEST addr;
   int solibs_changed = 0;
+  int nohistory = 0;
 
   status->kind = TARGET_WAITKIND_EXITED;
   status->value.integer = 0;
@@ -3734,6 +3744,16 @@ Packet: '%s'\n"),
 			solibs_changed = 1;
 			p = p_temp;
 		      }
+		    else if (strncmp (p, "nohistory", p1 - p) == 0)
+		      {
+			p1++;
+			p_temp = p1;
+			while (*p_temp && *p_temp != ';')
+			  p_temp++;
+
+			nohistory = 1;
+			p = p_temp;
+		      }
 		    else
  		      {
  			/* Silently skip unknown optional info.  */
@@ -3779,6 +3799,8 @@ Packet: '%s'\n"),
 	case 'S':		/* Old style status, just signal only.  */
 	  if (solibs_changed)
 	    status->kind = TARGET_WAITKIND_LOADED;
+	  else if (nohistory)
+	    status->kind = TARGET_WAITKIND_NO_HISTORY;
 	  else
 	    {
 	      status->kind = TARGET_WAITKIND_STOPPED;
@@ -7560,35 +7582,20 @@ remote_command (char *args, int from_tty
   help_list (remote_cmdlist, "remote ", -1, gdb_stdout);
 }
 
+static int
+remote_can_reverse_p (void)
+{
+  /* We can either detect that the remote side doesn't support the
+     reverse resume packets, or add a qSupported feature later.  */
+
+  /* Unconditionaly assume we can for now.  */
+  return 1;
+}
+
 /* Reverse execution.
    TODO: set up as a capability.  */
 static enum exec_direction_kind remote_exec_direction = EXEC_FORWARD;
 
-static enum exec_direction_kind remote_get_exec_direction (void)
-{
-  if (remote_debug && info_verbose)
-    printf_filtered ("remote exec direction is %s\n",
-		     remote_exec_direction == EXEC_FORWARD ? _("forward") :
-		     remote_exec_direction == EXEC_REVERSE ? _("reverse") :
-		     "unknown");
-  return remote_exec_direction;
-}
-
-static int remote_set_exec_direction (enum exec_direction_kind dir)
-{
-  if (remote_debug && info_verbose)
-    printf_filtered ("Set remote exec direction: %s\n",
-		     dir == EXEC_FORWARD ? _("forward") :
-		     dir == EXEC_REVERSE ? _("reverse") :
-		     "bad direction");
-
-  /* TODO: check target for capability.  */
-  if (dir == EXEC_FORWARD || dir == EXEC_REVERSE)
-    return (remote_exec_direction = dir);
-  else
-    return EXEC_ERROR;
-}
-
 static void
 init_remote_ops (void)
 {
@@ -7637,8 +7644,6 @@ Specify the serial device it is connecte
   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_get_exec_direction = remote_get_exec_direction;
-  remote_ops.to_set_exec_direction = remote_set_exec_direction;
   remote_ops.to_magic = OPS_MAGIC;
   remote_ops.to_memory_map = remote_memory_map;
   remote_ops.to_flash_erase = remote_flash_erase;
@@ -7651,6 +7656,7 @@ Specify the serial device it is connecte
   remote_ops.to_async_mask = remote_async_mask;
   remote_ops.to_terminal_inferior = remote_terminal_inferior;
   remote_ops.to_terminal_ours = remote_terminal_ours;
+  remote_ops.to_can_reverse_p = remote_can_reverse_p;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
Index: src/gdb/reverse.c
===================================================================
--- src.orig/gdb/reverse.c	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/reverse.c	2008-10-07 16:36:35.000000000 +0100
@@ -25,6 +25,30 @@
 #include "top.h"
 #include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
+#include "inferior.h"
+
+/* The default current (user command level) execution direction.  All
+   targets that support execution should be able to go forward.  */
+
+enum exec_direction_kind current_exec_direction = EXEC_FORWARD;
+
+/* Set the default desired direction.  We don't check for availability
+   here, because the user may set this before the target is
+   connected/open.  If the target doesn't support reverse, either the
+   execution command issued, or target_resume will take care of
+   erroring out.  */
+
+void
+set_execution_direction (enum exec_direction_kind dir)
+{
+  current_exec_direction = dir;
+}
+
+enum exec_direction_kind
+execution_direction (void)
+{
+  return current_exec_direction;
+}
 
 /* User interface for reverse debugging:
    Set exec-direction / show exec-direction commands (returns error 
@@ -43,50 +67,51 @@ static void
 set_exec_direction_func (char *args, int from_tty,
 			 struct cmd_list_element *cmd)
 {
-  if (target_get_execution_direction () != EXEC_ERROR)
-    {
-      enum exec_direction_kind dir = EXEC_ERROR;
+  enum exec_direction_kind dir;
 
-      if (!strcmp (exec_direction, exec_forward))
-	dir = EXEC_FORWARD;
-      else if (!strcmp (exec_direction, exec_reverse))
-	dir = EXEC_REVERSE;
-
-      if (target_set_execution_direction (dir) != EXEC_ERROR)
-	return;
+  if (!strcmp (exec_direction, exec_forward))
+    dir = EXEC_FORWARD;
+  else if (!strcmp (exec_direction, exec_reverse))
+    dir = EXEC_REVERSE;
+  else
+    {
+      internal_error (__FILE__, __LINE__,
+		      "unhandled execution direction");
+      return;
     }
+
+  set_execution_direction (dir);
 }
 
 static void
 show_exec_direction_func (struct ui_file *out, int from_tty,
 			  struct cmd_list_element *cmd, const char *value)
 {
-  enum exec_direction_kind dir = target_get_execution_direction ();
+  enum exec_direction_kind dir = execution_direction ();
 
-  switch (dir) {
-  case EXEC_FORWARD:
-    fprintf_filtered (out, _("Forward\n"));
-    break;
-  case EXEC_REVERSE:
-    fprintf_filtered (out, _("Reverse\n"));
-    break;
-  case EXEC_ERROR:
-  default:
-    fprintf_filtered,  (out, 
-			_("Forward (target `%s' does not support exec-direction)\n"),
-			target_shortname);
-    break;
+  switch (dir)
+    {
+    case EXEC_FORWARD:
+      fprintf_filtered (out, _("Forward\n"));
+      break;
+    case EXEC_REVERSE:
+      fprintf_filtered (out, _("Reverse\n"));
+      break;
+    default:
+      internal_error (__FILE__, __LINE__,
+		      "unhandled execution direction");
+      break;
   }
 }
 
 /* User interface:
-   reverse-step, reverse-next etc. (returns error unles 
-   target implements to_set_exec_direction method).  */
+   reverse-step, reverse-next etc.  */
 
-static void exec_direction_default (void *notused)
+static void
+exec_direction_forward (void *notused)
 {
   /* Return execution direction to default state.  */
-  target_set_execution_direction (EXEC_FORWARD);
+  set_execution_direction (EXEC_FORWARD);
 }
 
 static void
@@ -94,20 +119,16 @@ exec_reverse_once (char *cmd, char *args
 {
   /* String buffer for command consing.  */
   char reverse_command[512];
-  enum exec_direction_kind dir = target_get_execution_direction ();
+  enum exec_direction_kind dir = execution_direction ();
   struct cleanup *old_chain;
 
-  if (dir == EXEC_ERROR)
-    error (_("Target %s does not support this command."), target_shortname);
-
   if (dir == EXEC_REVERSE)
     error (_("Already in reverse mode.  Use '%s' or 'set exec-dir forward'."),
 	   cmd);
 
-  if (target_set_execution_direction (EXEC_REVERSE) == EXEC_ERROR)
-    error (_("Target %s does not support this command."), target_shortname);
+  set_execution_direction (EXEC_REVERSE);
 
-  old_chain = make_cleanup (exec_direction_default, NULL);
+  old_chain = make_cleanup (exec_direction_forward, NULL);
   sprintf (reverse_command, "%s %s", cmd, args ? args : "");
   execute_command (reverse_command, from_tty);
   do_cleanups (old_chain);
Index: src/gdb/target.c
===================================================================
--- src.orig/gdb/target.c	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/target.c	2008-10-07 16:39:33.000000000 +0100
@@ -104,7 +104,8 @@ static void debug_to_attach (char *, int
 
 static void debug_to_detach (char *, int);
 
-static void debug_to_resume (ptid_t, int, enum target_signal);
+static void debug_to_resume (ptid_t, enum target_resume_kind,
+			     enum target_signal);
 
 static ptid_t debug_to_wait (ptid_t, struct target_waitstatus *);
 
@@ -465,14 +466,13 @@ update_current_target (void)
       INHERIT (to_find_memory_regions, t);
       INHERIT (to_make_corefile_notes, t);
       INHERIT (to_get_thread_local_address, t);
-      INHERIT (to_get_exec_direction, t);
-      INHERIT (to_set_exec_direction, t);
       /* Do not inherit to_read_description.  */
       /* Do not inherit to_search_memory.  */
       INHERIT (to_magic, t);
       /* Do not inherit to_memory_map.  */
       /* Do not inherit to_flash_erase.  */
       /* Do not inherit to_flash_done.  */
+      INHERIT (to_can_reverse_p, t);
 
       /* Set the real beneath function pointers. */
       if (t != &record_ops)
@@ -526,7 +526,8 @@ update_current_target (void)
 	    (void (*) (char *, int))
 	    target_ignore);
   de_fault (to_resume,
-	    (void (*) (ptid_t, int, enum target_signal))
+	    (void (*) (ptid_t, enum target_resume_kind,
+		       enum target_signal))
 	    noprocess);
   de_fault (to_wait,
 	    (ptid_t (*) (ptid_t, struct target_waitstatus *))
@@ -661,6 +662,7 @@ update_current_target (void)
   de_fault (to_async_mask,
 	    (int (*) (int))
 	    return_one);
+  de_fault (to_can_reverse_p, return_zero);
   current_target.to_read_description = NULL;
 #undef de_fault
 
@@ -1836,10 +1838,11 @@ target_disconnect (char *args, int from_
 }
 
 void
-target_resume (ptid_t ptid, int step, enum target_signal signal)
+target_resume (ptid_t ptid, enum target_resume_kind how,
+	       enum target_signal signal)
 {
   dcache_invalidate (target_dcache);
-  (*current_target.to_resume) (ptid, step, signal);
+  (*current_target.to_resume) (ptid, how, signal);
   set_executing (ptid, 1);
   set_running (ptid, 1);
 }
@@ -2527,12 +2530,30 @@ debug_to_detach (char *args, int from_tt
 }
 
 static void
-debug_to_resume (ptid_t ptid, int step, enum target_signal siggnal)
+debug_to_resume (ptid_t ptid, enum target_resume_kind how, enum target_signal siggnal)
 {
-  debug_target.to_resume (ptid, step, siggnal);
+  const char *howstr;
 
-  fprintf_unfiltered (gdb_stdlog, "target_resume (%d, %s, %s)\n", PIDGET (ptid),
-		      step ? "step" : "continue",
+  debug_target.to_resume (ptid, how, siggnal);
+
+  switch (how)
+    {
+    case rk_step:
+      howstr = "step";
+      break;
+    case rk_continue:
+      howstr = "continue";
+      break;
+    case rk_step_reverse:
+      howstr = "step reverse";
+      break;
+    case rk_continue_reverse:
+      howstr = "continue reverse";
+      break;
+    }
+
+  fprintf_unfiltered (gdb_stdlog, "target_resume (%s, %s, %s)\n",
+		      target_pid_to_str (ptid), howstr,
 		      target_signal_to_name (siggnal));
 }
 
Index: src/gdb/target.h
===================================================================
--- src.orig/gdb/target.h	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/target.h	2008-10-07 16:20:31.000000000 +0100
@@ -152,6 +152,29 @@ struct target_waitstatus
     value;
   };
 
+/* I'm explicitly leaving the order as
+
+    rk_continue == 0
+    rk_step == 1
+
+    to be compatible with the old interface, so I can avoid fixing up
+    all the target_resume callers for now.  */
+
+enum target_resume_kind
+  {
+    /* Continue forward.  */
+    rk_continue,
+
+    /* Step forward.  */
+    rk_step,
+
+    /* Continue in the reverse direction.  */
+    rk_continue_reverse,
+
+    /* Step in the reverse direction.  */
+    rk_step_reverse,
+  };
+
 /* Reverse execution.  */
 enum exec_direction_kind
   {
@@ -340,7 +363,7 @@ struct target_ops
     void (*to_post_attach) (int);
     void (*to_detach) (char *, int);
     void (*to_disconnect) (struct target_ops *, char *, int);
-    void (*to_resume) (ptid_t, int, enum target_signal);
+    void (*to_resume) (ptid_t, enum target_resume_kind, enum target_signal);
     ptid_t (*to_wait) (ptid_t, struct target_waitstatus *);
     void (*to_fetch_registers) (struct regcache *, int);
     void (*to_store_registers) (struct regcache *, int);
@@ -536,10 +559,8 @@ struct target_ops
 			     const gdb_byte *pattern, ULONGEST pattern_len,
 			     CORE_ADDR *found_addrp);
 
-    /* Set execution direction (forward/reverse).  */
-    int (*to_set_exec_direction) (enum exec_direction_kind);
-    /* Get execution direction (forward/reverse).  */
-    enum exec_direction_kind (*to_get_exec_direction) (void);
+    /* Returns true if the target supports reverse execution.  */
+    int (*to_can_reverse_p) (void);
 
     /* Default value is 0. Mean that this target doesn't support record wait.
        Need the help of infrun.c(handle_inferior_event). Set to 1 if this
@@ -618,12 +639,13 @@ extern void target_detach (char *, int);
 
 extern void target_disconnect (char *, int);
 
-/* Resume execution of the target process PTID.  STEP says whether to
-   single-step or to run free; SIGGNAL is the signal to be given to
-   the target, or TARGET_SIGNAL_0 for no signal.  The caller may not
-   pass TARGET_SIGNAL_DEFAULT.  */
+/* Resume execution of the target process PTID.  HOW says how to
+   proceed; SIGGNAL is the signal to be given to the target, or
+   TARGET_SIGNAL_0 for no signal.  The caller may not pass
+   TARGET_SIGNAL_DEFAULT.  */
 
-extern void target_resume (ptid_t ptid, int step, enum target_signal signal);
+extern void target_resume (ptid_t ptid, enum target_resume_kind how,
+			   enum target_signal signal);
 
 /* 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;
@@ -992,6 +1014,9 @@ extern int target_async_permitted;
 
 int target_supports_non_stop (void);
 
+#define target_can_reverse_p() \
+  (current_target.to_can_reverse_p ())
+
 /* Put the target in async mode with the specified callback function. */
 #define target_async(CALLBACK,CONTEXT) \
      (current_target.to_async ((CALLBACK), (CONTEXT)))
@@ -1150,18 +1175,6 @@ extern int target_stopped_data_address_p
 #define target_watchpoint_addr_within_range(target, addr, start, length) \
   (*target.to_watchpoint_addr_within_range) (target, addr, start, length)
 
-/* Forward/reverse execution direction.
-   These will only be implemented by a target that supports reverse execution.
-*/
-#define target_get_execution_direction() \
-    (current_target.to_get_exec_direction ? \
-     (*current_target.to_get_exec_direction) () : EXEC_ERROR)
-
-#define target_set_execution_direction(DIR) \
-    (current_target.to_set_exec_direction ? \
-     (*current_target.to_set_exec_direction) (DIR) : EXEC_ERROR)
-
-
 extern const struct target_desc *target_read_description (struct target_ops *);
 
 /* Utility implementation of searching memory.  */
Index: src/gdb/fork-child.c
===================================================================
--- src.orig/gdb/fork-child.c	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/fork-child.c	2008-10-07 16:20:31.000000000 +0100
@@ -490,7 +490,7 @@ startup_inferior (int ntraps)
       if (resume_signal != TARGET_SIGNAL_TRAP)
 	{
 	  /* Let shell child handle its own signals in its own way.  */
-	  target_resume (resume_ptid, 0, resume_signal);
+	  target_resume (resume_ptid, rk_continue, resume_signal);
 	}
       else
 	{
@@ -516,7 +516,7 @@ startup_inferior (int ntraps)
 	    break;
 
 	  /* Just make it go on.  */
-	  target_resume (resume_ptid, 0, TARGET_SIGNAL_0);
+	  target_resume (resume_ptid, rk_continue, TARGET_SIGNAL_0);
 	}
     }
 }
Index: src/gdb/i386-linux-nat.c
===================================================================
--- src.orig/gdb/i386-linux-nat.c	2008-10-07 16:13:25.000000000 +0100
+++ src/gdb/i386-linux-nat.c	2008-10-07 16:20:31.000000000 +0100
@@ -744,9 +744,11 @@ static const unsigned char linux_syscall
    If SIGNAL is nonzero, give it that signal.  */
 
 static void
-i386_linux_resume (ptid_t ptid, int step, enum target_signal signal)
+i386_linux_resume (ptid_t ptid, enum target_resume_kind how,
+		   enum target_signal signal)
 {
   int pid = PIDGET (ptid);
+  int step = (how == rk_step);
 
   int request = PTRACE_CONT;
 

Re: [reverse] PATCH: Several interface changes

[teawater]

I think the idea of this patch is good.
But maybe process record still not need it now because p record still
not support multi-thread. Of course, p record will need it in the
feature.

Michael, how about your target?


On Wed, Oct 8, 2008 at 00:09, Pedro Alves <pedro@codesourcery.com> wrote:
> On Monday 06 October 2008 23:11:14, Michael Snyder wrote:
>> Pedro Alves wrote:
>> > A per-target property may seems to make sense on
>> > single-threaded,single-inferior targets, but when you add support
>> > for multi-inferiors per target (e.g., extended-remote has some of it now,
>> > and I'm going to push more of it), or multi-threaded support, the
>> > per-target setting may not make sense anymore --- explicit requests
>> > at the target resume interface (just like your new packets) may make
>> > more sense.  Imagine forward execution non-stop debugging in all threads
>> > but one, which the user is examining in reverse.  What's the target
>> > direction in this case?
>>
>> Yakkk!!!
>
> :-)  Here's an alternative interface I was considering and envisioning
> when I mentioned the above.  Consider this just a suggestion.  If it
> looks bad, let's quickly forget about it.
>
>> > The question to me is --- when/why does the target (as in, the debug
>> > API abstraction) ever need to know about the current direction that
>> > it couldn't get from the core's request?
>
> So, after last night's discussion, I came up with the attached to
> see how it would look like.  The major change is that I consider the
> reverse/forward-direction thing a property or the command the user
> requested, and as such, belongs together with the other thread
> stepping state we keep in struct thread_info, and the
> target_ops implementation, adjusts itself to the direction GDB
> requests with target_resume.  I've extended target_resume's interface
> to accept this instead of a `step' boolean:
>
>  enum target_resume_kind
>   {
>     /* Continue forward.  */
>     rk_continue,
>
>     /* Step forward.  */
>     rk_step,
>
>     /* Continue in the reverse direction.  */
>     rk_continue_reverse,
>
>     /* Step in the reverse direction.  */
>     rk_step_reverse,
>   };
>
> (notice that the order of the things in the enum allows me to
> miss some conversions --- I'm lazy).
>
>  I can't say if I like this new target_resume interface or
> not.  I just tried doing it to see how it would look.
>
> (I can imagine that we're in the future going to extend the
> target_resume interface to be more like gdbserver's, but, well,
> that's another issue.)
>
> So, the interface at the command level implementation is just
> like it was before:
>
>  1)  call clear_proceed_status ();
>
>  2)  /* construct the step/continue request */
>
>  3)  call proceed (...);
>
> Where in #2, you can set the thread to go backwards by
> doing:
>
>     thread->reverse = 1;
>
> The attached patch applies against the reverse-20080930-branch.
>
> Other things I've done in the patch:
>
>   * Added support for a "Tnn nohistory" stop reply that translates
>    to TARGET_WAITKIND_NO_HISTORY.  When going multi-threaded, or
>    multi-process this will allow things like "T05;thread:pPID.TID;nohistory"
>    for free.  I absolutelly didn't test this.  I've no reverse aware target
>    at hand.
>
>   * At places, error out if async + reverse or non-stop + reverse
>     was requested.
>
>   * Added a target_can_reverse_p method, so infcmd.c can check if the
>     target supports reverse execution before calling into the target.  Not
>     strictly necessary, though, but I thought this was nicer this way.
>
> I checked that I can use the record target on x86 (actually x86_64
> with -m32) as good as without the patch, but it's quite possible I
> broke things badly.
>
> --
> Pedro Alves
>

Re: [reverse] PATCH: Several interface changes

[Pedro Alves]

On Wednesday 08 October 2008 08:28:05, teawater wrote:
> I think the idea of this patch is good.

Thanks for taking a look!

> But maybe process record still not need it now because p record still
> not support multi-thread. 
> Of course, p record will need it in the feature.

The basic idea and motivation of the patch was to show that we
don't need a target_set_execution_direction method in the
target ops, or have the target_ops implementation query an infrun
global --- the request to go backwards is a property of
the command the user requested.  Much like a single-step range
property is a per-command/per-thread property.  I believe that
a single per-target_ops setting isn't quite the right abstraction.

If a remote stub doesn't need a special indication that
'all operations from now on apply to reverse debugging', and can
sort itself out from the stateless resume requests, a native
target also shouldn't need it.  If we find that a native target
reverse debugging implementation *does require* a
"set_execution_direction" command to work propertly and can't
get it from the resume interface request, then the remote protocol
may prove itself insuficient.  E.g., if "target record" needs it,
then implementing target record in gdbserver would be impossible
with the current remote protocol --- there's always extensions
and new packets, of course.

As I said, this is just a thought, a proof-of-concept, only
for consideration.  It's *not* meant to hold reverse
debugging from going into mainline.  I'm more worried with
the remote protocol being done right (and I manifested my
opinion about that already), than about this, which
is all internal implementation detail that we can change
at any time.

> 
> Michael, how about your target?
> 
> 
> On Wed, Oct 8, 2008 at 00:09, Pedro Alves <pedro@codesourcery.com> wrote:
> > On Monday 06 October 2008 23:11:14, Michael Snyder wrote:
> >> Pedro Alves wrote:
> >> > A per-target property may seems to make sense on
> >> > single-threaded,single-inferior targets, but when you add support
> >> > for multi-inferiors per target (e.g., extended-remote has some of it now,
> >> > and I'm going to push more of it), or multi-threaded support, the
> >> > per-target setting may not make sense anymore --- explicit requests
> >> > at the target resume interface (just like your new packets) may make
> >> > more sense.  Imagine forward execution non-stop debugging in all threads
> >> > but one, which the user is examining in reverse.  What's the target
> >> > direction in this case?
> >>
> >> Yakkk!!!
> >
> > :-)  Here's an alternative interface I was considering and envisioning
> > when I mentioned the above.  Consider this just a suggestion.  If it
> > looks bad, let's quickly forget about it.
> >
> >> > The question to me is --- when/why does the target (as in, the debug
> >> > API abstraction) ever need to know about the current direction that
> >> > it couldn't get from the core's request?
> >
> > So, after last night's discussion, I came up with the attached to
> > see how it would look like.  The major change is that I consider the
> > reverse/forward-direction thing a property or the command the user
> > requested, and as such, belongs together with the other thread
> > stepping state we keep in struct thread_info, and the
> > target_ops implementation, adjusts itself to the direction GDB
> > requests with target_resume.  I've extended target_resume's interface
> > to accept this instead of a `step' boolean:
> >
> >  enum target_resume_kind
> >   {
> >     /* Continue forward.  */
> >     rk_continue,
> >
> >     /* Step forward.  */
> >     rk_step,
> >
> >     /* Continue in the reverse direction.  */
> >     rk_continue_reverse,
> >
> >     /* Step in the reverse direction.  */
> >     rk_step_reverse,
> >   };
> >
> > (notice that the order of the things in the enum allows me to
> > miss some conversions --- I'm lazy).
> >
> >  I can't say if I like this new target_resume interface or
> > not.  I just tried doing it to see how it would look.
> >
> > (I can imagine that we're in the future going to extend the
> > target_resume interface to be more like gdbserver's, but, well,
> > that's another issue.)
> >
> > So, the interface at the command level implementation is just
> > like it was before:
> >
> >  1)  call clear_proceed_status ();
> >
> >  2)  /* construct the step/continue request */
> >
> >  3)  call proceed (...);
> >
> > Where in #2, you can set the thread to go backwards by
> > doing:
> >
> >     thread->reverse = 1;
> >
> > The attached patch applies against the reverse-20080930-branch.
> >
> > Other things I've done in the patch:
> >
> >   * Added support for a "Tnn nohistory" stop reply that translates
> >    to TARGET_WAITKIND_NO_HISTORY.  When going multi-threaded, or
> >    multi-process this will allow things like "T05;thread:pPID.TID;nohistory"
> >    for free.  I absolutelly didn't test this.  I've no reverse aware target
> >    at hand.
> >
> >   * At places, error out if async + reverse or non-stop + reverse
> >     was requested.
> >
> >   * Added a target_can_reverse_p method, so infcmd.c can check if the
> >     target supports reverse execution before calling into the target.  Not
> >     strictly necessary, though, but I thought this was nicer this way.
> >
> > I checked that I can use the record target on x86 (actually x86_64
> > with -m32) as good as without the patch, but it's quite possible I
> > broke things badly.

-- 
Pedro Alves

Re: [reverse] PATCH: Several interface changes

[Michael Snyder]

teawater wrote:
> I think the idea of this patch is good.
> But maybe process record still not need it now because p record still
> not support multi-thread. Of course, p record will need it in the
> feature.
> 
> Michael, how about your target?

No multi-thread support, yet.

Pedro, future enhancement?

> On Wed, Oct 8, 2008 at 00:09, Pedro Alves <pedro@codesourcery.com> wrote:
>> On Monday 06 October 2008 23:11:14, Michael Snyder wrote:
>>> Pedro Alves wrote:
>>>> A per-target property may seems to make sense on
>>>> single-threaded,single-inferior targets, but when you add support
>>>> for multi-inferiors per target (e.g., extended-remote has some of it now,
>>>> and I'm going to push more of it), or multi-threaded support, the
>>>> per-target setting may not make sense anymore --- explicit requests
>>>> at the target resume interface (just like your new packets) may make
>>>> more sense.  Imagine forward execution non-stop debugging in all threads
>>>> but one, which the user is examining in reverse.  What's the target
>>>> direction in this case?
>>> Yakkk!!!
>> :-)  Here's an alternative interface I was considering and envisioning
>> when I mentioned the above.  Consider this just a suggestion.  If it
>> looks bad, let's quickly forget about it.
>>
>>>> The question to me is --- when/why does the target (as in, the debug
>>>> API abstraction) ever need to know about the current direction that
>>>> it couldn't get from the core's request?
>> So, after last night's discussion, I came up with the attached to
>> see how it would look like.  The major change is that I consider the
>> reverse/forward-direction thing a property or the command the user
>> requested, and as such, belongs together with the other thread
>> stepping state we keep in struct thread_info, and the
>> target_ops implementation, adjusts itself to the direction GDB
>> requests with target_resume.  I've extended target_resume's interface
>> to accept this instead of a `step' boolean:
>>
>>  enum target_resume_kind
>>   {
>>     /* Continue forward.  */
>>     rk_continue,
>>
>>     /* Step forward.  */
>>     rk_step,
>>
>>     /* Continue in the reverse direction.  */
>>     rk_continue_reverse,
>>
>>     /* Step in the reverse direction.  */
>>     rk_step_reverse,
>>   };
>>
>> (notice that the order of the things in the enum allows me to
>> miss some conversions --- I'm lazy).
>>
>>  I can't say if I like this new target_resume interface or
>> not.  I just tried doing it to see how it would look.
>>
>> (I can imagine that we're in the future going to extend the
>> target_resume interface to be more like gdbserver's, but, well,
>> that's another issue.)
>>
>> So, the interface at the command level implementation is just
>> like it was before:
>>
>>  1)  call clear_proceed_status ();
>>
>>  2)  /* construct the step/continue request */
>>
>>  3)  call proceed (...);
>>
>> Where in #2, you can set the thread to go backwards by
>> doing:
>>
>>     thread->reverse = 1;
>>
>> The attached patch applies against the reverse-20080930-branch.
>>
>> Other things I've done in the patch:
>>
>>   * Added support for a "Tnn nohistory" stop reply that translates
>>    to TARGET_WAITKIND_NO_HISTORY.  When going multi-threaded, or
>>    multi-process this will allow things like "T05;thread:pPID.TID;nohistory"
>>    for free.  I absolutelly didn't test this.  I've no reverse aware target
>>    at hand.
>>
>>   * At places, error out if async + reverse or non-stop + reverse
>>     was requested.
>>
>>   * Added a target_can_reverse_p method, so infcmd.c can check if the
>>     target supports reverse execution before calling into the target.  Not
>>     strictly necessary, though, but I thought this was nicer this way.
>>
>> I checked that I can use the record target on x86 (actually x86_64
>> with -m32) as good as without the patch, but it's quite possible I
>> broke things badly.
>>
>> --
>> Pedro Alves
>>

Re: [reverse] PATCH: Several interface changes

[Pedro Alves]

On Wednesday 08 October 2008 21:19:00, Michael Snyder wrote:

> Pedro, future enhancement?

Yes, sure.

(Please do take a look at the remote protocol change I was making.)

-- 
Pedro Alves

Re: [reverse] PATCH: Several interface changes

[teawater]

I think about it again in this morning.

Maybe P record doesn't need make threads execute in different
direction. Cause P record reverse executes base on replay mode. This
mode will replay the memory change and the registers change. And all
threads of one process share the memory. So...

Of course, I am not clear the other reverse implement. Maybe they will need it.

Thanks,
Hui

On Thu, Oct 9, 2008 at 07:27, Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 08 October 2008 21:19:00, Michael Snyder wrote:
>
>> Pedro, future enhancement?
>
> Yes, sure.
>
> (Please do take a look at the remote protocol change I was making.)
>
> --
> Pedro Alves
>

Re: [reverse] PATCH: Several interface changes

[Pedro Alves]

On Thursday 09 October 2008 03:31:47, teawater wrote:
> Maybe P record doesn't need make threads execute in different
> direction. Cause P record reverse executes base on replay mode. This
> mode will replay the memory change and the registers change. And all
> threads of one process share the memory. So...

You're thinking single-inferior.  What about threads of different inferiors
behind a single target_ops?  Say, you're attached to process A, but you're
leaving it running (you'll hit internal breakpoints in forward mode),
while you're debugging/inspecting process B in reverse.  There you have
your two threads, on a single target, where the single per-target
direction flag stops making sense.

I understand the idealism behind this.  I just posted the patch
to show the direction I think we will end up taking, instead of
trying to explain it by: "it would be nice if you did it the way
I'm saying".

-- 
Pedro Alves

Re: [reverse] PATCH: Several interface changes

[teawater]

On Thu, Oct 9, 2008 at 10:49, Pedro Alves <pedro@codesourcery.com> wrote:
> On Thursday 09 October 2008 03:31:47, teawater wrote:
>> Maybe P record doesn't need make threads execute in different
>> direction. Cause P record reverse executes base on replay mode. This
>> mode will replay the memory change and the registers change. And all
>> threads of one process share the memory. So...
>
> You're thinking single-inferior.  What about threads of different inferiors
> behind a single target_ops?  Say, you're attached to process A, but you're
> leaving it running (you'll hit internal breakpoints in forward mode),
> while you're debugging/inspecting process B in reverse.  There you have
> your two threads, on a single target, where the single per-target
> direction flag stops making sense.

If there are multi-inferior use different memory, it must need a flag in resume.
Sorry I am not make it clear in before.

>
> I understand the idealism behind this.  I just posted the patch
> to show the direction I think we will end up taking, instead of
> trying to explain it by: "it would be nice if you did it the way
> I'm saying".

I always think GDB need it. :)

Thanks,
Hui