From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17784 invoked by alias); 19 Jul 2010 07:58:36 -0000 Received: (qmail 17294 invoked by uid 22791); 19 Jul 2010 07:58:34 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from mail-pw0-f41.google.com (HELO mail-pw0-f41.google.com) (209.85.160.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 19 Jul 2010 07:58:26 +0000 Received: by pwi8 with SMTP id 8so4047486pwi.0 for ; Mon, 19 Jul 2010 00:58:24 -0700 (PDT) Received: by 10.142.192.17 with SMTP id p17mr6326023wff.307.1279526304157; Mon, 19 Jul 2010 00:58:24 -0700 (PDT) MIME-Version: 1.0 Received: by 10.142.109.20 with HTTP; Mon, 19 Jul 2010 00:58:04 -0700 (PDT) In-Reply-To: <201006221112.56408.pedro@codesourcery.com> References: <201006111455.36401.pedro@codesourcery.com> <201006221112.56408.pedro@codesourcery.com> From: Hui Zhu Date: Mon, 19 Jul 2010 07:58:00 -0000 Message-ID: Subject: Re: [RFC] Add support of software single step to process record To: Pedro Alves Cc: ping huang , shuchang zhou , gdb-patches@sourceware.org, Joel Brobecker , Michael Snyder , paawan oza , Tom Tromey 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: 2010-07/txt/msg00273.txt.bz2 On Tue, Jun 22, 2010 at 18:12, Pedro Alves wrote: > Hi Hui, > > On Sunday 20 June 2010 08:28:40, Hui Zhu wrote: >> On Fri, Jun 11, 2010 at 21:55, Pedro Alves wrot= e: >> > I'm felling a bit dense, and I don't see what is that actually >> > catching. =A0If going backwards, the assertion always ends up >> > evaled as true, nomatter if sofware single-steps are inserted >> > or not, or whether `step' is set. =A0Did you mean to assert >> > that when going backwards, there shouldn't ever be software >> > single-step breakpoints inserted? >> > >> > This patch is okay otherwise. =A0Thanks. >> >> Thanks Pedro. >> I was also confused by this issue too. =A0I thought it will never happen >> too. =A0But Ping said he got this issue. =A0And I didn't have the risc >> board to test. =A0So I gived up and put this patch to him. >> >> So I think this patch is not very hurry to checked in until some one >> post a risc prec support patch. =A0At that time, I will make this issue >> clear. > > I'd be fine with putting the patch in now, but without the change to > that gdb_assert. =A0It looked like a step in the right direction, > and we can fix any left issues later. > > -- > Pedro Alves > Agree with you. I delay this patch some days because I want make it check in after 7.2. Now, following patch checked in. Thanks, Hui 2010-07-19 Hui Zhu * breakpoint.c (single_step_breakpoints_inserted): New function. * breakpoint.h (single_step_breakpoints_inserted): Extern. * infrun.c (maybe_software_singlestep): Add check code. * record.c (record_resume): Add code for software single step. (record_wait): Ditto. --- breakpoint.c | 10 +++++++++ breakpoint.h | 1 infrun.c | 3 +- record.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++= ----- 4 files changed, 72 insertions(+), 6 deletions(-) --- a/breakpoint.c +++ b/breakpoint.c @@ -10468,6 +10468,16 @@ insert_single_step_breakpoint (struct gd paddress (gdbarch, next_pc)); } +/* Check if the breakpoints used for software single stepping + were inserted or not. */ + +int +single_step_breakpoints_inserted (void) +{ + return (single_step_breakpoints[0] !=3D NULL + || single_step_breakpoints[1] !=3D NULL); +} + /* Remove and delete any breakpoints used for software single step. */ void --- a/breakpoint.h +++ b/breakpoint.h @@ -984,6 +984,7 @@ extern int remove_hw_watchpoints (void); twice before remove is called. */ extern void insert_single_step_breakpoint (struct gdbarch *, struct address_space *, CORE_ADDR); +extern int single_step_breakpoints_inserted (void); extern void remove_single_step_breakpoints (void); /* Manage manual breakpoints, separate from the normal chain of --- a/infrun.c +++ b/infrun.c @@ -1515,7 +1515,8 @@ maybe_software_singlestep (struct gdbarc { int hw_step =3D 1; - if (gdbarch_software_single_step_p (gdbarch) + if (execution_direction =3D=3D EXEC_FORWARD + && gdbarch_software_single_step_p (gdbarch) && gdbarch_software_single_step (gdbarch, get_current_frame ())) { hw_step =3D 0; --- a/record.c +++ b/record.c @@ -1011,9 +1011,43 @@ record_resume (struct target_ops *ops, p if (!RECORD_IS_REPLAY) { + struct gdbarch *gdbarch =3D target_thread_architecture (ptid); + record_message (get_current_regcache (), signal); - record_beneath_to_resume (record_beneath_to_resume_ops, ptid, 1, - signal); + + if (!step) + { + /* This is not hard single step. */ + if (!gdbarch_software_single_step_p (gdbarch)) + { + /* This is a normal continue. */ + step =3D 1; + } + else + { + /* This arch support soft sigle step. */ + if (single_step_breakpoints_inserted ()) + { + /* This is a soft single step. */ + record_resume_step =3D 1; + } + else + { + /* This is a continue. + Try to insert a soft single step breakpoint. */ + if (!gdbarch_software_single_step (gdbarch, + get_current_frame ())) + { + /* This system don't want use soft single step. + Use hard sigle step. */ + step =3D 1; + } + } + } + } + + record_beneath_to_resume (record_beneath_to_resume_ops, + ptid, step, signal); } } @@ -1089,12 +1123,16 @@ record_wait (struct target_ops *ops, /* This is not a single step. */ ptid_t ret; CORE_ADDR tmp_pc; + struct gdbarch *gdbarch =3D target_thread_architecture (inferior_ptid); while (1) { ret =3D record_beneath_to_wait (record_beneath_to_wait_ops, ptid, status, options); + if (single_step_breakpoints_inserted ()) + remove_single_step_breakpoints (); + if (record_resume_step) return ret; @@ -1134,8 +1172,12 @@ record_wait (struct target_ops *ops, } else { - /* This must be a single-step trap. Record the - insn and issue another step. */ + /* This is a single-step trap. Record the + insn and issue another step. + FIXME: this part can be a random SIGTRAP too. + But GDB cannot handle it. */ + int step =3D 1; + if (!record_message_wrapper_safe (regcache, TARGET_SIGNAL_0)) { @@ -1144,8 +1186,20 @@ record_wait (struct target_ops *ops, break; } + if (gdbarch_software_single_step_p (gdbarch)) + { + /* Try to insert the software single step breakpoint. + If insert success, set step to 0. */ + set_executing (inferior_ptid, 0); + reinit_frame_cache (); + if (gdbarch_software_single_step (gdbarch, + get_current_frame ())) + step =3D 0; + set_executing (inferior_ptid, 1); + } + record_beneath_to_resume (record_beneath_to_resume_ops, - ptid, 1, + ptid, step, TARGET_SIGNAL_0); continue; }