This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 3/2] Do not bpstat_clear_actions on throw_exception - async fixes
- From: Pedro Alves <pedro at codesourcery dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 24 Aug 2011 11:33:46 +0100
- Subject: Re: [patch 3/2] Do not bpstat_clear_actions on throw_exception - async fixes
- References: <20110823203520.GB4325@host1.jankratochvil.net>
[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 <jan.kratochvil@redhat.com>
>
> * 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