From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5837 invoked by alias); 15 Jun 2009 03:37:07 -0000 Received: (qmail 5828 invoked by uid 22791); 15 Jun 2009 03:37:06 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,SARE_MSGID_LONG40,SPF_PASS,WEIRD_PORT X-Spam-Check-By: sourceware.org Received: from mail-px0-f202.google.com (HELO mail-px0-f202.google.com) (209.85.216.202) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 15 Jun 2009 03:37:00 +0000 Received: by pxi40 with SMTP id 40so271609pxi.12 for ; Sun, 14 Jun 2009 20:36:58 -0700 (PDT) MIME-Version: 1.0 Received: by 10.142.254.2 with SMTP id b2mr2728191wfi.323.1245037018244; Sun, 14 Jun 2009 20:36:58 -0700 (PDT) In-Reply-To: <4A359C0C.9090508@vmware.com> References: <4A359C0C.9090508@vmware.com> Date: Mon, 15 Jun 2009 03:37:00 -0000 Message-ID: Subject: Re: [RFA] Patch to fix reverse-debug recursion function tail bug From: Hui Zhu To: Michael Snyder , Marc Khouzam Cc: "gdb-patches@sourceware.org" Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-06/txt/msg00377.txt.bz2 On Mon, Jun 15, 2009 at 08:55, Michael Snyder 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 wrote: >>> >>> PING >>> >>> On Wed, May 6, 2009 at 15:23, Hui Zhu 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 =A0Hui Zhu =A0 >>>> >>>> =A0 =A0 =A0* infrun.c (handle_inferior_event): Check frame_id when >>>> =A0 =A0 =A0check range in reverse debug mode. >>>> >>>> Thanks, >>>> Hui >>>> >>>> On Sat, Mar 21, 2009 at 16:52, Hui Zhu 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 =3D 0x8048457 >>>>> infrun: stepping inside range [0x8048457-0x804845a] >>>>> infrun: stop_stepping >>>>> factorial (x=3D4) 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 >=3D ecs->event_thread->step_range_start >>>>> =A0 =A0 && stop_pc < ecs->event_thread->step_range_end) >>>>> =A0 { >>>>> >>>>> This code is in front of: >>>>> =A0if (!frame_id_eq (get_frame_id (get_current_frame ()), >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ecs->event_thread->step_frame_id) >>>>> =A0 =A0 && (frame_id_eq (frame_unwind_id (get_current_frame ()), >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ecs->event_thread->step_fr= ame_id) >>>>> =A0 =A0 =A0 =A0 || execution_direction =3D=3D 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 =A0Hui Zhu =A0 >>>>> >>>>> =A0 =A0 =A0 * infrun.c (handle_inferior_event): Check frame_id when >>>>> =A0 =A0 =A0 check range in reverse debug mode. >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> Actually, there is another thing, when gdb begin reverse-debug, it's >>>>> range is: >>>>> =A08048439: =A0 =A0 =A0 8b 45 08 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mov = =A0 =A00x8(%ebp),%eax >>>>> =A0804843c: =A0 =A0 =A0 83 e8 01 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sub = =A0 =A0$0x1,%eax >>>>> =A0804843f: =A0 =A0 =A0 89 04 24 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mov = =A0 =A0%eax,(%esp) >>>>> =A08048442: =A0 =A0 =A0 e8 dd ff ff ff =A0 =A0 =A0 =A0 =A0call =A0 80= 48424 <_Z9factoriali> >>>>> =A08048447: =A0 =A0 =A0 0f af 45 08 =A0 =A0 =A0 =A0 =A0 =A0 imul =A0 = 0x8(%ebp),%eax >>>>> =A0804844b: =A0 =A0 =A0 89 45 fc =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mov = =A0 =A0%eax,-0x4(%ebp) >>>>> Why is changed to infrun: stepping inside range [0x8048457-0x804845a]? >>>>> That is because when inferior step at: >>>>> =A08048458: =A0 =A0 =A0 c3 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0ret >>>>> 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. =A0It will r= un >>>>> to: >>>>> =A0ecs->event_thread->step_range_start =3D stop_pc_sal.pc; >>>>> =A0ecs->event_thread->step_range_end =3D stop_pc_sal.end; >>>>> =A0ecs->event_thread->step_frame_id =3D get_frame_id (get_current_fra= me >>>>> ()); >>>>> =A0ecs->event_thread->current_line =3D stop_pc_sal.line; >>>>> =A0ecs->event_thread->current_symtab =3D stop_pc_sal.symtab; >>>>> >>>>> =A0if (debug_infrun) >>>>> =A0 =A0fprintf_unfiltered (gdb_stdlog, "infrun: keep going\n"); >>>>> =A0keep_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. =A0So I >>>>> didn't fix it. >>>>> >>>>> >>>>> >>>>> >>>>> Thanks, >>>>> Hui >>>>> >> >> > > > 2009-06-14 =A0Hui Zhu =A0 > =A0 =A0 =A0 =A0 =A0 =A0Michael Snyder =A0 > > =A0 =A0 =A0 =A0* infrun.c (handle_inferior_event): Improve reverse steppi= ng > =A0 =A0 =A0 =A0through function epilogue. > > Index: infrun.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /cvs/src/src/gdb/infrun.c,v > retrieving revision 1.387 > diff -u -p -r1.387 infrun.c > --- infrun.c =A0 =A011 Jun 2009 11:57:46 -0000 =A0 =A0 =A01.387 > +++ infrun.c =A0 =A015 Jun 2009 00:45:17 -0000 > @@ -3623,9 +3623,17 @@ infrun: not switching back to stepped th > > =A0 =A0 =A0Note that step_range_end is the address of the first instructi= on > =A0 =A0 =A0beyond the step range, and NOT the address of the last instruc= tion > - =A0 =A0 within it! */ > + =A0 =A0 within it! > + > + =A0 =A0 Note also that during reverse execution, we may be stepping > + =A0 =A0 through a function epilogue and therefore must detect when > + =A0 =A0 the current-frame changes in the middle of a line. =A0*/ > + > =A0 if (stop_pc >=3D ecs->event_thread->step_range_start > - =A0 =A0 =A0&& stop_pc < ecs->event_thread->step_range_end) > + =A0 =A0 =A0&& stop_pc < ecs->event_thread->step_range_end > + =A0 =A0 =A0&& (execution_direction !=3D EXEC_REVERSE > + =A0 =A0 =A0 =A0 || frame_id_eq (get_frame_id (get_current_frame ()), > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ecs->event_thread->step= _frame_id))) > =A0 =A0 { > =A0 =A0 =A0 if (debug_infrun) > =A0 =A0 =A0 =A0fprintf_unfiltered (gdb_stdlog, "infrun: stepping inside r= ange > [0x%s-0x%s]\n", > >