This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] mi_async_p: Use the default run target (PR gdb/18077)
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Pedro Alves <palves at redhat dot com>, <gdb-patches at sourceware dot org>
- Date: Wed, 4 Mar 2015 14:29:20 -0500
- Subject: Re: [PATCH] mi_async_p: Use the default run target (PR gdb/18077)
- Authentication-results: sourceware.org; auth=none
- References: <1425419133-7843-1-git-send-email-simon dot marchi at ericsson dot com> <54F6DBE4 dot 6080206 at redhat dot com>
Hi Pedro,
On 03/04/2015 05:18 AM, Pedro Alves wrote:
> 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
Is it possible that the last paragraph and the next one are missing some
parts? I'd like to have the complete information before I try to answer
something intelligible :).
> 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
>