From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26412 invoked by alias); 17 Feb 2016 11:46:16 -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 26337 invoked by uid 89); 17 Feb 2016 11:46:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=acknowledged, Hx-languages-length:2882 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; Wed, 17 Feb 2016 11:46:12 +0000 Received: from svr-orw-fem-06.mgc.mentorg.com ([147.34.97.120]) by relay1.mentorg.com with esmtp id 1aW0Yb-0004hw-Hb from Luis_Gustavo@mentor.com ; Wed, 17 Feb 2016 03:46:09 -0800 Received: from [172.30.1.77] (147.34.91.1) by SVR-ORW-FEM-06.mgc.mentorg.com (147.34.97.120) with Microsoft SMTP Server id 14.3.224.2; Wed, 17 Feb 2016 03:46:09 -0800 Subject: Re: [PATCH 4/5] gdbserver: Leave already-vCont-resumed threads as they were References: <1455677091-13683-1-git-send-email-palves@redhat.com> <1455677091-13683-5-git-send-email-palves@redhat.com> To: Pedro Alves , From: Luis Machado Reply-To: Luis Machado Message-ID: <56C45D7D.2040904@codesourcery.com> Date: Wed, 17 Feb 2016 11:46: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: <1455677091-13683-5-git-send-email-palves@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00505.txt.bz2 On 02/17/2016 12:44 AM, Pedro Alves wrote: > Currently GDB never sends more than one action per vCont packet, when > connected in non-stop mode. A follow up patch will change that, and > it exposed a gdbserver problem with the vCont handling. > > For example, this in non-stop mode: > > => vCont;s:p1.1;c > <= OK > > Should be equivalent to: > > => vCont;s:p1.1 > <= OK > => vCont;c > <= OK > > But gdbserver currently doesn't handle this. In the latter case, > "vCont;c" makes gdbserver clobber the previous step request. This > patch fixes that. > > Note the server side must ignore resume actions for the thread that > has a pending %Stopped notification (and any other threads with events > pending), until GDB acks the notification with vStopped. Otherwise, > e.g., the following case is mishandled: > > #1 => g (or any other packet) > #2 <= [registers] > #3 <= %Stopped T05 thread:p1.2 > #4 => vCont s:p1.1;c > #5 <= OK > > Above, the server must not resume thread p1.2 when it processes the > vCont. GDB can't know that p1.2 stopped until it acks the %Stopped > notification. (Otherwise it wouldn't send a default "c" action.) > > (The vCont documentation already specifies this.) > > Finally, special care must also be given to handling fork/vfork > events. A (v)fork event actually tells us that two processes stopped > -- the parent and the child. Until we follow the fork, we must not > resume the child. Therefore, if we have a pending fork follow, we > must not send a global wildcard resume action (vCont;c). We can still > send process-wide wildcards though. > > (The comments above will be added as code comments to gdb in a follow > up patch.) > > gdb/gdbserver/ChangeLog: > 2016-02-16 Pedro Alves > > * linux-low.c (linux_set_resume_request): Ignore resume requests > for already-resumed threads. > * server.c (in_queued_stop_replies_ptid, in_queued_stop_replies): > New functions. > * server.h (in_queued_stop_replies): New declaration. > --- > gdb/gdbserver/linux-low.c | 27 +++++++++++++++++++++++++++ > gdb/gdbserver/server.c | 33 ++++++++++++++++++++++++++++++++- > gdb/gdbserver/server.h | 4 ++++ > 3 files changed, 63 insertions(+), 1 deletion(-) > > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index 8b025bd..2cac4c0 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -4465,6 +4465,33 @@ linux_set_resume_request (struct inferior_list_entry *entry, void *arg) > continue; > } > > + /* Ignore (wildcard) resume requests for already-resumed > + requests. */ For already-resumed requests or threads? Looked a little confusing. If you really meant "requests", then we may need to adjust the wording a bit, like "for requests that have already been acknowledged.". The rest of the series looks good to me.