Re: [RFA] Patch to fix reverse-debug recursion function tail bug

Michael Snyder
2009-06-15T00:55:10Z

[-- Attachment #1: Type: text/plain, Size: 3958 bytes --]

Hui Zhu wrote:
> PING

Thanks for the reminder.

I added some comment and changed the order of evaluation a bit,
hoping to reduce the performance impact on normal debugging.
And I ran the testsuites, before and after.

Modified patch is attached -- is this OK with you guys?
Mark, can you confirm that it fixes your original bug?

Thx again,
Michael

> On Mon, May 11, 2009 at 15:07, Hui Zhu<teawater@gmail.com> wrote:
>> PING
>>
>> On Wed, May 6, 2009 at 15:23, Hui Zhu <teawater@gmail.com> wrote:
>>> Hi Michael,
>>>
>>> Like the prev patch I send to you, this issue still affect cvs-head
>>> and the patch can fix it.
>>> Please help me review it.
>>>
>>> The attachment is the new patch follow cvs-head.
>>>
>>> 2009-05-06 Hui Zhu <teawater@gmail.com>
>>>
>>> * infrun.c (handle_inferior_event): Check frame_id when
>>> check range in reverse debug mode.
>>>
>>> Thanks,
>>> Hui
>>>
>>> On Sat, Mar 21, 2009 at 16:52, Hui Zhu <teawater@gmail.com> wrote:
>>>> Hi,
>>>>
>>>> This patch is for bug report by Marc in
>>>> http://sourceware.org/ml/gdb/2009-03/msg00127.html.
>>>>
>>>> This bug in "handle_inferior_event" deal with recursion function tail
>>>> in reverse debug.
>>>> infrun: infwait_normal_state
>>>> infrun: TARGET_WAITKIND_STOPPED
>>>> infrun: stop_pc = 0x8048457
>>>> infrun: stepping inside range [0x8048457-0x804845a]
>>>> infrun: stop_stepping
>>>> factorial (x=4) at b.cc:5
>>>>
>>>> Inferior already step into another frame. But because this is a
>>>> recursion function call, And 0x8048457 is in
>>>> ecs->event_thread->step_range_start and
>>>> ecs->event_thread->step_range_start.
>>>>
>>>> So gdb run in:
>>>>
>>>> if (stop_pc >= ecs->event_thread->step_range_start
>>>> && stop_pc < ecs->event_thread->step_range_end)
>>>> {
>>>>
>>>> This code is in front of:
>>>> if (!frame_id_eq (get_frame_id (get_current_frame ()),
>>>> ecs->event_thread->step_frame_id)
>>>> && (frame_id_eq (frame_unwind_id (get_current_frame ()),
>>>> ecs->event_thread->step_frame_id)
>>>> || execution_direction == EXEC_REVERSE))
>>>>
>>>> So gdb check range without check frame_id.
>>>>
>>>> So I make a patch to check frame_id when check range in reverse debug mode.
>>>>
>>>> 2008-03-21 Hui Zhu <teawater@gmail.com>
>>>>
>>>> * infrun.c (handle_inferior_event): Check frame_id when
>>>> check range in reverse debug mode.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> Actually, there is another thing, when gdb begin reverse-debug, it's range is:
>>>> 8048439: 8b 45 08 mov 0x8(%ebp),%eax
>>>> 804843c: 83 e8 01 sub $0x1,%eax
>>>> 804843f: 89 04 24 mov %eax,(%esp)
>>>> 8048442: e8 dd ff ff ff call 8048424 <_Z9factoriali>
>>>> 8048447: 0f af 45 08 imul 0x8(%ebp),%eax
>>>> 804844b: 89 45 fc mov %eax,-0x4(%ebp)
>>>> Why is changed to infrun: stepping inside range [0x8048457-0x804845a]?
>>>> That is because when inferior step at:
>>>> 8048458: c3 ret
>>>> In this address, $ebp is same with high level function and this
>>>> function is factorial too.
>>>> So the gdb can't found inferior step into another frame. It will run to:
>>>> ecs->event_thread->step_range_start = stop_pc_sal.pc;
>>>> ecs->event_thread->step_range_end = stop_pc_sal.end;
>>>> ecs->event_thread->step_frame_id = get_frame_id (get_current_frame ());
>>>> ecs->event_thread->current_line = stop_pc_sal.line;
>>>> ecs->event_thread->current_symtab = stop_pc_sal.symtab;
>>>>
>>>> if (debug_infrun)
>>>> fprintf_unfiltered (gdb_stdlog, "infrun: keep going\n");
>>>> keep_going (ecs);
>>>> }
>>>> So ecs->event_thread->step_range_start and ecs->event_thread->step_range_end.
>>>>
>>>> I don't find that it affect the reverse debug or something. So I didn't fix it.
>>>>
>>>>
>>>>
>>>>
>>>> Thanks,
>>>> Hui
>>>>
>
>

[-- Attachment #2: tail.txt --]
[-- Type: text/plain, Size: 1303 bytes --]

2009-06-14 Hui Zhu <teawater@gmail.com>
Michael Snyder <msnyder@vmware.com>

* infrun.c (handle_inferior_event): Improve reverse stepping
through function epilogue.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.387
diff -u -p -r1.387 infrun.c
--- infrun.c 11 Jun 2009 11:57:46 -0000 1.387
+++ infrun.c 15 Jun 2009 00:45:17 -0000
@@ -3623,9 +3623,17 @@ infrun: not switching back to stepped th

Note that step_range_end is the address of the first instruction
beyond the step range, and NOT the address of the last instruction
- within it! */
+ within it!
+
+ Note also that during reverse execution, we may be stepping
+ through a function epilogue and therefore must detect when
+ the current-frame changes in the middle of a line. */
+
if (stop_pc >= ecs->event_thread->step_range_start
- && stop_pc < ecs->event_thread->step_range_end)
+ && stop_pc < ecs->event_thread->step_range_end
+ && (execution_direction != EXEC_REVERSE
+ || frame_id_eq (get_frame_id (get_current_frame ()),
+ ecs->event_thread->step_frame_id)))
{
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: stepping inside range [0x%s-0x%s]\n",