Re: [PATCH] Postpone connecting from gdb_start to gdb_load in native-extended-gdbserver

[Pedro Alves <palves at redhat dot com>]

On 09/04/2012 05:01 AM, Yao Qi wrote:
> Hi,
> When running test cases with board file native-extended-gdbserver.exp
> (to test my 'general async notification' in all-stop mode), I find
> gdb connects to remote in proc gdb_start instead of gdb_load, so that some
> settings in test doesn't happen in remote, because they have to be set
> before GDB connects to remote.  For example,
> 
>   GDB starts up, and connects to remote target.
> 
>   gdb_test_no_output "set target-async on"
>   gdb_test_no_output "set non-stop on"
> 
>   if ![runto_main] {
>      return
>   }
> 
> With current 'native-extended-gdbserver.exp', GDB is started and connects
> to remote before setting these flags, so the remote target is still running
> in sync/all-stop mode, which is not what we want.

Right.  This is a bullet in http://sourceware.org/gdb/wiki/LocalRemoteFeatureParity .

> This patch is to postpone GDB connecting to remote from gdb_start to
> gdb_load, so that we have a chance to set some flags, similar to
> 'native-gdbserver.exp'.

The downside is that this disconnects between runs, which causes
gdbserver to exit (due to --once).  Consider multi-process (+ non-stop)
tests, that will want to "run" more than once in the same session.
This patch would make gdbserver exit between those "run"s.

So, I think it'd be better to solve this some other way.

> Using the patched board file, run testsuite in sync and async mode,
> the test result is improved.
> 
> In async mode:
>  
> -# of expected passes		23370
> -# of unexpected failures	1089
> +# of expected passes		23519
> +# of unexpected failures	1050
>  # of unexpected successes	1
> -# of expected failures		120
> +# of expected failures		119
>  # of unknown successes		1
>  # of known failures		68
> -# of unresolved testcases	8
> 
> In sync mode:
> 
> -# of expected passes		23387
> -# of unexpected failures	1077
> +# of expected passes		23528
> +# of unexpected failures	1042
>  # of unexpected successes	1
> -# of expected failures		120
> +# of expected failures		119
>  # of unknown successes		1
>  # of known failures		67
> -# of unresolved testcases	7
> 
> gdb/testsuite:
> 
> 2012-09-04  Yao Qi  <yao@codesourcery.com>
> 
> 	* boards/native-extended-gdbserver.exp: Set 'gdb,do_reload_on_run'
> 	to 1.
> 	(gdb_start, mi_gdb_start): Remove.
> 	(mi_gdb_load): Invoke mi_gdbserver_start_multi.
> 	* config/extended-gdbserver.exp (gdb_reload): New.
> ---
>  gdb/testsuite/boards/native-extended-gdbserver.exp |   35 ++------------------
>  gdb/testsuite/config/extended-gdbserver.exp        |   12 +++++++
>  2 files changed, 15 insertions(+), 32 deletions(-)
> 
> diff --git a/gdb/testsuite/boards/native-extended-gdbserver.exp b/gdb/testsuite/boards/native-extended-gdbserver.exp
> index e7d3838..1a745f4 100644
> --- a/gdb/testsuite/boards/native-extended-gdbserver.exp
> +++ b/gdb/testsuite/boards/native-extended-gdbserver.exp
> @@ -55,43 +55,14 @@ set_board_info gdb_server_prog "../gdbserver/gdbserver"
>  
>  send_user "configuring for gdbserver local testing (extended-remote)\n";
>  
> +set_board_info gdb,do_reload_on_run 1
> +
>  # We must load this explicitly here, and rename the procedures we want
>  # to override.  If we didn't do this, given that mi-support.exp is
>  # loaded later in the test files, the procedures loaded then would
>  # override our definitions.
>  load_lib mi-support.exp
>  
> -# Overriden in order to start a "gdbserver --multi" instance whenever
> -# GDB is started.  Note nothing is needed for gdb_exit, since
> -# gdbserver is started with --once, causing it to exit once GDB
> -# disconnects.
> -proc gdb_start { } {
> -    # Spawn GDB.
> -    default_gdb_start
> -
> -    # And then GDBserver, ready for extended-remote mode.
> -    gdbserver_start_multi
> -
> -    return 0
> -}
> -
> -# Likewise, for MI.
> -#
> -if { [info procs extended_gdbserver_mi_gdb_start] == "" } {
> -    rename mi_gdb_start extended_gdbserver_mi_gdb_start
> -}
> -proc mi_gdb_start { args } {
> -    # Spawn GDB.
> -    set res [extended_gdbserver_mi_gdb_start $args]
> -    if { $res } {
> -	return $res
> -    }
> -
> -    # And then GDBserver, ready for extended-remote mode.
> -    mi_gdbserver_start_multi
> -    return 0
> -}
> -
>  # Overriden in order to set the remote exec-file whenever a file is
>  # loaded to gdb.
>  #
> @@ -134,7 +105,7 @@ proc mi_gdb_load { arg } {
>  	    perror "couldn't set the remote exec-file (timed out)."
>  	}
>      }
> -
> +    mi_gdbserver_start_multi
>      return 0
>  }
>  
> diff --git a/gdb/testsuite/config/extended-gdbserver.exp b/gdb/testsuite/config/extended-gdbserver.exp
> index 7166b66..0a1278f 100644
> --- a/gdb/testsuite/config/extended-gdbserver.exp
> +++ b/gdb/testsuite/config/extended-gdbserver.exp
> @@ -25,3 +25,15 @@ load_lib gdbserver-support.exp
>  proc gdb_reconnect { } {
>      return [gdbserver_reconnect]
>  }
> +
> +proc gdb_reload { } {
> +    global gdb_prompt
> +
> +    # Disconnect from remote target.
> +    send_gdb "disconnect\n"
> +    gdb_expect {
> +	-re ".*$gdb_prompt $" {
> +	}
> +    }
> +    return [gdbserver_start_multi]
> +}
> 


-- 
Pedro Alves