This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Patch to fix reverse return from subroutine error
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.