* [RFA] Patch to fix reverse-debug recursion function tail bug
@ 2009-03-21 9:17 Hui Zhu
2009-05-06 7:24 ` Hui Zhu
0 siblings, 1 reply; 9+ messages in thread
From: Hui Zhu @ 2009-03-21 9:17 UTC (permalink / raw)
To: gdb-patches; +Cc: Michael Snyder, Marc Khouzam
[-- Attachment #1: Type: text/plain, Size: 2521 bytes --]
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: fix-function-tail-stack-same.txt --]
[-- Type: text/plain, Size: 695 bytes --]
---
infrun.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/infrun.c
+++ b/infrun.c
@@ -3397,7 +3397,10 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
beyond the step range, and NOT the address of the last instruction
within it! */
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
+ && (frame_id_eq (get_frame_id (get_current_frame ()),
+ ecs->event_thread->step_frame_id)
+ || execution_direction != EXEC_REVERSE))
{
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: stepping inside range [0x%s-0x%s]\n",
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [RFA] Patch to fix reverse-debug recursion function tail bug 2009-03-21 9:17 [RFA] Patch to fix reverse-debug recursion function tail bug Hui Zhu @ 2009-05-06 7:24 ` Hui Zhu 2009-05-11 7:07 ` Hui Zhu 0 siblings, 1 reply; 9+ messages in thread From: Hui Zhu @ 2009-05-06 7:24 UTC (permalink / raw) To: Michael Snyder; +Cc: Marc Khouzam, gdb-patches [-- Attachment #1: Type: text/plain, Size: 3252 bytes --] 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: fix-function-tail-stack-same.txt --] [-- Type: text/plain, Size: 695 bytes --] --- infrun.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) --- a/infrun.c +++ b/infrun.c @@ -3421,7 +3421,10 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME ( beyond the step range, and NOT the address of the last instruction within it! */ 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 + && (frame_id_eq (get_frame_id (get_current_frame ()), + ecs->event_thread->step_frame_id) + || execution_direction != EXEC_REVERSE)) { if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: stepping inside range [0x%s-0x%s]\n", ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] Patch to fix reverse-debug recursion function tail bug 2009-05-06 7:24 ` Hui Zhu @ 2009-05-11 7:07 ` Hui Zhu 2009-06-09 2:18 ` Hui Zhu 0 siblings, 1 reply; 9+ messages in thread From: Hui Zhu @ 2009-05-11 7:07 UTC (permalink / raw) To: Michael Snyder; +Cc: Marc Khouzam, gdb-patches 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 >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] Patch to fix reverse-debug recursion function tail bug 2009-05-11 7:07 ` Hui Zhu @ 2009-06-09 2:18 ` Hui Zhu 2009-06-15 0:55 ` Michael Snyder 0 siblings, 1 reply; 9+ messages in thread From: Hui Zhu @ 2009-06-09 2:18 UTC (permalink / raw) To: Michael Snyder; +Cc: Marc Khouzam, gdb-patches PING 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 >>> >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] Patch to fix reverse-debug recursion function tail bug 2009-06-09 2:18 ` Hui Zhu @ 2009-06-15 0:55 ` Michael Snyder 2009-06-15 3:37 ` Hui Zhu 0 siblings, 1 reply; 9+ messages in thread From: Michael Snyder @ 2009-06-15 0:55 UTC (permalink / raw) To: Hui Zhu; +Cc: Marc Khouzam, gdb-patches [-- 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", ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] Patch to fix reverse-debug recursion function tail bug 2009-06-15 0:55 ` Michael Snyder @ 2009-06-15 3:37 ` Hui Zhu 2009-06-15 15:06 ` Marc Khouzam 0 siblings, 1 reply; 9+ messages in thread From: Hui Zhu @ 2009-06-15 3:37 UTC (permalink / raw) To: Michael Snyder, Marc Khouzam; +Cc: gdb-patches 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", > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [RFA] Patch to fix reverse-debug recursion function tail bug 2009-06-15 3:37 ` Hui Zhu @ 2009-06-15 15:06 ` Marc Khouzam 2009-06-15 18:03 ` Michael Snyder 0 siblings, 1 reply; 9+ messages in thread From: Marc Khouzam @ 2009-06-15 15:06 UTC (permalink / raw) To: Hui Zhu, Michael Snyder; +Cc: gdb-patches > -----Original Message----- > From: Hui Zhu [mailto:teawater@gmail.com] > Sent: June-14-09 11:37 PM > To: Michael Snyder; Marc Khouzam > Cc: gdb-patches@sourceware.org > Subject: Re: [RFA] Patch to fix reverse-debug recursion > function tail bug > > 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? I tested before and after the patch and it does fix the problem for me. Thanks! > > 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", > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] Patch to fix reverse-debug recursion function tail bug 2009-06-15 15:06 ` Marc Khouzam @ 2009-06-15 18:03 ` Michael Snyder 2009-06-18 23:56 ` Michael Snyder 0 siblings, 1 reply; 9+ messages in thread From: Michael Snyder @ 2009-06-15 18:03 UTC (permalink / raw) To: Marc Khouzam; +Cc: Hui Zhu, gdb-patches Marc Khouzam wrote: >> -----Original Message----- >> From: Hui Zhu [mailto:teawater@gmail.com] >> Sent: June-14-09 11:37 PM >> To: Michael Snyder; Marc Khouzam >> Cc: gdb-patches@sourceware.org >> Subject: Re: [RFA] Patch to fix reverse-debug recursion >> function tail bug >> >> 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? > > I tested before and after the patch and it does fix > the problem for me. Great, so sounds like this one can go in. Michael ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] Patch to fix reverse-debug recursion function tail bug 2009-06-15 18:03 ` Michael Snyder @ 2009-06-18 23:56 ` Michael Snyder 0 siblings, 0 replies; 9+ messages in thread From: Michael Snyder @ 2009-06-18 23:56 UTC (permalink / raw) To: Michael Snyder; +Cc: Marc Khouzam, Hui Zhu, gdb-patches Michael Snyder wrote: > Marc Khouzam wrote: >>> -----Original Message----- >>> From: Hui Zhu [mailto:teawater@gmail.com] >>> Sent: June-14-09 11:37 PM >>> To: Michael Snyder; Marc Khouzam >>> Cc: gdb-patches@sourceware.org >>> Subject: Re: [RFA] Patch to fix reverse-debug recursion >>> function tail bug >>> >>> 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? >> I tested before and after the patch and it does fix >> the problem for me. > > Great, so sounds like this one can go in. ... and committed. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-06-18 23:56 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-03-21 9:17 [RFA] Patch to fix reverse-debug recursion function tail bug Hui Zhu 2009-05-06 7:24 ` Hui Zhu 2009-05-11 7:07 ` Hui Zhu 2009-06-09 2:18 ` Hui Zhu 2009-06-15 0:55 ` Michael Snyder 2009-06-15 3:37 ` Hui Zhu 2009-06-15 15:06 ` Marc Khouzam 2009-06-15 18:03 ` Michael Snyder 2009-06-18 23:56 ` Michael Snyder
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox