Re: [+rfc] Re: [patch v6 00/21] record-btrace: reverse

[Jan Kratochvil <jan.kratochvil@redhat.com>]

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

On Thu, 07 Nov 2013 16:41:40 +0100, Metzger, Markus T wrote:
> I hacked a first prototype of this (see below).  It passes most tests but
> results in three fails in the record_goto suite.
> 
> One thing that it shows, though, is that it only removes the 'mostly harmless'
> hack in the various goto functions shown above.
> 
> The more serious hacks in record_btrace_start_replaying
> 
> 	  /* Make sure we're not using any stale registers.  */
> 	  registers_changed_ptid (tp->ptid);
> 
> 	  /* We just started replaying.  The frame id cached for stepping is based
> 	     on unwinding, not on branch tracing.  Recompute it.  */
> 	  frame = get_current_frame_nocheck ();
> 	  insn = btrace_insn_get (replay);
> 	  sal = find_pc_line (insn->pc, 0);
> 	  set_step_info (frame, sal);
> 
> and record_btrace_stop_replaying
> 
> 	  /* Make sure we're not leaving any stale registers.  */
> 	  registers_changed_ptid (tp->ptid);
> 
> however, are not removed by this.

In such case it is not finished.  These hacks should not be needed.


> They are required when reverse-stepping the first time or when
> stepping past the end of the execution trace.

I have patched what you describe as the problem.  But as I do not have a box
with reliably working BTS so it is difficult for me to say whether it works or
not.  I can look at other problems if you describe them from a reliable box.


> Plus the patch has the potential of messing things up pretty badly if
> somehow (maybe due to some unexpected error () somewhere in proceed ()) the
> record goto command does not complete and BTHR_GOTO remains set.

It can be cleaned up as I did there, thanks for catching it.


Thanks,
Jan

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

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index c50e11b..9fbff15 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -1300,16 +1300,6 @@ record_btrace_start_replaying (struct thread_info *tp)
   gdb_assert (btinfo->replay == NULL);
   btinfo->replay = replay;
 
-  /* Make sure we're not using any stale registers.  */
-  registers_changed_ptid (tp->ptid);
-
-  /* We just started replaying.  The frame id cached for stepping is based
-     on unwinding, not on branch tracing.  Recompute it.  */
-  frame = get_current_frame_nocheck ();
-  insn = btrace_insn_get (replay);
-  sal = find_pc_line (insn->pc, 0);
-  set_step_info (frame, sal);
-
   return replay;
 }
 
@@ -1324,9 +1314,6 @@ record_btrace_stop_replaying (struct thread_info *tp)
 
   xfree (btinfo->replay);
   btinfo->replay = NULL;
-
-  /* Make sure we're not leaving any stale registers.  */
-  registers_changed_ptid (tp->ptid);
 }
 
 /* The to_resume method of target record-btrace.  */
@@ -1619,10 +1606,6 @@ record_btrace_wait (struct target_ops *ops, ptid_t ptid,
   /* Start record histories anew from the current position.  */
   record_btrace_clear_histories (&tp->btrace);
 
-  /* GDB seems to need this.  Without, a stale PC seems to be used resulting in
-     the current location to be displayed incorrectly.  */
-  registers_changed_ptid (tp->ptid);
-
   return tp->ptid;
 }
 
@@ -1662,6 +1645,8 @@ record_btrace_goto_target (struct thread_info *tp,
   struct btrace_insn_iterator *goto_target;
   struct btrace_thread_info *btinfo;
   struct target_waitstatus ws;
+  struct btrace_insn_iterator target_it;
+  volatile struct gdb_exception exception;
 
   btinfo = &tp->btrace;
 
@@ -1686,11 +1671,17 @@ record_btrace_goto_target (struct thread_info *tp,
   btinfo->flags |= BTHR_GOTO;
   btinfo->goto_target = goto_target;
 
-#if 0
+  TRY_CATCH (exception, RETURN_MASK_ALL)
+    {
+
+#if 1
   if (goto_target != NULL)
+    target_it = *goto_target;
+  else
+    btrace_insn_end (&target_it, btinfo);
     tp->control.exception_resume_breakpoint =
       set_momentary_breakpoint_at_pc (target_gdbarch (),
-				      btrace_insn_get (goto_target)->pc,
+				      btrace_insn_get (&target_it)->pc,
 				      bp_until);
 #endif
 #if 0
@@ -1701,8 +1692,18 @@ record_btrace_goto_target (struct thread_info *tp,
   proceed ((CORE_ADDR) -1, GDB_SIGNAL_0, 0);
 #endif
 
-  if (btinfo->goto_target != NULL || (btinfo->flags & BTHR_GOTO) != 0)
+  // It will need a fix if reverse mode supports target-async mode.
+  if ((btinfo->flags & BTHR_GOTO) != 0)
     error (_("Record goto failed."));
+  gdb_assert (btinfo->goto_target == NULL);
+
+    }
+  if (exception.reason < 0)
+    {
+      xfree (btinfo->goto_target);
+      btinfo->goto_target = NULL;
+      btinfo->flags &= ~BTHR_GOTO;
+    }
 }
 
 /* The to_goto_record_begin method of target record-btrace.  */