From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24953 invoked by alias); 24 Aug 2011 10:34:40 -0000 Received: (qmail 24375 invoked by uid 22791); 24 Aug 2011 10:34:39 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 24 Aug 2011 10:33:50 +0000 Received: (qmail 6350 invoked from network); 24 Aug 2011 10:33:49 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 24 Aug 2011 10:33:49 -0000 From: Pedro Alves To: Jan Kratochvil Subject: Re: [patch 3/2] Do not bpstat_clear_actions on throw_exception - async fixes Date: Wed, 24 Aug 2011 10:34:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-11-generic; KDE/4.7.0; x86_64; ; ) Cc: gdb-patches@sourceware.org References: <20110823203520.GB4325@host1.jankratochvil.net> In-Reply-To: <20110823203520.GB4325@host1.jankratochvil.net> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201108241133.46693.pedro@codesourcery.com> X-IsSubscribed: yes 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 X-SW-Source: 2011-08/txt/msg00435.txt.bz2 [Hi Jan, adding the list back, it looks like you just forgot it] On Tuesday 23 August 2011 21:35:20, Jan Kratochvil wrote: > Hi, > > while trying to fix the async mode from: > Re: [patch 2/2] Do not bpstat_clear_actions on throw_exception #3 > http://sourceware.org/ml/gdb-patches/2011-08/msg00424.html > > I have found some errors are now silent - suppressed - in the async mode. > Fix it here. > > Not going to check in that `gdb_test_no_output "set target-async on"' part as > I guess one should run the whole testsuite in async mode as Pedro did for: > [WIP/FYI] fix remaining problems with target async > http://sourceware.org/ml/gdb-patches/2011-06/msg00158.html > > > Thanks, > Jan > > > gdb/ > 2011-08-23 Jan Kratochvil > > * inf-loop.c (fetch_inferior_event_wrapper): Remove declaration. > (fetch_inferior_event_error_cleanup): New function, with code from ... > (inferior_event_handler): ... here of INF_REG_EVENT, drop the > bpstat_clear_actions call, register it by make_cleanup instead, remove > catch_errors. Add exception_print in INF_EXEC_COMPLETE. Hmm, I don't understand the "drop the bpstat_clear_actions call" part? Isn't that undoing your last change to the previous patch? > (fetch_inferior_event_wrapper): Remove. > > --- a/gdb/inf-loop.c > +++ b/gdb/inf-loop.c > @@ -30,7 +30,16 @@ > #include "gdbthread.h" > #include "continuations.h" > > -static int fetch_inferior_event_wrapper (gdb_client_data client_data); > +/* Executed only if fetch_inferior_event throws an exception. */ > + > +static void > +fetch_inferior_event_error_cleanup (void *unused) > +{ > + do_all_intermediate_continuations (1); > + do_all_continuations (1); > + async_enable_stdin (); > + display_gdb_prompt (0); > +} > > /* General function to handle events in the inferior. So far it just > takes care of detecting errors reported by select() or poll(), > @@ -47,19 +56,14 @@ inferior_event_handler (enum inferior_event_type event_type, > switch (event_type) > { > case INF_REG_EVENT: > - /* Use catch errors for now, until the inner layers of > - fetch_inferior_event (i.e. readchar) can return meaningful > - error status. If an error occurs while getting an event from > - the target, just cancel the current command. */ > - if (!catch_errors (fetch_inferior_event_wrapper, > - client_data, "", RETURN_MASK_ALL)) > - { > - bpstat_clear_actions (); > - do_all_intermediate_continuations (1); > - do_all_continuations (1); > - async_enable_stdin (); > - display_gdb_prompt (0); > - } > + /* Catch exceptions for now by pushing the error case to > + cleanup_if_error, until the inner layers of fetch_inferior_event (i.e. > + readchar) can return meaningful error status. If an error occurs > + while getting an event from the target, just cancel the current > + command. */ > + > + make_cleanup (fetch_inferior_event_error_cleanup, NULL); > + fetch_inferior_event (client_data); This now lets exceptions go all the way back to start_event_loop, correct? I think we'll call async_enable_stdin and display_gdb_prompt twice then, possibly printing a double prompt. I guess removing those calls from fetch_inferior_event_error_cleanup would fix it. But I don't understand the desire to mix this change with ... > break; > > case INF_EXEC_COMPLETE: > @@ -121,6 +125,7 @@ inferior_event_handler (enum inferior_event_type event_type, > { > bpstat_do_actions (); > } > + exception_print (gdb_stderr, e); ... this one though. Isn't this bit the only thing missing, and the rest would be considered just a cleanup? What am I missing? > > if (!was_sync > && exec_done_display_p > @@ -147,10 +152,3 @@ inferior_event_handler (enum inferior_event_type event_type, > > discard_cleanups (cleanup_if_error); > } > - > -static int > -fetch_inferior_event_wrapper (gdb_client_data client_data) > -{ > - fetch_inferior_event (client_data); > - return 1; > -} > --- a/gdb/testsuite/gdb.base/commands.exp > +++ b/gdb/testsuite/gdb.base/commands.exp > @@ -740,6 +740,7 @@ proc error_clears_commands_left {} { > } > gdb_test_no_output "end" "main commands 2c" > > + gdb_test_no_output "set target-async on" > gdb_run_cmd > gdb_test "" "\r\nhook-stop1\r\n.*\r\ncmd1\r\nUndefined command: \"errorcommandxy\"\\. Try \"help\"\\." "cmd1 error" > > -- Pedro Alves