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

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

> -----Original Message-----
> From: Metzger, Markus T
> Sent: Tuesday, December 17, 2013 3:15 PM
> To: Pedro Alves


> > >>> +	  if (breakpoint_here_p (aspace, insn->pc))
> > >>> +	    return btrace_step_stopped ();
> > >>
> > >> How is adjust_pc_after_break handled?
> > >
> > > I'm not doing anything special.  The PC register is writable,
> > > so the PC can be adjusted.
> >
> > Eh, it's writable even when forward executing through history?
> >
> > I see this in patch 14:
> >
> > > +static void
> > > +record_btrace_store_registers (struct target_ops *ops,
> > > +			       struct regcache *regcache, int regno)
> > > +{
> > > +  struct target_ops *t;
> > > +
> > > +  if (record_btrace_is_replaying ())
> > > +    error (_("This record target does not allow writing registers."));
> > > +
> >
> > Was it changed in later patches?
> 
> Oops.  You are right.  It is read-only.
> 
> After some playing around, I also managed to trigger the error
> when adjust_pc_after_break tries to write the adjusted PC.
> 
> I'm inclined to simply allow it.  The adjusted PC will be ignored for
> stepping, of course.  We just follow the recorded execution.
> 
> Concerns?

Besides RIP, we also write orig_rax, so we still fail.

I'm beginning to wonder if we should actually adjust the PC for
btrace replay.  The way I interpret the function, we got a SIGTRAP
for executing the breakpoint instruction.  The PC is after the breakpoint
instruction, so it is one byte after the actual breakpoint location.  We're
now trying to undo the execution of the breakpoint instruction.
Is this correct?

In that case, we shouldn't adjust the PC since we did not execute
the breakpoint instruction in the trace.  We just asked whether there
is a breakpoint at the current location.

This seems to be different for s/w record.  This is the first time, I need
to distinguish between the different record targets.

I would add a new target method to_omit_pc_after_break_adjustment
that would default to false if not present.  The btrace record target will
provide it and return true if we're replaying.

OK with that?


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