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

Pedro Alves
2013-12-17T20:41:16Z

On 12/17/2013 03:47 PM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: Metzger, Markus T
>> Sent: Tuesday, December 17, 2013 4:06 PM
>> To: Pedro Alves
>
>
>> 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.
>
> The s/w record target also does not execute the breakpoint instruction.
> It increments the PC by gdbarch_decr_pc_after_break () when it finds a
> s/w breakpoint at the current location.
>
> I wonder why they are doing that. Is this a workaround that was considered
> less intrusive than what I am proposing below?

It just seemed better at the time. When it was originally submitted
the precord target was a lot more invasive in the core side, and
I pushed for redoing it as a target (new stratum), and make it
as transparent as possible. The current decr_pc_after_break
handling was a result of that mindset. I'm not seeing a reason
your suggested direction couldn't work for the s/w record target
too.

--
Pedro Alves