Message ID | 1450697443-29067-2-git-send-email-yao.qi@linaro.org |
---|---|
State | New, archived |
Headers |
Received: (qmail 1148 invoked by alias); 21 Dec 2015 11:30:53 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 1023 invoked by uid 89); 21 Dec 2015 11:30:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=003, sk:request, Hx-languages-length:1623, control-c X-HELO: mail-pa0-f54.google.com Received: from mail-pa0-f54.google.com (HELO mail-pa0-f54.google.com) (209.85.220.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 21 Dec 2015 11:30:51 +0000 Received: by mail-pa0-f54.google.com with SMTP id jx14so70361229pad.2 for <gdb-patches@sourceware.org>; Mon, 21 Dec 2015 03:30:51 -0800 (PST) X-Received: by 10.66.101.36 with SMTP id fd4mr26713384pab.76.1450697450179; Mon, 21 Dec 2015 03:30:50 -0800 (PST) Received: from E107787-LIN.cambridge.arm.com (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id zo4sm26286984pac.28.2015.12.21.03.30.49 for <gdb-patches@sourceware.org> (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 21 Dec 2015 03:30:49 -0800 (PST) From: Yao Qi <qiyaoltc@gmail.com> X-Google-Original-From: Yao Qi <yao.qi@linaro.org> To: gdb-patches@sourceware.org Subject: [RFC 1/2] Check input interrupt first when reading packet Date: Mon, 21 Dec 2015 11:30:42 +0000 Message-Id: <1450697443-29067-2-git-send-email-yao.qi@linaro.org> In-Reply-To: <1450697443-29067-1-git-send-email-yao.qi@linaro.org> References: <1450697443-29067-1-git-send-email-yao.qi@linaro.org> X-IsSubscribed: yes |
Commit Message
Yao Qi
Dec. 21, 2015, 11:30 a.m. UTC
Hi, I see timeout in one of several runs of random-signal.exp like this, $ (set -e; while true; do make check RUNTESTFLAGS="--target_board=native-gdbserver random-signal.exp"; done) In about every five runs, we can see a fail, PASS: gdb.base/random-signal.exp: continue ^CFAIL: gdb.base/random-signal.exp: stop with control-c (timeout) after some investigation, I find '\003' may be discarded by GDBserver when it is expecting '$'. In GDB side, both normal packets and '\003' are sent via function send, but GDBserver may receive them in one time, that is to say, in the GDBserver's receive buffer, '\003' may appear before or after normal packet. However, current GDBserver doesn't handle this case. With this patch applied, I don't see this fail in multiple runs. Although there is still another timeout fail, that is a different problem, the next patch will fix it. gdb/gdbserver: 2015-12-21 Yao Qi <yao.qi@linaro.org> * remote-utils.c (getpkt): If c is '\003', call target hook request_interrupt. --- gdb/gdbserver/remote-utils.c | 9 +++++++++ 1 file changed, 9 insertions(+)
Comments
On 12/21/2015 11:30 AM, Yao Qi wrote: > Hi, > I see timeout in one of several runs of random-signal.exp like this, > > $ (set -e; while true; do make check RUNTESTFLAGS="--target_board=native-gdbserver random-signal.exp"; done) > > In about every five runs, we can see a fail, > > PASS: gdb.base/random-signal.exp: continue > ^CFAIL: gdb.base/random-signal.exp: stop with control-c (timeout) > > after some investigation, I find '\003' may be discarded by GDBserver when > it is expecting '$'. In GDB side, both normal packets and '\003' are sent > via function send, but GDBserver may receive them in one time, that is to By "in one time", ITYM, "at any time". Yeah, the target may have already stopped just while the user presses ctrl-c, and gdb sends '\003' over. E.g., the test is doing this in a loop: #1 -> vCont;s #2 -- process stops #3 -- kernel sends SIGCHLD to gdbserver #4 <- T05 (gdbserver processes SIGCHLD, sees SIGTRAP stop) #5 -- gdb processes the T05 #6 -- watchpoint didn't trigger, re-resume. The \003 can arrive at any time above. For example: #1 -> vCont;s #2 -- process stops #3 -- kernel sends SIGCHLD to gdbserver #4 <- T05 (gdbserver processes SIGCHLD, sees SIGTRAP stop) + #4.1 -> \003 (user presses ctrl-c) #5 -- gdb processes the T05 #6 -- watchpoint didn't trigger, re-resume. The trouble is that the manual says: "Interrupts received while the program is stopped are discarded." However, I can't see how that can work in general. This can also happen: #1 -> vCont;s #2 -- process stops + #2.1 -> \003 (user presses ctrl-c) #3 -- kernel sends SIGCHLD to gdbserver #4 <- T05 (gdbserver processes SIGCHLD, sees SIGTRAP stop) #5 -- gdb processes the T05 And in that case, if the SIGTRAP happens to cause a user-visible stop (e.g., a breakpoint hit), then the target ends up with a SIGINT queued, that is only seen on next resume (e.g., after the next "continue"). This is similar to the reason we print "Quit (expect signal SIGINT when the program is resumed)" on some ports (utils.c:quit). I once wrote a patch that would fix this while preserving that "while the program is stopped are discarded" invariant: https://sourceware.org/ml/gdb-patches/2013-05/msg00933.html https://sourceware.org/ml/gdb-patches/2013-05/msg00933/step_over.patch See long rationale at: https://sourceware.org/ml/gdb-patches/2013-05/txtHFb6rkZ8Dz.txt ... by making both gdb and gdbserver remember that the user pressed ctrl-c. But that was not complete -- a fuller fix would fix the user-interface issue of sometimes getting "Quit" instead of a SIGINT. The simpler approach of "not ignoring ctrl-c when stopped" is quite tempting. Given that the issue of a ctrl-c happening while the process had just stopped ending up producing an unexpected SIGINT when the program is next resumed can also happen with native debugging and "attach", maybe the simpler approach of always queueing is the right approach. We'll need to tweak the documentation though. > say, in the GDBserver's receive buffer, '\003' may appear before or after > normal packet. However, current GDBserver doesn't handle this case. > > With this patch applied, I don't see this fail in multiple runs. > Although there is still another timeout fail, that is a different problem, > the next patch will fix it. > > gdb/gdbserver: > > 2015-12-21 Yao Qi <yao.qi@linaro.org> > > * remote-utils.c (getpkt): If c is '\003', call target hook > request_interrupt. > --- > gdb/gdbserver/remote-utils.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c > index 05e3d63..8bb5b13 100644 > --- a/gdb/gdbserver/remote-utils.c > +++ b/gdb/gdbserver/remote-utils.c > @@ -959,6 +959,15 @@ getpkt (char *buf) > while (1) > { > c = readchar (); > + > + /* The '\003' may appear before or after each packet, so > + check for an input interrupt. */ > + if (c == '\003') > + { > + (*the_target->request_interrupt) (); > + c = readchar (); I'd write "continue;" instead of another readchar, (Pedantically, you could have another '\003' in the buffer.) > + } > + > if (c == '$') > break; > if (remote_debug) > Thanks, Pedro Alves
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c index 05e3d63..8bb5b13 100644 --- a/gdb/gdbserver/remote-utils.c +++ b/gdb/gdbserver/remote-utils.c @@ -959,6 +959,15 @@ getpkt (char *buf) while (1) { c = readchar (); + + /* The '\003' may appear before or after each packet, so + check for an input interrupt. */ + if (c == '\003') + { + (*the_target->request_interrupt) (); + c = readchar (); + } + if (c == '$') break; if (remote_debug)