This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]