From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 60241 invoked by alias); 23 Feb 2016 20:23:12 -0000 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 Received: (qmail 60229 invoked by uid 89); 23 Feb 2016 20:23:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=translate, beside, stepped, U*func X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 23 Feb 2016 20:23:10 +0000 Received: from svr-orw-fem-02x.mgc.mentorg.com ([147.34.96.206] helo=SVR-ORW-FEM-02.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1aYJUB-00075Y-46 from Luis_Gustavo@mentor.com ; Tue, 23 Feb 2016 12:23:07 -0800 Received: from [172.30.1.130] (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.3.224.2; Tue, 23 Feb 2016 12:23:06 -0800 Subject: Re: [PATCH 1/8] [GDBserver] Leave child suspended when step over parent References: <1455892594-2294-1-git-send-email-yao.qi@linaro.org> <1455892594-2294-2-git-send-email-yao.qi@linaro.org> To: Yao Qi , From: Luis Machado Reply-To: Luis Machado Message-ID: <56CCBFA7.90009@codesourcery.com> Date: Tue, 23 Feb 2016 20:23:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1455892594-2294-2-git-send-email-yao.qi@linaro.org> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00698.txt.bz2 On 02/19/2016 12:36 PM, Yao Qi wrote: > I see the following GDBserver internal error in two cases, > > gdb/gdbserver/linux-low.c:1922: A problem internal to GDBserver has been detected. > unsuspend LWP 17200, suspended=-1 > > 1. step over a breakpoint on fork/vfork syscall instruction, > 2. step over a breakpoint on clone syscall instruction and child > threads hits a breakpoint, > > the stack backtrace is > > #0 internal_error (file=file@entry=0x44c4c0 "gdb/gdbserver/linux-low.c", line=line@entry=1922, > fmt=fmt@entry=0x44c7d0 "unsuspend LWP %ld, suspended=%d\n") at gdb/gdbserver/../common/errors.c:51 > #1 0x0000000000424014 in lwp_suspended_decr (lwp=, lwp=) at gdb/gdbserver/linux-low.c:1922 > #2 0x000000000042403a in unsuspend_one_lwp (entry=, except=0x66e8c0) at gdb/gdbserver/linux-low.c:2885 > #3 0x0000000000405f45 in find_inferior (list=, func=func@entry=0x424020 , arg=arg@entry=0x66e8c0) > at gdb/gdbserver/inferiors.c:243 > #4 0x00000000004297de in unsuspend_all_lwps (except=0x66e8c0) at gdb/gdbserver/linux-low.c:2895 > #5 linux_wait_1 (ptid=..., ourstatus=ourstatus@entry=0x665ec0 , target_options=target_options@entry=0) > at gdb/gdbserver/linux-low.c:3632 > #6 0x000000000042a764 in linux_wait (ptid=..., ourstatus=0x665ec0 , target_options=0) > at gdb/gdbserver/linux-low.c:3770 > #7 0x0000000000411163 in mywait (ptid=..., ourstatus=ourstatus@entry=0x665ec0 , options=options@entry=0, connected_wait=connected_wait@entry=1) > at gdb/gdbserver/target.c:214 > #8 0x000000000040b1f2 in resume (actions=0x66f800, num_actions=1) at gdb/gdbserver/server.c:2757 > #9 0x000000000040f660 in handle_v_cont (own_buf=0x66a630 "vCont;c:p45e9.-1") at gdb/gdbserver/server.c:2719 > > when GDBserver steps over a thread, other threads have been suspended, > the "stepping" thread may create new thread, but GDBserver doesn't set > it suspend count to 1. When GDBserver unsuspend threads, the child's > suspend count goes to -1, and the assert is triggered. In fact, GDBserver > has already taken care of suspend count of new thread when GDBserver is > suspending all threads except the one GDBserver wants to step over by > https://sourceware.org/ml/gdb-patches/2015-07/msg00946.html > > + /* If we're suspending all threads, leave this one suspended > + too. */ > + if (stopping_threads == STOPPING_AND_SUSPENDING_THREADS) > + { > + if (debug_threads) > + debug_printf ("HEW: leaving child suspended\n"); > + child_lwp->suspended = 1; > + } > > but that is not enough, because new thread can be still spawned in > the thread which is being stepped over. This patch extends the > condition that GDBserver set child's suspend count to one if it is > suspending threads or stepping over the thread. > > gdb/gdbserver: > > 2016-02-19 Yao Qi > > * linux-low.c (handle_extended_wait): Set child suspended > if event_lwp->bp_reinsert isn't zero. > --- > gdb/gdbserver/linux-low.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index 3f085fd..3765e08 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -529,8 +529,10 @@ handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat) > child_thr->last_status.kind = TARGET_WAITKIND_STOPPED; > > /* If we're suspending all threads, leave this one suspended > - too. */ > - if (stopping_threads == STOPPING_AND_SUSPENDING_THREADS) > + too. If we're stepping over the parent, all other threads > + have been suspended already, leave this one suspended too. */ > + if (stopping_threads == STOPPING_AND_SUSPENDING_THREADS > + || event_lwp->bp_reinsert != 0) > { > if (debug_threads) > debug_printf ("HEW: leaving child suspended\n"); > @@ -583,9 +585,11 @@ handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat) > before calling linux_resume_one_lwp. */ > new_lwp->stopped = 1; > > - /* If we're suspending all threads, leave this one suspended > - too. */ > - if (stopping_threads == STOPPING_AND_SUSPENDING_THREADS) > + /* If we're suspending all threads, leave this one suspended > + too. If we're stepping over the parent, all other threads > + have been suspended already, leave this one suspended too. */ > + if (stopping_threads == STOPPING_AND_SUSPENDING_THREADS > + || event_lwp->bp_reinsert != 0) > new_lwp->suspended = 1; > > /* Normally we will get the pending SIGSTOP. But in some cases > The comment is a bit off with regards to what is really being checked here. Maybe it is the code, but "event_lwp->bp_reinsert != 0" doesn't exactly translate to "stepping over the parent". Maybe things would improve if we defined what "this one" means in the second case. It is a new thread or a child process thread, right? I'm looking at the two hunks above and they seem to want to be unified, but it is beside the point of the patch. :-)