Re: [RFC v5 7/8] separate MI and target notions of async

[Pedro Alves <palves@redhat.com>]

On 03/04/2014 06:32 PM, Tom Tromey wrote:
> This separates the MI and target notions of "async".
> 
> Unlike the CLI, MI chose to treat target-async specially -- setting it
> changes the default behavior of commands.  So, we can't get rid of the
> option.  Instead we have to make it MI-only.
> 
> Rather than make the targets always async, this patch introduces a new
> "maint" parameter so that the gdb developer can control whether the
> target is async.  This makes it simpler to debug issues arising only
> in the synchronous mode; important because it sync mode seems unlikely
> to go away.
> 
> Note that "set target-async" also affects this new maint parameter.
> The rationale for this is that then one can easily run the test suite
> in the "maint set target-async off" mode and have tests which enable
> target-async continue to work properly.  This is unusual but, but it
> is a maint command after all, and this behavior is useful.

As discussed, v6 will not have this coupling.  the idea is that
"maint set target-async off" emulates a non-async-capable target,
so "maint set target-async off" + "set target-async on" + "c&"
should result in failure to run a background command, just like
e.g., if one tries "set target-async on" + "c&" on Windows.

In v6, I'm proposing adding an "set mi-async" option, and making
"set target-async" a deprecated alias for "set mi-async" going
forward, to avoid confusion.

> 
> The hardest part of this patch, to my surprise, was getting the MI
> prompt to work properly.  It was reasonably easy, and clean, to get
> close to what the test suite expects; but to fix the last remaining
> failure (mi-async.exp), I had to resort to a hack.

I think I found a cleaner way to do this.  v6 will include it.

>  /* Command implementations.  FIXME: Is this libgdb?  No.  This is the MI
>     layer that calls libgdb.  Any operation used in the below should be
>     formalized.  */
> @@ -260,6 +269,11 @@ exec_continue (char **argv, int argc)
>      {
>        struct cleanup *back_to = make_cleanup_restore_integer (&sched_multi);
>  
> +      /* If MI is in sync mode but the target is async, then
> +	 normal_stop enabled stdin.  We undo the change here.  */

This comment threw me off for a while.  It doesn't matter what
normal_stop did in a previous command.  The code below is actually
correct, in as much as we need to do that before all execution
commands.  We can now just call prepare_execution_command to
do it for us.

> +      if (!target_async_permitted && target_can_async_p ())
> +	async_disable_stdin ();


> diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
> index e158b7e..8cfab3b 100644
> --- a/gdb/testsuite/gdb.mi/mi-cli.exp
> +++ b/gdb/testsuite/gdb.mi/mi-cli.exp
> @@ -134,20 +134,9 @@ mi_gdb_test "500-stack-select-frame 0" \
>    {500\^done} \
>    "-stack-select-frame 0"
>  
> -# When a CLI command is entered in MI session, the respose is different in
> -# sync and async modes. In sync mode normal_stop is called when current
> -# interpreter is CLI. So:
> -#   - print_stop_reason prints stop reason in CLI uiout, and we don't show it
> -#     in MI
> -#   - The stop position is printed, and appears in MI 'console' channel.
> -#
> -# In async mode the stop event is processed when we're back to MI interpreter,
> -# so the stop reason is printed into MI uiout an.
> -if {$async} {
> -    set reason "end-stepping-range"
> -} else {
> -    set reason ""
> -}
> +# Allow a reason, or not.  Which we get depends at least on whether
> +# the target being tested supports async.
> +set reason ".*"

This patch gets rid of this in the opposite direction:

 https://sourceware.org/ml/gdb-patches/2014-05/msg00566.html

>  mi_execute_to "interpreter-exec console step" $reason "callee4" "" ".*basics.c" $line_callee4_next \
>      "" "check *stopped from CLI command"
> diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
> index 213073a..16c7951 100644
> --- a/gdb/testsuite/lib/mi-support.exp
> +++ b/gdb/testsuite/lib/mi-support.exp
> @@ -1096,7 +1096,9 @@ proc mi_expect_stop { reason func args file line extra test } {
>      }
>  
>      set r ""
> -    if { $reason != "" } {
> +    if { $reason == ".*" } {
> +	set r "(?:reason=\".*\",)?"
> +    } elseif { $reason != "" } {
>  	set r "reason=\"$reason\","
>      }
>  

And ends up no longer necessary.

v6 coming up...  Getting close.  :-)

-- 
Pedro Alves