From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24782 invoked by alias); 9 Aug 2005 13:45:43 -0000 Mailing-List: contact gdb-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sources.redhat.com Received: (qmail 24688 invoked by uid 22791); 9 Aug 2005 13:45:34 -0000 Received: from zproxy.gmail.com (HELO zproxy.gmail.com) (64.233.162.198) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Tue, 09 Aug 2005 13:45:34 +0000 Received: by zproxy.gmail.com with SMTP id x3so743625nze for ; Tue, 09 Aug 2005 06:45:31 -0700 (PDT) Received: by 10.36.101.10 with SMTP id y10mr2771554nzb; Tue, 09 Aug 2005 06:45:31 -0700 (PDT) Received: by 10.36.118.14 with HTTP; Tue, 9 Aug 2005 06:45:30 -0700 (PDT) Message-ID: <6f48278f050809064556b5ae5b@mail.gmail.com> Date: Tue, 09 Aug 2005 13:45:00 -0000 From: Jie Zhang To: Jie Zhang , gdb@sources.redhat.com Subject: Re: [RFC] Reinsert the single stepping breakpoint after being hopped over In-Reply-To: <20050802154456.GA5342@nevyn.them.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline References: <6f48278f05080208114fb35462@mail.gmail.com> <20050802154456.GA5342@nevyn.them.org> X-SW-Source: 2005-08/txt/msg00040.txt.bz2 Daniel, Thanks for your thoughts. I tried your patch. It also works. However, I think the concept of reinserting the hopped stepping breakpoint is more intuitive. Thanks, Jie On 8/2/05, Daniel Jacobowitz wrote: > On Tue, Aug 02, 2005 at 11:11:45PM +0800, Jie Zhang wrote: > > Hi, > > > > I'm working on a new port of GDB for Blackfin. It's usually used for > > remote debugging using gdbserver and software single stepping. It has > > many FAILs for schedlock.exp. I traced these FAILs and, I think, found > > a bug in GDB. > > > > Assume there are two threads in the program and we are stepping on > > one. (I use GDB for the whole of GDB and gdbserver.) > > > > 1. GDB inserts a breakpoint for stepping and resumes both threads. > > 2. Both threads hit this breakpoint. > > 3. The first stopped thread GDB looks for (for some reason) is not the > > thread we are stepping. GDB make it hop over the stepping breakpoint. > > 4. GDB resume both threads by > > > > resume (1, TARGET_SIGNAL_0); > > > > So the thread we are stepping will do a new step and the previous stop > > of step will never be noticed by GDB. (It's caught by gdbserver and > > saved as a pending status. However, when it stops again for the new > > step, the previous stepping breakpoint has been removed, so > > check_removed_breakpoint () will clear status_pending_p and the > > previous stop is lost.) GDB will never get a chance to check if the > > stepping is out of the range. The step command will not return. >=20 > Yes... I fixed this once already, but I failed to consider the case > where both threads have hit the singlestep breakpoint, rather than > another thread hitting the breakpoint before the original thread has > had a chance to step. >=20 > I've fixed the version you encountered before, but it appears I never > submitted the patch. I'm not sure if it still applies, but could you > give this a try? >=20 > > To fix it, GDB should put the previous stepping breakpoint back, not > > do a new stepping when resuming both threads. The following patch is > > trying to fix this. It adds a new field "singlestep_breakpoint_addr" > > in "struct thread_info" to remember the last single stepping > > breakpoint address. Passing 2 as the second argument to > > SOFTWARE_SINGLE_STEP () to tell the target software single stepping > > function that we just reinsert the previous single stepping > > breakpoint. >=20 > Interesting approach. I don't like the implementation - I'd rather not > extend context_switch - but the concept may be better than mine. Let > me think about this for a little. >=20 > -- > Daniel Jacobowitz > CodeSourcery, LLC >=20 > 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: /big/fsf/rsync/src-cvs/src/gdb/infrun.c,v > retrieving revision 1.137 > diff -u -p -r1.137 infrun.c > --- infrun.c 16 Feb 2004 20:49:51 -0000 1.137 > +++ infrun.c 6 Mar 2004 04:23:06 -0000 > @@ -479,6 +479,9 @@ static int singlestep_breakpoints_insert > /* The thread we inserted single-step breakpoints for. */ > static ptid_t singlestep_ptid; >=20 > +/* PC when we started this single-step. */ > +static CORE_ADDR singlestep_pc; > + > /* If another thread hit the singlestep breakpoint, we save the original > thread here so that we can resume single-stepping it later. */ > static ptid_t saved_singlestep_ptid; > @@ -570,6 +573,7 @@ resume (int step, enum target_signal sig > `wait_for_inferior' */ > singlestep_breakpoints_inserted_p =3D 1; > singlestep_ptid =3D inferior_ptid; > + singlestep_pc =3D read_pc (); > } >=20 > /* Handle any optimized stores to the inferior NOW... */ > @@ -1201,6 +1205,7 @@ context_switch (struct execution_control > &ecs->current_line, &ecs->current_symtab, &step_= sp); > } > inferior_ptid =3D ecs->ptid; > + flush_cached_frames (); > } >=20 > /* Wrapper for PC_IN_SIGTRAMP that takes care of the need to find the > @@ -1803,7 +1808,10 @@ handle_inferior_event (struct execution_ > } > else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inser= ted_p) > { > + gdb_assert (ptid_equal (inferior_ptid, singlestep_ptid)); > + > ecs->random_signal =3D 0; > + > /* The call to in_thread_list is necessary because PTIDs someti= mes > change when we go from single-threaded to multi-threaded. If > the singlestep_ptid is still in the list, assume that it is > @@ -1811,9 +1819,32 @@ handle_inferior_event (struct execution_ > if (!ptid_equal (singlestep_ptid, ecs->ptid) > && in_thread_list (singlestep_ptid)) > { > - thread_hop_needed =3D 1; > - stepping_past_singlestep_breakpoint =3D 1; > - saved_singlestep_ptid =3D singlestep_ptid; > + /* If the PC of the thread we were trying to single-step > + has changed, discard this event (which we were going > + to ignore anyway), and pretend we saw that thread > + trap. This runs a risk of losing signal information > + for singlestep_ptid, but prevents us continuously > + moving the single-step breakpoint. This situation > + means that the thread has trapped or been signalled, > + but the event has not been reported to GDB yet. > + Really we should arrange to report all events, or to > + re-poll the remote looking for this particular > + thread (i.e. temporarily enable schedlock). */ > + if (read_pc_pid (singlestep_ptid) !=3D singlestep_pc) > + { > + /* The current context still belongs to > + singlestep_ptid. Don't swap here, since that's > + the context we want to use. Just fudge our > + state and continue. */ > + ecs->ptid =3D singlestep_ptid; > + stop_pc =3D read_pc_pid (ecs->ptid); > + } > + else > + { > + thread_hop_needed =3D 1; > + stepping_past_singlestep_breakpoint =3D 1; > + saved_singlestep_ptid =3D singlestep_ptid; > + } > } > } >=20 > @@ -1942,8 +1973,6 @@ handle_inferior_event (struct execution_ >=20 > if (context_hook) > context_hook (pid_to_thread_id (ecs->ptid)); > - > - flush_cached_frames (); > } >=20 > if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p) >