This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [+rfc] Re: [patch v6 00/21] record-btrace: reverse
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: "Metzger, Markus T" <markus dot t dot metzger at intel dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Wed, 27 Nov 2013 19:57:27 +0100
- Subject: Re: [+rfc] Re: [patch v6 00/21] record-btrace: reverse
- Authentication-results: sourceware.org; auth=none
- References: <1379676639-31802-1-git-send-email-markus dot t dot metzger at intel dot com> <20131006195913 dot GA2518 at host2 dot jankratochvil dot net> <A78C989F6D9628469189715575E55B230AA127B9 at IRSMSX104 dot ger dot corp dot intel dot com>
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
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. */