Re: [RFC] new board file 'remote-host-native.exp'

Pedro Alves
2012-07-24T16:08:58Z

On 07/24/2012 02:10 PM, Yao Qi wrote:
> Hi, when fixing other PR, I need to make sure test case running well on
> 'remote host' as well. I write a new board file remote-host-native.exp
> so that we can run gdb testsuite in 'remote-host' locally.
>
> When run testsuite for 'remote-host', I get following error,
>
> ERROR: tcl error sourcing ../../../../git/gdb/testsuite/gdb.base/break.exp.
> ERROR: wrong # args: should be "gdb_download filename"
> while executing
> "gdb_download "remote-host" ../../../../git/gdb/testsuite/gdb.base/break.c break.c"
> ("eval" body line 1)
> invoked from within
> "eval ${try}_${proc} \"$dest\" $args"
> (procedure "call_remote" line 66)
> invoked from within
> "call_remote "" download $dest $file $destfile"
> (procedure "remote_download" line 30)
> invoked from within
> "remote_download host $x"
> (procedure "default_target_compile" line 323)
> invoked from within
> "default_target_compile $source $destfile $type $options"
> (procedure "target_compile" line 6)
> invoked from within
> "target_compile $source $dest $type $options"
> (procedure "gdb_compile" line 142)
> invoked from within
> "gdb_compile "${srcdir}/${subdir}/${s}" "${binfile}${i}.o" object $options"
>
> As we can see, dejagnu looks proc ${board}_download, and matches gdb_download. However,

I'm confused at why ${board} expanded to gdb, the tool.

> we can't simply change the parameters of gdb_download to meet the required parameters.
> This will cause an endless recurisive call "remote_download->call_remote->gdb_download
> ->remote_download". This patch simply renames this proc to gdb_download_1.
>
> As this patch applied, there are a lot of fails when running testsuite with
> board file remote-host-native.exp. Some of them are intermittent, and
> some of them reflect the flaws in test cases.
>
> gdb/testsuite:
>
> 2012-07-24 Yao Qi <yao@codesourcery.com>
>
> * boards/remote-host-native.exp: New board file.
> * lib/gdb.exp: Rename gdb_downlad to gdb_download_1.
> (gdb_load_shlibs): Caller update.
> * lib/gdbserver-support.exp (gdbserver_download_current_prog):
> Likewise.
> * lib/mi-support.exp (mi_load_shlibs): Likewise.
> * gdb.base/jit-so.exp, gdb.base/jit.exp: Likewise.
> ---
> gdb/testsuite/boards/remote-host-native.exp | 60 +++++++++++++++++++++++++++
> gdb/testsuite/gdb.base/jit-so.exp | 2 +-
> gdb/testsuite/gdb.base/jit.exp | 2 +-
> gdb/testsuite/lib/gdb.exp | 6 +-
> gdb/testsuite/lib/gdbserver-support.exp | 2 +-
> gdb/testsuite/lib/mi-support.exp | 2 +-
> 6 files changed, 67 insertions(+), 7 deletions(-)
> create mode 100644 gdb/testsuite/boards/remote-host-native.exp
>
> diff --git a/gdb/testsuite/boards/remote-host-native.exp b/gdb/testsuite/boards/remote-host-native.exp
> new file mode 100644
> index 0000000..cdb0684
> --- /dev/null
> +++ b/gdb/testsuite/boards/remote-host-native.exp
> @@ -0,0 +1,60 @@
> +# Copyright 2012 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# This file is a dejagnu "board file" and is used to run the testsuite
> +# natively in remote host.

Why natively? I'd think you could use both this host board with e.g.,
a gdbserver target board file. I suggest:

+# This file is a dejagnu "board file" and is used to run the testsuite
+# against local host, in remote host mode.

I'd even rename the file to local-remote-host.exp.

> +#
> +# To use this file:
> +# bash$ touch ${my_dejagnu_dir}/my-dejagnu.exp
> +# bash$ export DEJAGNU=${my_dejagnu_dir}/my-dejagnu.exp
> +# bash$ mkdir ${my_dejagnu_dir}/boards
> +# bash$ cp ${src_dir}/gdb/testsuite/boards/remote-host-native.exp \
> +# ${my_dejagnu_dir}/boards
> +
> +# Modify the location of GDB and the username to access your box
> +# below.
> +
> +# bash$ cd ${build_dir}/gdb
> +# bash$ make check RUNTESTFLAGS="--host_board=remote-host-native"
> +
> +load_generic_config "gdb"

Why is this necessary? Why "gdb", and not "unix" ? What is "gdb"
actually loading? Isn't this "gdb" the reason remote_download maps
to gdb_download ? Otherwise, that gdb_download issue should have tripped
up other configurations doing remote host testing, I'd think.

> +
> +set_board_info gdb_protocol "standard"

Why is this necessary? A host board file should only specify the bits
for downloading and executing bits on the host, I think.

> +
> +global GDB
> +# Specify the location of GDB binary.
> +set GDB "/your/built/gdb"

How about we make this pick up the just built GDB ?

set GDB [file join [pwd] "../gdb"]

> +
> +set_board_info compiler "[find_gcc]"

Is this necessary in a host board file?

> +set_board_info hostname 127.0.0.1
> +
> +set_board_info username YOUR.USER.NAME

We can make this pick up the current user by default on at least GNU/Linux,
and probably other Unixen:

set_board_info username $::env(USER)

> +
> +# The ssh key should be correctly set up that you ssh to 127.0.0.1
> +# without having to type password.
> +set_board_info rsh_prog /usr/bin/ssh
> +set_board_info rcp_prog /usr/bin/scp
> +set_board_info file_transfer "rsh"

> +
> +proc ${board}_download { board src dest } {
> +
> + # If file name is a relative, convert it to absolute, otherwise file can't
> + # be found on host, because the current directory usually is /home/$USER.
> + if { [file pathtype $src] == "relative" } {
> + return [file join [pwd] $src]
> + } else {
> + return $src
> + }
> +}

Hmm, so the intention is to bypass the download to the host. I'd like a small
comment above the function that says something to the effect.

> diff --git a/gdb/testsuite/gdb.base/jit-so.exp b/gdb/testsuite/gdb.base/jit-so.exp
> index 36f18ad..95c2eac 100644
> --- a/gdb/testsuite/gdb.base/jit-so.exp
> +++ b/gdb/testsuite/gdb.base/jit-so.exp
> @@ -61,7 +61,7 @@ if { [gdb_compile_shlib ${solib_srcfile} ${solib_binfile} {}] != "" } {
> return -1
> }
>
> -set solib_binfile_target [gdb_download ${solib_binfile}]
> +set solib_binfile_target [gdb_download_1 ${solib_binfile}]

Someone might well in the future notice there's a "gdb_download_1" overload
but no "gdb_download", and decide to "clean up" things by renaming "gdb_download_1"
back to "gdb_download"... I'd prefer a better name. But, in any case, I'm confused
at why this board trips on this, while other remote host testing CS does, does not.

--
Pedro Alves