Re: [RFA] Patch to fix reverse return from subroutine error

[Michael Snyder <msnyder@vmware.com>]

Pedro Alves wrote:
>> -         || execution_direction == EXEC_REVERSE))
>> +         || (execution_direction == EXEC_REVERSE
>> +             && ecs->event_thread->step_frame_id.stack_addr_p
>> +             && get_frame_id (get_current_frame ()).stack_addr_p
>> +              && !gdbarch_inner_than (current_gdbarch,
>> +                                     ecs->event_thread->step_frame_id.stack_addr,
>> +                                     get_frame_id
> 
> Sorry to pitch in so late, but this doesn't look right to me.
> Common code shouldn't be accessing frame id members directly, frame ids
> are supposed to be opaque.  What is this trying to do?

It's trying to answer the question "have we stepped into a
subroutine call?", in reverse.  This unfortunately involves
corner cases that we don't see when we're going forward.

Originally the code just looked (approximately) like this:

   /* Check for subroutine calls.  The check for the current frame
      equalling the step ID is not necessary - the check of the
      previous frame's ID is sufficient - but it is a common case and
      cheaper than checking the previous frame's ID.  */

   if (!frame_id_eq (get_frame_id (frame), step_frame_id)
       && frame_id_eq (frame_unwind_id (frame), step_frame_id))

The problem is that the second "frame_id_eq" test fails in
the case where we've just stepped backward to the RET instruction
of a function which, in forward-time, had just returned.

It's possible that what we're trying to do here is work around a
bug in the i386 implementation of frame_unwind_id.  When I look at
the frame_id that it returns at this point, it does not match either
the caller or the callee, and its code_addr is particularly wrong.

We don't encounter this situation in forward execution, because
it is caught earler by the stepping-within-line-range code, and
we never reach this test on the RET instruction.