From patchwork Tue Jan 5 16:43:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 10226 Received: (qmail 65380 invoked by alias); 5 Jan 2016 16:43:27 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 65070 invoked by uid 89); 5 Jan 2016 16:43:26 -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=daniel, Daniel, conversation, inclined 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; Tue, 05 Jan 2016 16:43:25 +0000 Received: by mail-pa0-f54.google.com with SMTP id cy9so217677385pac.0 for ; Tue, 05 Jan 2016 08:43:24 -0800 (PST) X-Received: by 10.66.158.193 with SMTP id ww1mr134788209pab.21.1452012203182; Tue, 05 Jan 2016 08:43:23 -0800 (PST) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id n64sm129349558pfi.19.2016.01.05.08.43.20 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Tue, 05 Jan 2016 08:43:22 -0800 (PST) From: Yao Qi To: Pedro Alves Cc: Yao Qi , gdb-patches@sourceware.org Subject: Re: [RFC 1/2] Check input interrupt first when reading packet References: <1450697443-29067-1-git-send-email-yao.qi@linaro.org> <1450697443-29067-2-git-send-email-yao.qi@linaro.org> <568AAEA6.4050303@redhat.com> Date: Tue, 05 Jan 2016 16:43:19 +0000 In-Reply-To: <568AAEA6.4050303@redhat.com> (Pedro Alves's message of "Mon, 04 Jan 2016 17:40:54 +0000") Message-ID: <86lh84x9i0.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Pedro Alves writes: Hi Pedro, sorry for the delayed reply. Takes much time reading these patches and discussions in archives. > 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). This line of doc was added due to the request from Daniel's comments https://www.sourceware.org/ml/gdb/2005-11/msg00349.html however, I am not sure what is the conversation Daniel referred to. > > > 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. I am inclined to tweak the doc as well, because looks people at that moment believed that ^C is meaningless when the target is stopped. See https://www.sourceware.org/ml/gdb-patches/2005-11/msg00307.html >> 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.) Done. diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c index 5f43820..c5f4647 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) (); + continue; + } + if (c == '$') break; if (remote_debug)