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

["Metzger, Markus T" <markus.t.metzger@intel.com>]

> -----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.

Record full needs to single-step in order to restore the memory and
register contents.  But for record btrace, this would be completely
artificial.  I don't think we should do it this way.  Could we maybe polish
my solution so it is more in line with the rest of GDB?


Regards,
Markus.

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052