Re: [patch v4 23/24] record-btrace: add (reverse-)stepping support

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

On Mon, 30 Sep 2013 11:30:14 +0200, Metzger, Markus T wrote:
> > -----Original Message-----
> > From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> > owner@sourceware.org] On Behalf Of Jan Kratochvil
> 
> 
> > > But this code compares a NORMAL_FRAME from before the step with a
> > > BTRACE_FRAME from after the wait.  They will always be unequal hence
> > > we will never recognize that we just reverse-stepped into a function.
> > >
> > > When I reset the frame cache, GDB re-computes the stored frame and
> > now
> > > compares two BTRACE_FRAMEs, which works OK.
> > [...]
> > > See above.  Alternatively, I might add a special case to frame comparison,
> > > but this would be quite ugly, as well.  Do you have a better idea?
> > 
> > +record_btrace_start_replaying (struct thread_info *tp)
> > [...]
> > +  /* 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);
> > 
> > The problem comes from the new commands like "record goto" which
> > change
> > inferior content without resuming+stopping it.
> > 
> > Former "record full" could only change history position by "step/reverse-
> > step"
> > (or similar commands) which did resume+stop the inferior.
> > 
> > To make the "record goto" command friendly to the GDB infrastructure
> > expectations I think you should put a temporary breakpoint to the target
> > instruction, resume the inferior and simulate stop at the temporary
> > breakpoint.
> > 
> > I think all the registers_changed_ptid() calls could be removed afterwards.
> 
> That would cause quite some overhead if we're moving by a big number
> of instructions.
> 
> First, we'd single-step instead of just setting the PC.  Second, I'd need to
> examine all instruction addresses on the way in order to compute the ignore
> count of that temporary breakpoint.

I did not mean single-stepping.  Just do the single to_resume + to_wait where
to_wait will return the new PC.  Unfortunately one has to create a temporary
breakpoint otherwise GDB will print unexpected SIGTRAP but many commands (like
"next" over a function call) create temporary breakpoints.

This way all the actions in current proceed(), handle_inferior_event() etc.
get executed.


Jan