Implement ${board}_{exec,spawn} for native-extended-gdbserver
Commit Message
Hi,
Mark Wielaard is kindly running two Debian buildslaves in our
buildbot, and he noticed that, when testing things under the
native-extended-gdbserver board, there is a huge slowndown due to
several timeouts that are happening.
If you look at the following log:
<http://gdb-build.sergiodj.net/builders/Debian-i686-native-extended-gdbserver/builds/48/steps/test%20gdb/logs/stdio>
You will notice that this happens because dejagnu is trying to open a
rsh connection to test native-extended-gdbserver, which of course is
wrong, and we end up not testing things properly.
After some investigation (motivated by an idea from Pedro), I found
that our native-extended-gdbserver board does not set the "isremote"
flag properly. I fixed that by doing two things:
- Defining the ${board}_spawn procedure inside the board file, and
- Moving the ${board}_exec procedure (present at the native-gdbserver
and native-stdio-gdbserver boards) to the gdbserver-base file.
After that, I am not seeing this failure anymore.
Is this OK to check in?
gdb/testsuite/ChangeLog:
2015-02-03 Sergio Durigan Junior <sergiodj@redhat.com>
* boards/gdbserver-base.exp (${board}_exec): New procedure, moved
from native-gdbserver.exp and native-stdio-gdbserver.exp.
* boards/native-extended-gdbserver.exp: Do not set isremote using
board_info.
(${board}_spawn): New procedure.
* boards/native-gdbserver.exp (${board}_exec): Moved to
gdbserver-base.exp.
* boards/native-stdio-gdbserver.exp (${board}_exec): Likewise.
---
gdb/testsuite/boards/gdbserver-base.exp | 12 ++++++++++++
gdb/testsuite/boards/native-extended-gdbserver.exp | 18 ++++++++++++------
gdb/testsuite/boards/native-gdbserver.exp | 12 ------------
gdb/testsuite/boards/native-stdio-gdbserver.exp | 12 ------------
4 files changed, 24 insertions(+), 30 deletions(-)
Comments
On 02/04/2015 12:20 AM, Sergio Durigan Junior wrote:
> diff --git a/gdb/testsuite/boards/native-extended-gdbserver.exp b/gdb/testsuite/boards/native-extended-gdbserver.exp
> index 57503db..0059c4e 100644
> --- a/gdb/testsuite/boards/native-extended-gdbserver.exp
> +++ b/gdb/testsuite/boards/native-extended-gdbserver.exp
> @@ -23,12 +23,6 @@
> 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
> -set board_info($board,isremote) 0
I don't think we can/should remove this. That'll make
"is_remote" checks all over the testsuite return true,
and thus skip tests that weren't skipped before. E.g.,
solib-display.exp.
I think we need to instead do here what ${board}_spawn
is already doing. That is, remove any target variant
specifications from $board, like:
--- a/gdb/testsuite/boards/native-extended-gdbserver.exp
+++ b/gdb/testsuite/boards/native-extended-gdbserver.exp
@@ -27,7 +27,9 @@ load_board_description "gdbserver-base"
# matches localhost. Force it to be NOT remote.
global board
global board_info
-set board_info($board,isremote) 0
+# Remove any target variant specifications from the name.
+set baseboard [lindex [split $board "/"] 0]
+set board_info($baseboard,isremote) 0
I'm testing that.
Thanks,
Pedro Alves
Pedro Alves <palves@redhat.com> writes:
> On 02/04/2015 12:20 AM, Sergio Durigan Junior wrote:
>
>> diff --git a/gdb/testsuite/boards/native-extended-gdbserver.exp b/gdb/testsuite/boards/native-extended-gdbserver.exp
>> index 57503db..0059c4e 100644
>> --- a/gdb/testsuite/boards/native-extended-gdbserver.exp
>> +++ b/gdb/testsuite/boards/native-extended-gdbserver.exp
>> @@ -23,12 +23,6 @@
>> 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
>> -set board_info($board,isremote) 0
>
> I don't think we can/should remove this. That'll make
> "is_remote" checks all over the testsuite return true,
> and thus skip tests that weren't skipped before. E.g.,
> solib-display.exp.
>
> I think we need to instead do here what ${board}_spawn
> is already doing. That is, remove any target variant
> specifications from $board, like:
>
> --- a/gdb/testsuite/boards/native-extended-gdbserver.exp
> +++ b/gdb/testsuite/boards/native-extended-gdbserver.exp
> @@ -27,7 +27,9 @@ load_board_description "gdbserver-base"
> # matches localhost. Force it to be NOT remote.
> global board
> global board_info
> -set board_info($board,isremote) 0
> +# Remove any target variant specifications from the name.
> +set baseboard [lindex [split $board "/"] 0]
> +set board_info($baseboard,isremote) 0
>
> I'm testing that.
Eewwww....
[not your patch, just the fact that $board ends up with entire string,
including, e.g., /-m32]
We should document this somewhere, as I can imagine it being
all too easy to continue to just use $board without realizing this.
A little grepping makes me wonder.
E.g., all the procs with ${board} as a prefix.
bash$ grep -F '${board}' */*.exp
proc ${board}_file { dest op args } {
...
and sure enough, those procs get "/-m32" in the name too.
I added
verbose -log "[info procs *${board}*]" to gdbserver-base.exp
and got
native-gdbserver/-m32_file native-gdbserver/-m32_download native-gdbserver/-m32_upload
Yikes.
btw, The name "baseboard" is a bit confusing because I immediately
think of /usr/share/dejagnu/baseboards, which is a different thing.
@@ -50,3 +50,15 @@ proc ${board}_download { board host dest } {
proc ${board}_upload {dest srcfile args} {
return $srcfile
}
+
+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
+}
@@ -23,12 +23,6 @@
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
-set board_info($board,isremote) 0
-
set_board_info sockethost "localhost:"
# We will be using the extended GDB remote protocol.
@@ -119,3 +113,15 @@ proc mi_gdb_load { arg } {
return 0
}
+
+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
+}
@@ -47,15 +47,3 @@ proc ${board}_spawn { board cmd } {
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
-}
@@ -75,15 +75,3 @@ proc ${board}_spawn { board cmd } {
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
-}