Make native gdbserver boards no longer be "remote" (in DejaGnu terms)

Message ID 1508178928-30820-1-git-send-email-palves@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves Oct. 16, 2017, 6:35 p.m. UTC
  This commit finally clears the "isremote" flag in the native-gdbserver
and native-stdio-gdbserver boards.  The goal is to make all "native"
boards be considered not remote in DejaGnu terms, like the
native-extended-gdbserver board is too.

DejaGnu automatically considers boards remote if their names don't
match the local hostname.  That means that native-gdbserver and
native-extended-gdbserver are considered remote by default by DejaGnu,
even though they run locally.  native-extended-gdbserver, however,
overrides its isremote flag to force it to be not remote.  So we are
in that weird state where native-gdbserver is considered remote, and
native-extended-gdbserver is considered not remote.

A recent set of commits fixed all the problems (and some more) exposed
by testing with --target_board=native-gdbserver and
--target_board=native-stdio-gdbserver with isremote forced off on
x86-64 GNU/Linux.  I believe we're good to go now.

The native-stdio-gdbserver.exp/remote-stdio-gdbserver.exp boards
required deep non-obvious modifications unfortunately...  The problem
is that if a board is not remote, then DejaGnu doesn't call
${board}_spawn / ${board}_exec at all, and the
native-stdio-gdbserver.exp board relies on those procedures being
called.  To fix that, this commit redesigns how the stdio boards hook
into the testing framework to spawn gdbserver.  IMO, this is a good
change anyway, because the way its done currently is a bit of a hack,
and the result turns out to be simpler, even.  With this commit, they
now no longer load the "gdbserver" generic config, and hook at the
mi_gdb_target_load/gdb_reload level instead, making them more like
traditional board files.

To share code between native-stdio-gdbserver.exp and
remote-stdio-gdbserver.exp, a new shared stdio-gdbserver-base.exp file
is created.

Instead of having each native board clear isremote manually, boards
source the new "local-base.exp" file.

Simon, I've migrated the text you had in the commit log of your
version of this change to the testsuite/README file, so that we can
refer to it.  WDYT?

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
	    Simon Marchi  <simon.marchi@polymtl.ca>

	* README (Local vs Remote vs Native): New section.
	* boards/local-base.exp: New file, with bits factored out from ...
	* boards/native-extended-gdbserver.exp: ... here.  Load
	"local-board".
	* boards/native-gdbserver.exp: Load "local-board".
	(${board}_spawn, ${board}_exec): Delete.
	* boards/native-stdio-gdbserver.exp: Most contents factored out to
	...
	* boards/stdio-gdbserver-base.exp: ... this new file.
	* boards/native-stdio-gdbserver.exp: Reimplement, by loading
	"stdio-gdbserver-base" and defining a get_target_remote_pipe_cmd
	procedure.
	* boards/remote-stdio-gdbserver.exp: Load stdio-gdbserver-base
	instead of native-stdio-gdbserver.  Don't set gdb_server_prog nor
	stdio_gdbserver_command.
	(${board}_get_remote_address, ${board}_get_comm_port)
	(${board}_download, ${board}_upload): Delete.
	(get_target_remote_pipe_cmd): New.
---
 gdb/testsuite/README                               | 45 ++++++++++++++
 gdb/testsuite/boards/local-base.exp                | 24 ++++++++
 gdb/testsuite/boards/native-extended-gdbserver.exp |  9 +--
 gdb/testsuite/boards/native-gdbserver.exp          | 25 +-------
 gdb/testsuite/boards/native-stdio-gdbserver.exp    | 70 ++--------------------
 gdb/testsuite/boards/remote-stdio-gdbserver.exp    | 31 ++--------
 gdb/testsuite/boards/stdio-gdbserver-base.exp      | 54 +++++++++++++++++
 7 files changed, 134 insertions(+), 124 deletions(-)
 create mode 100644 gdb/testsuite/boards/local-base.exp
 create mode 100644 gdb/testsuite/boards/stdio-gdbserver-base.exp
  

Comments

Simon Marchi Oct. 16, 2017, 6:57 p.m. UTC | #1
On 2017-10-16 02:35 PM, Pedro Alves wrote:
> This commit finally clears the "isremote" flag in the native-gdbserver
> and native-stdio-gdbserver boards.  The goal is to make all "native"
> boards be considered not remote in DejaGnu terms, like the
> native-extended-gdbserver board is too.
> 
> DejaGnu automatically considers boards remote if their names don't
> match the local hostname.  That means that native-gdbserver and
> native-extended-gdbserver are considered remote by default by DejaGnu,
> even though they run locally.  native-extended-gdbserver, however,
> overrides its isremote flag to force it to be not remote.  So we are
> in that weird state where native-gdbserver is considered remote, and
> native-extended-gdbserver is considered not remote.
> 
> A recent set of commits fixed all the problems (and some more) exposed
> by testing with --target_board=native-gdbserver and
> --target_board=native-stdio-gdbserver with isremote forced off on
> x86-64 GNU/Linux.  I believe we're good to go now.
> 
> The native-stdio-gdbserver.exp/remote-stdio-gdbserver.exp boards
> required deep non-obvious modifications unfortunately...  The problem
> is that if a board is not remote, then DejaGnu doesn't call
> ${board}_spawn / ${board}_exec at all, and the
> native-stdio-gdbserver.exp board relies on those procedures being
> called.  To fix that, this commit redesigns how the stdio boards hook
> into the testing framework to spawn gdbserver.  IMO, this is a good
> change anyway, because the way its done currently is a bit of a hack,
> and the result turns out to be simpler, even.  With this commit, they
> now no longer load the "gdbserver" generic config, and hook at the
> mi_gdb_target_load/gdb_reload level instead, making them more like
> traditional board files.
> 
> To share code between native-stdio-gdbserver.exp and
> remote-stdio-gdbserver.exp, a new shared stdio-gdbserver-base.exp file
> is created.
> 
> Instead of having each native board clear isremote manually, boards
> source the new "local-base.exp" file.
> 
> Simon, I've migrated the text you had in the commit log of your
> version of this change to the testsuite/README file, so that we can
> refer to it.  WDYT?

That looks fine to me.  Good idea to put that in the README, it will be
easier to refer people to it.  I didn't test remote-stdio-gdbserver though,
but I did test native-stdio-gdbserver.

Thanks for doing this!

Simon
  
Pedro Alves Oct. 16, 2017, 7:26 p.m. UTC | #2
On 10/16/2017 07:57 PM, Simon Marchi wrote:

> That looks fine to me.  Good idea to put that in the README, it will be
> easier to refer people to it.  I didn't test remote-stdio-gdbserver though,
> but I did test native-stdio-gdbserver.
> 
> Thanks for doing this!
> 

Alright, this is now in!

/me crosses fingers

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/testsuite/README b/gdb/testsuite/README
index 037a340..446346e 100644
--- a/gdb/testsuite/README
+++ b/gdb/testsuite/README
@@ -579,3 +579,48 @@  Note that it is also acceptable, and often preferable, to avoid
 running the test at all.  This is the better option if the limitation
 is intrinsic to the environment, rather than a bug expected to be
 fixed in the near future.
+
+Local vs Remote vs Native
+*************************
+
+It's unfortunately easy to get confused in the testsuite about what's
+native and what's not, what's remote and what's not.  The confusion is
+caused by the overlap in vocabulary between DejaGnu and GDB.
+
+From a DejaGnu point of view:
+
+ - native: the host or target board is considered native if the its
+   triplet is the same as the build system's triplet
+
+ - remote: the host or target board is considered remote if it's
+   running on a different machine, and thus require ssh, for example,
+   to run commands, versus simply running commands directly.
+
+Note that they are not mutually exclusive, as you can have a remote
+machine that has the same triplet as the build machine.
+
+From a GDB point of view:
+
+ - native: when GDB uses system calls such as ptrace to interact
+   directly with processes on the same system its running on
+
+ - remote: when GDB speaks the Remote Protocol language with another
+   program doing the ptrace stuff.
+
+Note that they are mutually exclusive.  An inferior can only be either
+debugged with the native target, or with the remote target a specific
+time.
+
+That means that there are cases where the target is not remote for
+DejaGnu, but is remote for GDB (e.g. running gdbserver on the same
+machine).
+
+You can also have a remote target for DejaGnu, but native for GDB
+(e.g.  building on x86 a GDB that runs on ARM and running the
+testsuite with a remote host).
+
+Therefore, care must be taken to check for the right kind of remote.
+Use [is_remote target] to check whether the DejaGnu target board is
+remote.  When what you really want to know is whether GDB is using the
+remote protocol, because feature X is only available when GDB debugs
+natively, check gdb_protocol instead.
diff --git a/gdb/testsuite/boards/local-base.exp b/gdb/testsuite/boards/local-base.exp
new file mode 100644
index 0000000..42aca01
--- /dev/null
+++ b/gdb/testsuite/boards/local-base.exp
@@ -0,0 +1,24 @@ 
+# Copyright 2011-2017 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/>.
+
+# By default, DejaGnu makes the board remote unless the board name
+# matches localhost.  Sourcing this script forces the board to be NOT
+# remote.
+
+global board
+global board_info
+# Remove any target variant specifications from the name.
+set baseboard [lindex [split $board "/"] 0]
+set board_info($baseboard,isremote) 0
diff --git a/gdb/testsuite/boards/native-extended-gdbserver.exp b/gdb/testsuite/boards/native-extended-gdbserver.exp
index 564ffd2..cf0ea47 100644
--- a/gdb/testsuite/boards/native-extended-gdbserver.exp
+++ b/gdb/testsuite/boards/native-extended-gdbserver.exp
@@ -22,14 +22,7 @@ 
 
 load_generic_config "extended-gdbserver"
 load_board_description "gdbserver-base"
-
-# By default, dejagnu makes the board remote unless the board name
-# matches localhost.  Force it to be NOT remote.
-global board
-global board_info
-# Remove any target variant specifications from the name.
-set baseboard [lindex [split $board "/"] 0]
-set board_info($baseboard,isremote) 0
+load_board_description "local-board"
 
 set_board_info sockethost "localhost:"
 
diff --git a/gdb/testsuite/boards/native-gdbserver.exp b/gdb/testsuite/boards/native-gdbserver.exp
index e7c3d15..093289d 100644
--- a/gdb/testsuite/boards/native-gdbserver.exp
+++ b/gdb/testsuite/boards/native-gdbserver.exp
@@ -22,6 +22,7 @@ 
 
 load_generic_config "gdbserver"
 load_board_description "gdbserver-base"
+load_board_description "local-board"
 
 # This gdbserver can only run a process once per session.
 set_board_info gdb,do_reload_on_run 1
@@ -35,27 +36,3 @@  set_board_info exit_is_reliable 1
 
 # We will be using the standard GDB remote protocol.
 set_board_info gdb_protocol "remote"
-
-proc ${board}_spawn { board cmd } {
-    global board_info
-
-    set baseboard [lindex [split $board "/"] 0]
-
-    set board_info($baseboard,isremote) 0
-    set result [remote_spawn $board $cmd]
-    set board_info($baseboard,isremote) 1
-
-    return $result
-}
-
-proc ${board}_exec { hostname program args } {
-    global board_info
-
-    set baseboard [lindex [split $hostname "/"] 0]
-
-    set board_info($baseboard,isremote) 0
-    set result [remote_exec $hostname $program $args]
-    set board_info($baseboard,isremote) 1
-
-    return $result
-}
diff --git a/gdb/testsuite/boards/native-stdio-gdbserver.exp b/gdb/testsuite/boards/native-stdio-gdbserver.exp
index 4ab5b28..f8e7468 100644
--- a/gdb/testsuite/boards/native-stdio-gdbserver.exp
+++ b/gdb/testsuite/boards/native-stdio-gdbserver.exp
@@ -20,70 +20,10 @@ 
 # bash$ cd ${build_dir}/gdb
 # bash$ make check RUNTESTFLAGS="--target_board=native-stdio-gdbserver"
 
-load_generic_config "gdbserver"
-load_board_description "gdbserver-base"
+load_board_description "stdio-gdbserver-base"
 
-# This gdbserver can only run a process once per session.
-set_board_info gdb,do_reload_on_run 1
-
-# There's no support for argument-passing (yet).
-set_board_info noargs 1
-
-# Hack the host,port to pass our peculiar remote connection string.
-set_board_info sockethost ""
-set_board_info gdb,socketport "stdio"
-set_board_info gdb,get_remote_address ${board}_get_remote_address
-set_board_info gdbserver,get_comm_port ${board}_get_comm_port
-
-set_board_info use_gdb_stub 1
-set_board_info exit_is_reliable 1
-
-# We will be using the standard GDB remote protocol.
-set_board_info gdb_protocol "remote"
-
-# Used to pass a value between ${board}_spawn and ${board}_get_remote_address.
-set stdio_gdbserver_command "--unset--"
-
-proc ${board}_get_remote_address { host port } {
-    global stdio_gdbserver_command
-    return "| $stdio_gdbserver_command"
-}
-
-proc ${board}_get_comm_port { port } {
-    return $port
-}
-
-proc ${board}_spawn { board cmd } {
-    global board_info
-
-    verbose -log "${board}_spawn: $board $cmd"
-
-    # Save the command to start gdbserver for later retrieval by
-    # ${board}_get_remote_address.
-    global stdio_gdbserver_command
-    set stdio_gdbserver_command $cmd
-
-    set baseboard [lindex [split $board "/"] 0]
-
-    # We don't spawn gdbserver here, that is done by the subsequent
-    # "target remote | ..." command.
-    set board_info($baseboard,isremote) 0
-    # Pretend as if we've started gdbserver, provide the test harness
-    # with what it's waiting for.
-    set result [remote_spawn $board "echo Listening on stdio"]
-    set board_info($baseboard,isremote) 1
-
-    return $result
-}
-
-proc ${board}_exec { hostname program args } {
-    global board_info
-
-    set baseboard [lindex [split $hostname "/"] 0]
-
-    set board_info($baseboard,isremote) 0
-    set result [remote_exec $hostname $program $args]
-    set board_info($baseboard,isremote) 1
-
-    return $result
+proc get_target_remote_pipe_cmd {} {
+    global last_loaded_file
+    set gdbserver [find_gdbserver]
+    return "$gdbserver --once stdio $last_loaded_file"
 }
diff --git a/gdb/testsuite/boards/remote-stdio-gdbserver.exp b/gdb/testsuite/boards/remote-stdio-gdbserver.exp
index 3d76829..7570dfe 100644
--- a/gdb/testsuite/boards/remote-stdio-gdbserver.exp
+++ b/gdb/testsuite/boards/remote-stdio-gdbserver.exp
@@ -24,7 +24,7 @@ 
 #    REMOTE_USERNAME=... REMOTE_HOSTNAME=... REMOTE_PORTNUM=... \
 #    [REMOTE_TMPDIR=${remote_dir}] [GDBSERVER=${remote_gdbserver}]"
 
-load_board_description "native-stdio-gdbserver"
+load_board_description "stdio-gdbserver-base"
 
 # Test machine info. The generic_config gdbserver reads some of these
 # values from board_info, so this file must set them there.
@@ -56,12 +56,6 @@  if [info exists REMOTE_TMPDIR] {
     set_board_info remotedir $REMOTE_TMPDIR
 }
 
-unset_board_info gdb_server_prog
-set_board_info gdb_server_prog "/usr/bin/gdbserver"
-
-# Used to pass a value between ${board}_spawn and ${board}_get_remote_address.
-set stdio_gdbserver_command "--unset--"
-
 proc get_remote_login { } {
     set result ""
     if {[board_info [target_info name] exists username]} {
@@ -73,27 +67,10 @@  proc get_remote_login { } {
     return $result
 }
 
-proc ${board}_get_remote_address { host port } {
-    global stdio_gdbserver_command
+proc get_target_remote_pipe_cmd { } {
+    set target_exec [gdbserver_download_current_prog]
     set rsh_cmd "[board_info [target_info name] rsh_prog] [get_remote_login]"
-    return "| $rsh_cmd $stdio_gdbserver_command"
-}
-
-proc ${board}_get_comm_port { port } {
-    return $port
-}
-
-proc ${board}_download { board host dest } {
-    if { [board_info [target_info name] exists remotedir] } {
-	set remotedir "[board_info [target_info name] remotedir]/"
-    } else {
-	set remotedir ""
-    }
-    return [standard_download $board $host "$remotedir$dest"]
-}
-
-proc ${board}_upload {dest srcfile args} {
-    return [standard_upload $dest $srcfile $args]
+    return "$rsh_cmd /usr/bin/gdbserver --once stdio $target_exec"
 }
 
 proc ${board}_file { dest op args } {
diff --git a/gdb/testsuite/boards/stdio-gdbserver-base.exp b/gdb/testsuite/boards/stdio-gdbserver-base.exp
new file mode 100644
index 0000000..f3bf25c
--- /dev/null
+++ b/gdb/testsuite/boards/stdio-gdbserver-base.exp
@@ -0,0 +1,54 @@ 
+# Copyright 2011-2017 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 has common bits shared between other dejagnu "board files"
+# that are used to run the testsuite with gdbserver connected via
+# stdio.  Boards that source this must reimplement the
+# get_target_remote_pipe_address procedure.
+
+load_board_description "gdbserver-base"
+
+# Note this is loaded for gdb_target_cmd, not for making this board
+# use the generic "gdbserver" config.
+load_lib gdbserver-support.exp
+
+# This gdbserver can only run a process once per session.
+set_board_info gdb,do_reload_on_run 1
+
+# There's no support for argument-passing (yet).
+set_board_info noargs 1
+
+set_board_info use_gdb_stub 1
+set_board_info exit_is_reliable 1
+
+# We will be using the standard GDB remote protocol.
+set_board_info gdb_protocol "remote"
+
+# Return the CMD string in "target remote | CMD".
+proc get_target_remote_pipe_cmd {} {
+    error "must reimplement this procedure"
+}
+
+proc make_gdbserver_stdio_port {} {
+    return "| [get_target_remote_pipe_cmd]"
+}
+
+proc gdb_reload { } {
+    return [gdb_target_cmd "remote" [make_gdbserver_stdio_port]]
+}
+
+proc mi_gdb_target_load { } {
+    return [mi_gdb_target_cmd "remote" [make_gdbserver_stdio_port]]
+}