Re: [RFC][gdb/testsuite] Fix silent timeout in gdb.multi/multi-target.exp

[Pedro Alves <palves@redhat.com>]

Hi,

Thanks for addressing this, and sorry for the delay.

On 3/16/20 5:01 PM, Tom de Vries wrote:
> Hi,
> 
> While running test-case gdb.multi/multi-target.exp, I observed a silent
> timeout related to "monitor exit".
> 
> By making the timeout explicit in an expect clause in gdbserver_gdb_exit:
> ...
> +  timeout {
> +    warning "Timed out waiting for EOF in server after $monitor_exit"
> +  }
> ...
> we get in the log:
> ...
> monitor exit^M
> "monitor" command not supported by this target.^M
> (gdb) WARNING: Timed out waiting for EOF in server after monitor exit
> ...
> 
> What happens is the following:
> - the inferior 5 is selected
> - a breakpoint is set in inferior 1
> - the breakpoint triggers and we switch to inferior 1
> - setup is called by test_continue, which calls clean_restarts, which calls

Typo: clean_restarts -> clean_restart

>   gdbserver_gdb_exit (due to load_lib gdbserver-support.exp)
> - gdbserver_gdb_exit issues "monitor exit"
> - gdb responds with "not supported by this target" because inferior 1 is
>   native
> 
> Fix this by keeping a list of server_spawn_id, and cleaning those up before
> calling gdbserver_gdb_exit.
> 
> This reduces testing time from 1m22s to 32s.
> 
> Any comments?

Looks mostly good to me, though I have a comment below.

> diff --git a/gdb/testsuite/gdb.multi/multi-target.exp b/gdb/testsuite/gdb.multi/multi-target.exp
> index 6c727b5e3b..2e66cb55fa 100644
> --- a/gdb/testsuite/gdb.multi/multi-target.exp
> +++ b/gdb/testsuite/gdb.multi/multi-target.exp
> @@ -33,8 +33,13 @@ if { [prepare_for_testing "failed to prepare" ${binfile} "${srcfile}" \
>      return
>  }
>  
> +# Keep list of server_spawn_id
> +set server_spawn_ids [list]

It'd not a huge deal, but how about making this a list of pairs
of (inferior ID, spawn ID)?  That would make cleanup_gdbservers not have
to hardcode the inferior numbers.  add_inferior, which is
connect_target_extended_remote's caller, has the inferior number
handy already in the NUM parameter, so it'd just need to pass it down
to connect_target_extended_remote.

Thanks,
Pedro Alves