From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29266 invoked by alias); 5 Sep 2011 14:43:57 -0000 Received: (qmail 29257 invoked by uid 22791); 5 Sep 2011 14:43:55 -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; Mon, 05 Sep 2011 14:43:41 +0000 Received: (qmail 10412 invoked from network); 5 Sep 2011 14:43:40 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 5 Sep 2011 14:43:40 -0000 From: Pedro Alves To: Matt Rice Subject: Re: Flip the interpreter to synchronously wait for commands finishing, in command lists and similars Date: Mon, 05 Sep 2011 14:48: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: <201109021807.41992.pedro@codesourcery.com> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201109051543.37663.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-09/txt/msg00080.txt.bz2 On Saturday 03 September 2011 00:28:15, Matt Rice wrote: > On Fri, Sep 2, 2011 at 10:07 AM, Pedro Alves wrote: > > > > With this patch, synchronous execution commands (the regular step, next, > > etc., that is, those with an & suffixed) with a target running > > in async mode work correctly (AFAICT) with all current use cases. We > > can consider flipping on async on by default, and then incrementally > > convert specific paths to state machines, for new use cases, rather than > > delaying flipping on async on by default until _everything_ is converted > > into a state-machine. > > I was hoping this might fix, (but doesn't seem to) the following case: > > ./gdb/gdb -ex 'set target-async on' -ex 'attach 7625' -ex 'continue' > ./gdb/gdb -ex 'set target-async on' -ex 'attach 7625' -ex 'continue&' > > both these commands exhibit: > Continuing. > > Program received signal SIGSTOP, Stopped (signal). > ..... > (gdb) Thanks. An attach ends with a TARGET_SIGNAL_STOP/SIGSTOP, that is normally suppressed by STOP_QUIETLY_NO_SIGSTOP, but we weren't entering the new loop in execute_command because the current thread isn't marked running at that point yet. So, "continue" ran before the attach was complete, and "continue"ing calls clear_proceed_status which wipes inferior->stop_soon. By the time we process the TARGET_SIGNAL_STOP, we can no longer explain it, and so report it as a regular SIGSTOP. I had added the "is_running()" check for the hook-stop-continue.exp test, as otherwise, when we went to go execute the first command in the hook-stop command list, being that a command that doesn't start the target running, we'd enter the loop in execute_command: if (!interpreter_async && sync_execution) { while (gdb_do_one_event () >= 0) if (!sync_execution) break; } but we'd never leave it, hanging in gdb_do_one_event, because the previous command had already finished, and the target was stopped already, but sync_execution was still set. Here's a better fix for that issue, which fixes your example too. Clear sync_execution before running the hook-stop, as soon as the previous sync execution command finishes. This make a hell lot more sense. I don't know why I didn't think of it before :-). Tested it on x86_64-linux (async) and applied. > thus changing the behavior of '-ex continue' should we turn > target-async on by default. ECANTPARSE? -- Pedro Alves 2011-09-05 Pedro Alves gdb/ * inf-loop.c (execute_command): Don't check if the current thread if running before synchronously waiting for command completion. * infrun.c (fetch_inferior_event): Handle "set exec-done-display" here. (normal_stop): Call async_enable_stdin here. * inf-loop.c (inferior_event_handler): Don't call async_enable_stdin, nor handle "set exec-done-display" here. --- gdb/inf-loop.c | 14 -------------- gdb/infrun.c | 17 +++++++++++++++-- gdb/top.c | 2 +- 3 files changed, 16 insertions(+), 17 deletions(-) Index: src/gdb/top.c =================================================================== --- src.orig/gdb/top.c 2011-09-05 15:11:13.063963929 +0100 +++ src/gdb/top.c 2011-09-05 15:11:54.333963936 +0100 @@ -447,7 +447,7 @@ execute_command (char *p, int from_tty) command's list, running command hooks or similars), and we just ran a synchronous command that started the target, wait for that command to end. */ - if (!interpreter_async && sync_execution && is_running (inferior_ptid)) + if (!interpreter_async && sync_execution) { while (gdb_do_one_event () >= 0) if (!sync_execution) Index: src/gdb/infrun.c =================================================================== --- src.orig/gdb/infrun.c 2011-09-05 15:11:12.353963929 +0100 +++ src/gdb/infrun.c 2011-09-05 15:11:54.333963936 +0100 @@ -2713,6 +2713,7 @@ fetch_inferior_event (void *client_data) struct cleanup *old_chain = make_cleanup (null_cleanup, NULL); struct cleanup *ts_old_chain; int was_sync = sync_execution; + int cmd_done = 0; memset (ecs, 0, sizeof (*ecs)); @@ -2804,7 +2805,10 @@ fetch_inferior_event (void *client_data) && ecs->event_thread->control.stop_step) inferior_event_handler (INF_EXEC_CONTINUE, NULL); else - inferior_event_handler (INF_EXEC_COMPLETE, NULL); + { + inferior_event_handler (INF_EXEC_COMPLETE, NULL); + cmd_done = 1; + } } /* No error, don't finish the thread states yet. */ @@ -2814,9 +2818,17 @@ fetch_inferior_event (void *client_data) do_cleanups (old_chain); /* If the inferior was in sync execution mode, and now isn't, - restore the prompt. */ + restore the prompt (a synchronous execution command has finished, + and we're ready for input). */ if (interpreter_async && was_sync && !sync_execution) display_gdb_prompt (0); + + if (cmd_done + && !was_sync + && exec_done_display_p + && (ptid_equal (inferior_ptid, null_ptid) + || !is_running (inferior_ptid))) + printf_unfiltered (_("completed.\n")); } /* Record the frame and location we're currently stepping through. */ @@ -5814,6 +5826,7 @@ normal_stop (void) goto done; target_terminal_ours (); + async_enable_stdin (); /* Set the current source location. This will also happen if we display the frame below, but the current SAL will be incorrect Index: src/gdb/inf-loop.c =================================================================== --- src.orig/gdb/inf-loop.c 2011-09-05 15:11:12.353963929 +0100 +++ src/gdb/inf-loop.c 2011-09-05 15:11:54.333963936 +0100 @@ -41,7 +41,6 @@ void inferior_event_handler (enum inferior_event_type event_type, gdb_client_data client_data) { - int was_sync = 0; struct cleanup *cleanup_if_error = make_bpstat_clear_actions_cleanup (); switch (event_type) @@ -63,7 +62,6 @@ inferior_event_handler (enum inferior_ev break; case INF_EXEC_COMPLETE: - if (!non_stop) { /* Unregister the inferior from the event loop. This is done @@ -73,12 +71,6 @@ inferior_event_handler (enum inferior_ev target_async (NULL, 0); } - /* The call to async_enable_stdin below resets 'sync_execution'. - However, if sync_execution is 1 now, we also need to show the - prompt below, so save the current value. */ - was_sync = sync_execution; - async_enable_stdin (); - /* Do all continuations associated with the whole inferior (not a particular thread). */ if (!ptid_equal (inferior_ptid, null_ptid)) @@ -129,12 +121,6 @@ inferior_event_handler (enum inferior_ev } exception_print (gdb_stderr, e); } - - if (!was_sync - && exec_done_display_p - && (ptid_equal (inferior_ptid, null_ptid) - || !is_running (inferior_ptid))) - printf_unfiltered (_("completed.\n")); break; case INF_EXEC_CONTINUE: