From patchwork Thu Aug 20 17:00:40 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 8324 Received: (qmail 5928 invoked by alias); 20 Aug 2015 17:00:47 -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 5914 invoked by uid 89); 20 Aug 2015 17:00:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 20 Aug 2015 17:00:46 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 8661F96C6; Thu, 20 Aug 2015 17:00:44 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7KH0fHg021286; Thu, 20 Aug 2015 13:00:41 -0400 Message-ID: <55D607B8.70103@redhat.com> Date: Thu, 20 Aug 2015 18:00:40 +0100 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Sandra Loosemore , Gary Benson CC: gdb-patches@sourceware.org, Joel Brobecker , Doug Evans , Jan Kratochvil , =?windows-1252?Q?Andr=E9_P?= =?windows-1252?Q?=F6nitz?= , Paul Koning Subject: Re: [PATCH] Prelimit number of bytes to read in "vFile:pread:" References: <55D3DB83.4050204@redhat.com> <1439980862-21305-1-git-send-email-gbenson@redhat.com> <20150819105054.GA22009@blade.nx> <55D4B190.6080700@codesourcery.com> <20150819172059.GA31845@blade.nx> <55D4F125.4080409@codesourcery.com> <55D5F799.9020700@redhat.com> In-Reply-To: <55D5F799.9020700@redhat.com> On 08/20/2015 04:51 PM, Pedro Alves wrote: > I don't see a quick, easy and good way around waiting for the whole file to read. > Having GDB handle ctrl-c differently while it is handling internal events > is already a source of trouble, and the fix always seemed to me to be to make ctrl-c > work like what is happening here. Specifically, I mean that e.g., say that the > user sets a conditional breakpoint that is evaluated on gdb's side, and then the > target stops for that conditional breakpoint, gdb evaluates the expression, > and then resumes the target again, on and on. If the user presses ctrl-c just > while the conditional breakpoint's expression is being evaluated, and something along > that code path called target_terminal_ours (thus pulling input/ctrl-c to the > regular "Quit" SIGINT handler), gdb behaves differently (shows a "Quit" and the > debug session ends up likely broken) from when the ctrl-c is pressed while the target > is really running. I'd argue that the ctrl-c in both cases should be passed > down all the way to the target the same way, and that any internal stop and > breakpoint condition evaluation is magic that should be hidden. Just like > what is happening here with file reading. > > Though having said that, it does look like even that isn't working properly, > as I'd expect this: > > (top-gdb) c > Continuing. > (no debugging symbols found)...done. > > Breakpoint 1, main (argc=1, argv=0x7fffffffe3d8) at ../../../src/gdb/gdbserver/server.c:3635 > 3635 ../../../src/gdb/gdbserver/server.c: No such file or directory. > (gdb) > > to be slow (that is, the file reading isn't really interrupted), but then > the target stops with SIGINT as soon as gdb resumes it again after reading > the DSOs. But it is reaching main anyway, and there's no sign > of "Program stopped with SIGINT"... > > Not sure where the bug is. It may be on the gdbserver side. OK, so gdbserver receives the \003, but since the target is stopped, the normal packet processing loop discards the \003, as it doesn't look like a start of a RSP packet frame (that is, it isn't a $). I'm thinking that maybe the best way to handle this may be to still leave SIGINT forwarding to the target, so that in case gdb re-resumes the target quick enough, the ctrl-c turns into a real SIGINT on the target. But then if it takes long for gdb or gdbserver or the target to react to the ctrl-c, then the user presses ctrl-c again, and gdb shows the old: Interrupted while waiting for the program. Give up (and stop debugging it)? (y or n) AFAICS, that query is never issued anywhere if the target is async. And the remote target is always async nowadays. To make that query appear promptly, we'd hook it to the QUIT macro. Something like this: (gdb) c Continuing. Reading symbols from target:/lib/x86_64-linux-gnu/libdl.so.2...(no debugging symbols found)...done. Reading symbols from target:/lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done. ^CInterrupted while waiting for the program. Give up (and stop debugging it)? (y or n) y Quit (gdb) Could you try the hacky patch below just to see if it makes a difference to you? It seems that GDB still doesn't react as soon as it could, but I'd guess that Gary's previous patch to add a QUIT around vFile:pread's would fix that. diff --git a/gdb/extension.c b/gdb/extension.c index dac203b..d754eb1 100644 --- a/gdb/extension.c +++ b/gdb/extension.c @@ -822,6 +822,8 @@ set_quit_flag (void) quit_flag = 1; } +void target_check_quit_flag (void); + /* Return true if the quit flag has been set, false otherwise. Note: The flag is cleared as a side-effect. The flag is checked in all extension languages that support cooperative @@ -833,6 +835,8 @@ check_quit_flag (void) int i, result = 0; const struct extension_language_defn *extlang; + target_check_quit_flag (); + ALL_ENABLED_EXTENSION_LANGUAGES (i, extlang) { if (extlang->ops->check_quit_flag != NULL) diff --git a/gdb/remote.c b/gdb/remote.c index bfa5469..e0c61b7 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -5206,6 +5206,8 @@ async_handle_remote_sigint (int sig) gdb_call_async_signal_handler (async_sigint_remote_token, 0); } +static int sigint_twice = 0; + /* Signal handler for SIGINT, installed after SIGINT has already been sent once. It will take effect the second time that the user sends a ^C. */ @@ -5215,6 +5217,19 @@ async_handle_remote_sigint_twice (int sig) signal (sig, async_handle_remote_sigint); /* See note in async_handle_remote_sigint. */ gdb_call_async_signal_handler (async_sigint_remote_twice_token, 0); + sigint_twice = 1; +} + +void target_check_quit_flag (void); + +void +target_check_quit_flag (void) +{ + if (sigint_twice) + { + sigint_twice = 0; + gdb_call_async_signal_handler (async_sigint_remote_twice_token, 1); + } } /* Perform the real interruption of the target execution, in response @@ -5391,20 +5406,12 @@ interrupt_query (void) { target_terminal_ours (); - if (target_is_async_p ()) + if (query (_("Interrupted while waiting for the program.\n\ +Give up (and stop debugging it)? "))) { - signal (SIGINT, handle_sigint); + remote_unpush_target (); quit (); } - else - { - if (query (_("Interrupted while waiting for the program.\n\ -Give up (and stop debugging it)? "))) - { - remote_unpush_target (); - quit (); - } - } target_terminal_inferior (); }