Re: [PATCH] mi_async_p: Use the default run target (PR gdb/18077)

[Pedro Alves <palves at redhat dot com>]

On 03/03/2015 09:45 PM, Simon Marchi wrote:
> When using -exec-run in mi-async mode on a fresh gdb launch, we can see
> that it is not actually done asynchronously.
> 
> The problem is that when we issue -exec-run, the linux native target is
> not pushed yet. So when the code in mi_cmd_exec-run checks if we support
> async (by calling mi_async_p), tdefault_can_async_p from the dummy
> target answers 0.
> 
> I am not certain of the conceptual correctness of this solution, but it
> seems to work. It changes mi_async_p so that it uses find_run_target()
> instead of using the current_target. When -exec-run is used before the
> native target is pushed, mi_async_p will now report that the target that
> will eventually be used for running supports async, instead of saying
> that the current target (dummy) does not.

This is not correct.  E.g., when some target is already pushed,
and it's one that does support async, but can't "run", in other places
that we use mi_async_p we should be consulting the already connected
target, not fallback to the run target.
Please make sure to test with native and gdbserver in both
remote and extended remote modes, to cover different modes of
operation, though you're likely not seeing an issue with
"target remote", which does not support "run", just because that
does implement t->to_create_inferior, but that's for
extended-remote, really (see find_run_target).

I think we need to make run_one_inferior itself check whether the
run target can async, instead of using mi_async_p() there.  Likewise
for

Note that the "mi_async && target_can_async_p()" checks intend to
mimic GDB's behavior before target-async was the default.  In order
gdb's, if you did "set target-async on" and then
-exec-run/continue/step/whatever, gdb would just ignore the target-async
request.  This is actually documented:

 On some targets, @value{GDBN} is capable of processing MI commands
 even while the target is running.  This is called @dfn{asynchronous
 command execution} (@pxref{Background Execution}).  The frontend may
 specify a preferrence for asynchronous execution using the
 @code{-gdb-set mi-async 1} command, which should be emitted before
 either running the executable or attaching to the target.  After the
 frontend has started the executable or attached to the target, it can
 find if asynchronous execution is enabled using the
 @code{-list-target-features} command.

I think it'd be cleaner if when "set mi-async on" (the new spelling of
"set target-async on", it's just an alias) is in effect with a target
that really CANNOT async, we'd still make -exec-run try to run
in async mode, which would error out just like if the user does
"run&" on such a target, but alas, that's not how previous gdb
releases worked.  We'd need to hear from frontend developers whether
that's a desirable change.

> 
> I added a small testcase that I copied from mi-async.exp. Please
> indicate if you think it should be integrated to an existing test rather
> than in a new test.
> 
> I have two questions regarding the test:
> 
>  - Why do we have mi_expect_stop and mi_expect_interrupt? It seems like
>    the functionality of _interrupt could be integrated in _stop.

So we can cope with differences like: ...

>  - The signal reported when interrupting a thread changes when in non-stop vs all-stop:
> 
>    non-stop: *stopped,reason="signal-received",signal-name="0",signal-meaning="Signal 0",..
>    all-stop: *stopped,reason="signal-received",signal-name="SIGINT",signal-meaning="Interrupt",...

... these?

> 
>    As a consequence, mi_expect_interrupt only works with non-stop.

Could you fix it?  I think expecting both signals would be fine.

> +#include <unistd.h>
> +
> +int main ()

int
main (void)


> +# The purpose of this test if to verify that -exec-run with mi-async on
> +# results in asynchronous execution (PR 18077).
> +
> +# The plan is for async mode to become the default but toggle for now.
> +set saved_gdbflags $GDBFLAGS
> +set GDBFLAGS [concat $GDBFLAGS " -ex \"set mi-async on\""]
> +

Can you make this a regular "-gdb-set mi-async", after GDB is
started, please?  (You'll then need to call mi_detect_async too.)

> +load_lib mi-support.exp
> +
> +gdb_exit
> +if [mi_gdb_start] {
> +    continue
> +}
> +
> +standard_testfile
> +
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> +     untested mi-async.exp
> +     return -1
> +}
> +
> +mi_delete_breakpoints
> +mi_gdb_reinitialize_dir $srcdir/$subdir
> +mi_gdb_load ${binfile}
> +
> +# Necessary for mi_expect_interrupt to work, as the reported signal is not the
> +# same in all-stop.
> +mi_gdb_test "-gdb-set non-stop 1" ".*"
> +
> +proc linux_async_run_test {} {

Drop the "linux" prefix, please.

> +    global mi_gdb_prompt
> +    global hex

These don't appear to be used.

> +
> +    mi_run_cmd
> +    mi_gdb_test "123-exec-interrupt --all" "123\\^done" "send interrupt command"
> +    mi_expect_interrupt "expect interrupt"
> +}
> +
> +linux_async_run_test
> +
> +mi_gdb_exit
> +
> +set GDBFLAGS $saved_gdbflags
> +
> +return 0
> 

Thanks,
Pedro Alves