From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 64867 invoked by alias); 26 Jan 2016 13:53:23 -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 64849 invoked by uid 89); 26 Jan 2016 13:53:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-pf0-f196.google.com Received: from mail-pf0-f196.google.com (HELO mail-pf0-f196.google.com) (209.85.192.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 26 Jan 2016 13:53:21 +0000 Received: by mail-pf0-f196.google.com with SMTP id 65so8250609pfd.1 for ; Tue, 26 Jan 2016 05:53:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-type :content-transfer-encoding; bh=67G6MrK/Z0RLSJ6A36y7xDNMCbANPQLG7KZYn38RoVs=; b=X2vt1KlNeYaUiw00gB2MLxw2GkCuqZfh7zaiN+xi1kliFwxF+7BflrsL3ToM9HMacm jRiuxEbtUWcAudtOrA5+foQVRv+xCYR7bWNDIlQdaBbMRxBD/6tywMlbehr35IFgPiHd +gJ8jPDrV/DwpwHNQwel9uJHYRk8bCK9vNwb4pFhbmuBqPKn1zfTxmkEcf0nx0fc5h6L J2Dwk1LfGenRdrMXUoiDsFKtfoL3v1FWHEOtKZ728YJBn/KI60sC2tSp+OCEMHSxDvfo x7l7OxVRp3tGx81Ay9eZZwlpg0zRoY9BA7IlhxgzuK2JG1QmagiJspWQRboAwZxJO3AP +K+A== X-Gm-Message-State: AG10YORNNxpYDzJWNvfD5eVlj9hQwDvjX0PRBaknpfyGrY+Sj81LAH9lCW5nVXit2Xbinw== X-Received: by 10.98.68.193 with SMTP id m62mr33780422pfi.153.1453816399200; Tue, 26 Jan 2016 05:53:19 -0800 (PST) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id z5sm2118477pas.29.2016.01.26.05.53.15 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Tue, 26 Jan 2016 05:53:18 -0800 (PST) From: Yao Qi To: Pedro Alves Cc: Yao Qi , gdb-patches@sourceware.org Subject: Re: [PATCH 1/2] [GDBserver] Check input interrupt after reading in a packet References: <86powqqa57.fsf@gmail.com> <1453802339-20401-1-git-send-email-yao.qi@linaro.org> <1453802339-20401-2-git-send-email-yao.qi@linaro.org> <56A75BA7.6070602@redhat.com> Date: Tue, 26 Jan 2016 13:53:00 -0000 In-Reply-To: <56A75BA7.6070602@redhat.com> (Pedro Alves's message of "Tue, 26 Jan 2016 11:42:31 +0000") Message-ID: <86h9i0qwfu.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-01/txt/msg00654.txt.bz2 Pedro Alves writes: > I think we should expand the explanation, like: > > /* The readchar above may have already read a '\003' out of the socket = and > moved it to the local buffer. For example, when GDB sends vCont;c imm= ediately > followed by interrupt (see gdb.base/interrupt-noterm.exp). As soon as= we see > the vCont;c, we'll resume the inferior and wait. Since we've already = moved > the '\003' to the local buffer, SIGIO won't help. In that case, if we= don't > check for interrupt after the vCont;c packet, the interrupt character = would > stay in the buffer unattended until after the next (unrelated) stop. = */ > Take it into the patch. >> + if (readchar_bufcnt > 0 && *readchar_bufp =3D=3D '\003') > > This should be a "while" instead of a single "if". > Fixed. >> + { >> + /* Consume the interrupt character in the buffer. */ >> + readchar (); >> + (*the_target->request_interrupt) (); >> + } >> + >> return bp - buf; >> } > > Otherwise LGTM. Patch below is pushed in. --=20 Yao (=E9=BD=90=E5=B0=A7) =46rom 18879fef1741464e522624bcc529048400453e0d Mon Sep 17 00:00:00 2001 From: Yao Qi Date: Tue, 26 Jan 2016 13:50:22 +0000 Subject: [PATCH] [GDBserver] Check input interrupt after reading in a packet GDBserver may read some packet together with '\003' in one go. We've already checked '\003' first when reading packet by my patch, Check input interrupt first when reading packet https://sourceware.org/ml/gdb-patches/2016-01/msg00057.html but if we don't check '\003' *after* each packet, the interrupt will be processed next time GDBserver reads from the buffer, so that the interrupt isn't processed in time. For example, GDB sends vCont;c and interrupt (see gdb.base/interrupt-noterm.exp), we'll resume the inferior and wait once packet vCont;c is seen. If we don't check the interrupt character after vCont;c packet, interrupt character will stay in the buffer unattended until GDBserver returns from the wait, which may take a while. Note that since we've read '\003' from file descriptor, SIGIO signal handler input_interrupt doesn't help either. This issue can be exposed by hacking the end of getpkt like @@ -1041,6 +1050,9 @@ getpkt (char *buf) } } + if (readchar_bufcnt > 0) + gdb_assert (*readchar_bufp !=3D '\003'); + return bp - buf; } and this can trigger internal error, (gdb) PASS: gdb.base/interrupt-noterm.exp: interrupt Remote connection closed^M (gdb) FAIL: gdb.base/interrupt-noterm.exp: inferior received SIGINT Remote debugging from host 10.2.206.40^M /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/remote-utils.c:1054: A probl= em internal to GDBserver has been detected.^M getpkt: Assertion `*readchar_bufp !=3D '\003'' failed.^M This patch is to peek the buffer, if it is '\003', consume it and call *the_target->request_interrupt. gdb/gdbserver: 2016-01-26 Yao Qi * remote-utils.c (getpkt): If the buffer isn't empty, and the first character is '\003', call *the_target->request_interrupt. diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index 2e94aa8..4aa7350 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,8 @@ +2016-01-26 Yao Qi + + * remote-utils.c (getpkt): If the buffer isn't empty, and the + first character is '\003', call *the_target->request_interrupt. + 2016-01-25 Yao Qi =20 * remote-utils.c (new_thread_notify): Remove. diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c index 292197a..ccc99f1 100644 --- a/gdb/gdbserver/remote-utils.c +++ b/gdb/gdbserver/remote-utils.c @@ -1041,6 +1041,22 @@ getpkt (char *buf) } } =20 + /* The readchar above may have already read a '\003' out of the socket + and moved it to the local buffer. For example, when GDB sends + vCont;c immediately followed by interrupt (see + gdb.base/interrupt-noterm.exp). As soon as we see the vCont;c, we'll + resume the inferior and wait. Since we've already moved the '\003' + to the local buffer, SIGIO won't help. In that case, if we don't + check for interrupt after the vCont;c packet, the interrupt character + would stay in the buffer unattended until after the next (unrelated) + stop. */ + while (readchar_bufcnt > 0 && *readchar_bufp =3D=3D '\003') + { + /* Consume the interrupt character in the buffer. */ + readchar (); + (*the_target->request_interrupt) (); + } + return bp - buf; } =20