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

[Hui Zhu <teawater at gmail dot com>]

On Mon, Jun 15, 2009 at 08:55, Michael Snyder<msnyder@vmware.com> wrote:
> 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?
>

This patch is OK with me.
Marc, what do you think about it?

Thanks,
Hui


>
>
>> 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
>>>>>
>>
>>
>
>
> 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",
>
>