[PATCH] Testsuite: Add gdbserver sysroot test

[PATCH] Testsuite: Add gdbserver sysroot test

[Alan Hayward]

The local board file ensures that the sysroot is always set to load
files from the local filesystem.

Add a gdbserver test to explicitly test the sysroot set to both the
remote target and the local filesystem.

gdb/testsuite/ChangeLog:

2019-04-08  Alan Hayward  <alan.hayward@arm.com>

	* gdb.server/sysroot.c: New test.
	* gdb.server/sysroot.exp: New file.
---
 gdb/testsuite/gdb.server/sysroot.c   | 25 +++++++++++++
 gdb/testsuite/gdb.server/sysroot.exp | 55 ++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)
 create mode 100644 gdb/testsuite/gdb.server/sysroot.c
 create mode 100644 gdb/testsuite/gdb.server/sysroot.exp

diff --git a/gdb/testsuite/gdb.server/sysroot.c b/gdb/testsuite/gdb.server/sysroot.c
new file mode 100644
index 0000000000..6fc1443e3b
--- /dev/null
+++ b/gdb/testsuite/gdb.server/sysroot.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 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/>.  */
+
+#include <stdio.h>
+
+int
+main ()
+{
+  printf("Hello World!\n");
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.server/sysroot.exp b/gdb/testsuite/gdb.server/sysroot.exp
new file mode 100644
index 0000000000..9db833fc7e
--- /dev/null
+++ b/gdb/testsuite/gdb.server/sysroot.exp
@@ -0,0 +1,55 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2019 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/>.
+
+# Test GDB can correct read the binary with different sysroot setups.
+
+load_lib gdbserver-support.exp
+
+if { [skip_gdbserver_tests] } {
+    verbose "skipping gdbserver tests"
+    return -1
+}
+
+standard_testfile
+if [prepare_for_testing "failed to prepare" $testfile $srcfile "additional_flags=--no-builtin"] {
+    return -1
+}
+
+# Run once with sysroot set to the remote target and once to the local filesystem.
+foreach_with_prefix sysroot {"target:" "/"} {
+
+    # Make sure we're disconnected, in case we're testing with an
+    # extended-remote board, therefore already connected.
+    gdb_test "disconnect" ".*"
+
+    # Start GDBserver.
+    set res [gdbserver_start "" $binfile]
+    set gdbserver_protocol [lindex $res 0]
+    set gdbserver_gdbport [lindex $res 1]
+
+    # Set the sysroot.
+    gdb_test_no_output "set sysroot $sysroot"
+
+    gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
+
+    gdb_breakpoint main
+    gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
+
+    # Test we can stop inside a library.
+    gdb_breakpoint printf
+    gdb_test "continue" "Breakpoint $decimal.* printf.*" "continue to printf"
+}
-- 
2.20.1 (Apple Git-117)

Re: [PATCH] Testsuite: Add gdbserver sysroot test

[Pedro Alves]

Hi Alan,

Thanks for following through.

On 4/8/19 4:34 PM, Alan Hayward wrote:
> The local board file ensures that the sysroot is always set to load
> files from the local filesystem.
> 
> Add a gdbserver test to explicitly test the sysroot set to both the
> remote target and the local filesystem.
> 
> gdb/testsuite/ChangeLog:
> 
> 2019-04-08  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* gdb.server/sysroot.c: New test.
> 	* gdb.server/sysroot.exp: New file.
> ---
>  gdb/testsuite/gdb.server/sysroot.c   | 25 +++++++++++++
>  gdb/testsuite/gdb.server/sysroot.exp | 55 ++++++++++++++++++++++++++++
>  2 files changed, 80 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.server/sysroot.c
>  create mode 100644 gdb/testsuite/gdb.server/sysroot.exp
> 
> diff --git a/gdb/testsuite/gdb.server/sysroot.c b/gdb/testsuite/gdb.server/sysroot.c
> new file mode 100644
> index 0000000000..6fc1443e3b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.server/sysroot.c
> @@ -0,0 +1,25 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 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/>.  */
> +
> +#include <stdio.h>
> +
> +int
> +main ()
> +{
> +  printf("Hello World!\n");
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.server/sysroot.exp b/gdb/testsuite/gdb.server/sysroot.exp
> new file mode 100644
> index 0000000000..9db833fc7e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.server/sysroot.exp
> @@ -0,0 +1,55 @@
> +# This testcase is part of GDB, the GNU debugger.
> +#
> +# Copyright 2019 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/>.
> +
> +# Test GDB can correct read the binary with different sysroot setups.
> +
> +load_lib gdbserver-support.exp
> +
> +if { [skip_gdbserver_tests] } {
> +    verbose "skipping gdbserver tests"
> +    return -1
> +}
> +
> +standard_testfile
> +if [prepare_for_testing "failed to prepare" $testfile $srcfile "additional_flags=--no-builtin"] {
> +    return -1
> +}
> +
> +# Run once with sysroot set to the remote target and once to the local filesystem.
> +foreach_with_prefix sysroot {"target:" "/"} {
> +
> +    # Make sure we're disconnected, in case we're testing with an
> +    # extended-remote board, therefore already connected.
> +    gdb_test "disconnect" ".*"
> +
> +    # Start GDBserver.
> +    set res [gdbserver_start "" $binfile]
> +    set gdbserver_protocol [lindex $res 0]
> +    set gdbserver_gdbport [lindex $res 1]
> +
> +    # Set the sysroot.
> +    gdb_test_no_output "set sysroot $sysroot"
> +
> +    gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
> +
> +    gdb_breakpoint main
> +    gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
> +
> +    # Test we can stop inside a library.
> +    gdb_breakpoint printf
> +    gdb_test "continue" "Breakpoint $decimal.* printf.*" "continue to printf"
> +}
> 

I'm not sure I understand the overall approach you took here.

Because, if you don't restart between each iteration, then the
second iteration is using the program from the previous connection,
I think?

Also, you've used prepare_for_testing, which results in loading the
program into gdb with the "file" command.

So the main program is not being fetched from the target.

So the testcase is trying to ensure that we load the DSOs from
the target, right?

But the thing is, even if you don't have debug info for shared libraries, if you
debug info for the main program, you'll be able to set a breakpoint on "printf".
The result is you end up with a breakpoint at printf@plt.  So I'm thinking that
the test would pass even if we failed to load the shared libraries from the target.

I tried to do that manually, by issuing a "nosharedlibrary" after connecting
to gdbserver, and then running to the breakpoint, but unfortunately, that
runs into a nasty gdb bug:

 (gdb) nosharedlibrary 
 (gdb) c
 Continuing.
 pure virtual method called
 terminate called without an active exception
 Aborted (core dumped)
 $

I'm looking into that...

Also, the test as is fails for me, on x86-64:

 set sysroot /
 warning: Could not load shared library symbols for linux-vdso.so.1.
 Do you need "set solib-search-path" or "set sysroot"?
 (gdb) FAIL: gdb.server/sysroot.exp: sysroot=/: set sysroot /

Thanks,
Pedro Alves

Re: [PATCH] Testsuite: Add gdbserver sysroot test

[Alan Hayward]

> On 8 Apr 2019, at 19:58, Pedro Alves <alves.ped@gmail.com> wrote:
> 
> Hi Alan,
> 
> Thanks for following through.
> 
>> 
> 
> I'm not sure I understand the overall approach you took here.
> 
> Because, if you don't restart between each iteration, then the
> second iteration is using the program from the previous connection,
> I think?

Yes :(
Fixed.

> 
> Also, you've used prepare_for_testing, which results in loading the
> program into gdb with the "file" command.
> 
> So the main program is not being fetched from the target.

I didn’t spot the “file” part. All the libraries do get fetched though.

Fixed.

> 
> So the testcase is trying to ensure that we load the DSOs from
> the target, right?

I did wonder if I should add checks to test for the "Reading * from
remote target...” vs "Reading symbols from *...”. But, we can’t really
know in advance which libraries will be read.

But now I’ve removed the “file” command, I can add a check to make sure
that the binary is correctly read in.

I’ve overriden gdb_target_cmd locally as I didn’t think it was worth moving the
changed version into the library. 

> 
> But the thing is, even if you don't have debug info for shared libraries, if you
> debug info for the main program, you'll be able to set a breakpoint on "printf".
> The result is you end up with a breakpoint at printf@plt.  So I'm thinking that
> the test would pass even if we failed to load the shared libraries from the target.
> 

I just wanted to make sure we could stop somewhere outside the binary.
Any suggestions? Or is is best to just remove that part.


> I tried to do that manually, by issuing a "nosharedlibrary" after connecting
> to gdbserver, and then running to the breakpoint, but unfortunately, that
> runs into a nasty gdb bug:
> 
> (gdb) nosharedlibrary 
> (gdb) c
> Continuing.
> pure virtual method called
> terminate called without an active exception
> Aborted (core dumped)
> $
> 
> I'm looking into that…

I get the same error for that.

> 
> Also, the test as is fails for me, on x86-64:
> 
> set sysroot /
> warning: Could not load shared library symbols for linux-vdso.so.1.
> Do you need "set solib-search-path" or "set sysroot"?
> (gdb) FAIL: gdb.server/sysroot.exp: sysroot=/: set sysroot /

Works for me on:
Ubuntu 16.04, both AArch64 and x86-64

Just tried it on:
Centos 7.4 AArch64
OpenSuse 13.3 AArch64

And works there too. But I don’t see anything with “vdso” in any of the logs.

What OS are you running?



With all the above changes, I now have:


diff --git a/gdb/testsuite/gdb.server/sysroot.c b/gdb/testsuite/gdb.server/sysroot.c
new file mode 100644
index 0000000000..6fc1443e3b
--- /dev/null
+++ b/gdb/testsuite/gdb.server/sysroot.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 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/>.  */
+
+#include <stdio.h>
+
+int
+main ()
+{
+  printf("Hello World!\n");
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.server/sysroot.exp b/gdb/testsuite/gdb.server/sysroot.exp
new file mode 100644
index 0000000000..2429a4caee
--- /dev/null
+++ b/gdb/testsuite/gdb.server/sysroot.exp
@@ -0,0 +1,134 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2019 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/>.
+
+# Test GDB can correct read the binary with different sysroot setups.
+
+load_lib gdbserver-support.exp
+
+if { [skip_gdbserver_tests] } {
+    verbose "skipping gdbserver tests"
+    return -1
+}
+
+standard_testfile
+if {[build_executable "failed to prepare" $testfile $srcfile "additional_flags=--no-builtin"] == -1} {
+    return -1
+}
+
+# Override gdb_target_cmd, with additional flag $additional_text which must
+# exist in the output of the target connection for the function to pass.
+proc gdb_target_cmd { targetname serialport additional_text } {
+    global gdb_prompt
+
+    set serialport_re [string_to_regexp $serialport]
+    for {set i 1} {$i <= 3} {incr i} {
+       send_gdb "target $targetname $serialport\n"
+       gdb_expect 60 {
+           -re "A program is being debugged already.*ill it.*y or n. $" {
+               send_gdb "y\n"
+               exp_continue
+           }
+           -re "unknown host.*$gdb_prompt" {
+               verbose "Couldn't look up $serialport"
+           }
+           -re "Couldn't establish connection to remote.*$gdb_prompt $" {
+               verbose "Connection failed"
+           }
+           -re "Remote MIPS debugging.*$additional_text.*$gdb_prompt" {
+               verbose "Set target to $targetname"
+               return 0
+           }
+           -re "Remote debugging using .*$serialport_re.*$additional_text.*$gdb_prompt $" {
+               verbose "Set target to $targetname"
+               return 0
+           }
+           -re "Remote debugging using stdio.*$additional_text.*$gdb_prompt $" {
+               verbose "Set target to $targetname"
+               return 0
+           }
+           -re "Remote target $targetname connected to.*$additional_text.*$gdb_prompt $" {
+               verbose "Set target to $targetname"
+               return 0
+           }
+           -re "Connected to.*$additional_text.*$gdb_prompt $" {
+               verbose "Set target to $targetname"
+               return 0
+           }
+           -re "Ending remote.*$gdb_prompt $" { }
+           -re "Connection refused.*$gdb_prompt $" {
+               verbose "Connection refused by remote target.  Pausing, and trying again."
+               sleep 30
+               continue
+           }
+           -re "Timeout reading from remote system.*$gdb_prompt $" {
+               verbose "Got timeout error from gdb."
+           }
+           -notransfer -re "Remote debugging using .*\r\n> $" {
+               # We got an unexpected prompt while creating the target.
+               # Leave it there for the test to diagnose.
+               return 1
+           }
+           timeout {
+               send_gdb "^C"
+               break
+           }
+       }
+    }
+    return 1
+}
+
+# Run once with sysroot set to the local filesystem and once set to the remote target.
+foreach_with_prefix sysroot { "local" "remote" } {
+    global srcdir
+    global subdir
+    global binfile
+
+    if { $sysroot == "local" } {
+       set sysroot_command "/"
+       set reading_symbols "Reading symbols from $binfile..."
+    } else {
+       set sysroot_command "target:"
+       set reading_symbols "Reading $binfile from remote target..."
+    }
+
+    # Restart GDB
+    gdb_exit
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+
+    # Make sure we're disconnected, in case we're testing with an
+    # extended-remote board, therefore already connected.
+    gdb_test "disconnect $reading_symbols" ".*"
+
+    # Start GDBserver.
+    set res [gdbserver_start "" $binfile]
+    set gdbserver_protocol [lindex $res 0]
+    set gdbserver_gdbport [lindex $res 1]
+
+    # Set the sysroot.
+    gdb_test_no_output "set sysroot $sysroot_command"
+
+    # Connect to gdbsever, making sure GDB reads in the binary correctly.
+    gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport $reading_symbols
+
+    gdb_breakpoint main
+    gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
+
+    # Test we can stop inside a library.
+    gdb_breakpoint printf
+    gdb_test "continue" "Breakpoint $decimal.* printf.*" "continue to printf"
+}

Re: [PATCH] Testsuite: Add gdbserver sysroot test

[Pedro Alves]

On 4/9/19 12:04 PM, Alan Hayward wrote:

> I’ve overriden gdb_target_cmd locally as I didn’t think it was worth moving the
> changed version into the library. 

That doesn't seem like a good idea to me.  Likely people will forget to keep the
new copy in sync if/when the main copy changes.

Plus, try --target_board=native-extended-remote, and you'll see:

Running /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.server/sysroot.exp ...
ERROR: tcl error sourcing /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.server/sysroot.exp.
ERROR: wrong # args: should be "gdb_target_cmd targetname serialport additional_text"
    while executing
"gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport"
    (procedure "gdbserver_start_multi" line 12)
    invoked from within
"gdbserver_start_multi"
    (procedure "gdb_start" line 6)
    invoked from within
...

Why not add the new parameter to the lib proc, but make it optional, like?

 proc gdb_target_cmd { targetname serialport {additional_text ""} } {

> 
>>
>> But the thing is, even if you don't have debug info for shared libraries, if you
>> debug info for the main program, you'll be able to set a breakpoint on "printf".
>> The result is you end up with a breakpoint at printf@plt.  So I'm thinking that
>> the test would pass even if we failed to load the shared libraries from the target.
>>
> 
> I just wanted to make sure we could stop somewhere outside the binary.
> Any suggestions? Or is is best to just remove that part.

Maybe just add an empty space after "printf", like:

 -    gdb_test "continue" "Breakpoint $decimal.* printf.*" "continue to printf"
 +    gdb_test "continue" "Breakpoint $decimal.* printf .*" "continue to printf"

so that it doesn't match printf@plt.

> 
> 
>> I tried to do that manually, by issuing a "nosharedlibrary" after connecting
>> to gdbserver, and then running to the breakpoint, but unfortunately, that
>> runs into a nasty gdb bug:
>>
>> (gdb) nosharedlibrary 
>> (gdb) c
>> Continuing.
>> pure virtual method called
>> terminate called without an active exception
>> Aborted (core dumped)
>> $
>>
>> I'm looking into that…
> 
> I get the same error for that.

Fix for that posted:
  https://sourceware.org/ml/gdb-patches/2019-04/msg00141.html

> 
>>
>> Also, the test as is fails for me, on x86-64:
>>
>> set sysroot /
>> warning: Could not load shared library symbols for linux-vdso.so.1.
>> Do you need "set solib-search-path" or "set sysroot"?
>> (gdb) FAIL: gdb.server/sysroot.exp: sysroot=/: set sysroot /
> 
> Works for me on:
> Ubuntu 16.04, both AArch64 and x86-64
> 
> Just tried it on:
> Centos 7.4 AArch64
> OpenSuse 13.3 AArch64
> 
> And works there too. But I don’t see anything with “vdso” in any of the logs.
> 
> What OS are you running?
> 

x86-64 Fedora 27.  But the new version passes, except for the
extended-remote issue above.
> +
> +int
> +main ()
> +{
> +  printf("Hello World!\n");

Missing space before parens.

> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.server/sysroot.exp b/gdb/testsuite/gdb.server/sysroot.exp
> new file mode 100644

> +
> +# Test GDB can correct read the binary with different sysroot setups.

"Test THAT GDB can correctLY" ?

I'd suggest extending that a bit, to mention shared libraries, and "target:":

 Test that GDB can correctly read the binary and shared libraries
 with different sysroot setups: local, and "target:".

> +
> +# Run once with sysroot set to the local filesystem and once set to the remote target.

Hit M-q in your emacs here.

> +foreach_with_prefix sysroot { "local" "remote" } {
> +    global srcdir
> +    global subdir
> +    global binfile
> +
> +    if { $sysroot == "local" } {
> +       set sysroot_command "/"
> +       set reading_symbols "Reading symbols from $binfile..."
> +    } else {
> +       set sysroot_command "target:"
> +       set reading_symbols "Reading $binfile from remote target..."
> +    }
> +
> +    # Restart GDB
> +    gdb_exit
> +    gdb_start
> +    gdb_reinitialize_dir $srcdir/$subdir

Use clean_restart, and add period:

    # Restart GDB.
    clean_restart

> +
> +    # Make sure we're disconnected, in case we're testing with an
> +    # extended-remote board, therefore already connected.
> +    gdb_test "disconnect $reading_symbols" ".*"
> +
> +    # Start GDBserver.
> +    set res [gdbserver_start "" $binfile]
> +    set gdbserver_protocol [lindex $res 0]
> +    set gdbserver_gdbport [lindex $res 1]
> +
> +    # Set the sysroot.
> +    gdb_test_no_output "set sysroot $sysroot_command"
> +
> +    # Connect to gdbsever, making sure GDB reads in the binary correctly.
> +    gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport $reading_symbols
> +
> +    gdb_breakpoint main
> +    gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
> +
> +    # Test we can stop inside a library.

 "Test that" ?

> +    gdb_breakpoint printf
> +    gdb_test "continue" "Breakpoint $decimal.* printf.*" "continue to printf"
> +}
Thanks,
Pedro Alves

Re: [PATCH] Testsuite: Add gdbserver sysroot test

[Alan Hayward]

> On 9 Apr 2019, at 15:35, Pedro Alves <palves@redhat.com> wrote:
> 
> On 4/9/19 12:04 PM, Alan Hayward wrote:
> 
>> I’ve overriden gdb_target_cmd locally as I didn’t think it was worth moving the
>> changed version into the library. 
> 
> That doesn't seem like a good idea to me.  Likely people will forget to keep the
> new copy in sync if/when the main copy changes.

Agreed. But was balancing that against adding the additional param for just the
single use, which I doubted anything else would need.

> 
> Plus, try --target_board=native-extended-remote, and you'll see:
> 
> Running /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.server/sysroot.exp ...
> ERROR: tcl error sourcing /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.server/sysroot.exp.
> ERROR: wrong # args: should be "gdb_target_cmd targetname serialport additional_text"
>    while executing
> "gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport"
>    (procedure "gdbserver_start_multi" line 12)
>    invoked from within
> "gdbserver_start_multi"
>    (procedure "gdb_start" line 6)
>    invoked from within
> ...
> 

....but given this errors, then yes, I’ll change the common version.


> Why not add the new parameter to the lib proc, but make it optional, like?
> 
> proc gdb_target_cmd { targetname serialport {additional_text ""} } {
> 

Done.


>> 
>>> 
>>> But the thing is, even if you don't have debug info for shared libraries, if you
>>> debug info for the main program, you'll be able to set a breakpoint on "printf".
>>> The result is you end up with a breakpoint at printf@plt.  So I'm thinking that
>>> the test would pass even if we failed to load the shared libraries from the target.
>>> 
>> 
>> I just wanted to make sure we could stop somewhere outside the binary.
>> Any suggestions? Or is is best to just remove that part.
> 
> Maybe just add an empty space after "printf", like:
> 
> -    gdb_test "continue" "Breakpoint $decimal.* printf.*" "continue to printf"
> +    gdb_test "continue" "Breakpoint $decimal.* printf .*" "continue to printf"
> 
> so that it doesn't match printf@plt.

Done.

> 
>> 
>> 
>>> I tried to do that manually, by issuing a "nosharedlibrary" after connecting
>>> to gdbserver, and then running to the breakpoint, but unfortunately, that
>>> runs into a nasty gdb bug:
>>> 
>>> (gdb) nosharedlibrary 
>>> (gdb) c
>>> Continuing.
>>> pure virtual method called
>>> terminate called without an active exception
>>> Aborted (core dumped)
>>> $
>>> 
>>> I'm looking into that…
>> 
>> I get the same error for that.
> 
> Fix for that posted:
>  https://sourceware.org/ml/gdb-patches/2019-04/msg00141.html

Excellent. I would give it a review, but don’t know anything in that area.

> 
>> 
>>> 
>>> Also, the test as is fails for me, on x86-64:
>>> 
>>> set sysroot /
>>> warning: Could not load shared library symbols for linux-vdso.so.1.
>>> Do you need "set solib-search-path" or "set sysroot"?
>>> (gdb) FAIL: gdb.server/sysroot.exp: sysroot=/: set sysroot /
>> 
>> Works for me on:
>> Ubuntu 16.04, both AArch64 and x86-64
>> 
>> Just tried it on:
>> Centos 7.4 AArch64
>> OpenSuse 13.3 AArch64
>> 
>> And works there too. But I don’t see anything with “vdso” in any of the logs.
>> 
>> What OS are you running?
>> 
> 
> x86-64 Fedora 27.  But the new version passes, except for the
> extended-remote issue above.

Hmmm, odd.

>> +
>> +int
>> +main ()
>> +{
>> +  printf("Hello World!\n");
> 
> Missing space before parens.

Done.

> 
>> +  return 0;
>> +}
>> diff --git a/gdb/testsuite/gdb.server/sysroot.exp b/gdb/testsuite/gdb.server/sysroot.exp
>> new file mode 100644
> 
>> +
>> +# Test GDB can correct read the binary with different sysroot setups.
> 
> "Test THAT GDB can correctLY" ?
> 
> I'd suggest extending that a bit, to mention shared libraries, and "target:":
> 
> Test that GDB can correctly read the binary and shared libraries
> with different sysroot setups: local, and "target:".
> 
>> +
>> +# Run once with sysroot set to the local filesystem and once set to the remote target.
> 
> Hit M-q in your emacs here.

Done (assuming you just meant to wrap before column 80).

> 
>> +foreach_with_prefix sysroot { "local" "remote" } {
>> +    global srcdir
>> +    global subdir
>> +    global binfile
>> +
>> +    if { $sysroot == "local" } {
>> +       set sysroot_command "/"
>> +       set reading_symbols "Reading symbols from $binfile..."
>> +    } else {
>> +       set sysroot_command "target:"
>> +       set reading_symbols "Reading $binfile from remote target..."
>> +    }
>> +
>> +    # Restart GDB
>> +    gdb_exit
>> +    gdb_start
>> +    gdb_reinitialize_dir $srcdir/$subdir
> 
> Use clean_restart, and add period:
> 
>    # Restart GDB.
>    clean_restart

This was intentional to avoid the “file” command at the end of clean_restart.

> 
>> +
>> +    # Make sure we're disconnected, in case we're testing with an
>> +    # extended-remote board, therefore already connected.
>> +    gdb_test "disconnect $reading_symbols" ".*"
>> +
>> +    # Start GDBserver.
>> +    set res [gdbserver_start "" $binfile]
>> +    set gdbserver_protocol [lindex $res 0]
>> +    set gdbserver_gdbport [lindex $res 1]
>> +
>> +    # Set the sysroot.
>> +    gdb_test_no_output "set sysroot $sysroot_command"
>> +
>> +    # Connect to gdbsever, making sure GDB reads in the binary correctly.
>> +    gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport $reading_symbols
>> +
>> +    gdb_breakpoint main
>> +    gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
>> +
>> +    # Test we can stop inside a library.
> 
> "Test that” ?

Done.


> 
>> +    gdb_breakpoint printf
>> +    gdb_test "continue" "Breakpoint $decimal.* printf.*" "continue to printf"
>> +}
> Thanks,
> Pedro Alves


With latest changes:


    gdb/testsuite/ChangeLog:

    2019-04-10  Alan Hayward  <alan.hayward@arm.com>

            * gdb.server/sysroot.c: New test.
            * gdb.server/sysroot.exp: New file.
            * lib/gdbserver-support.exp (gdb_target_cmd): Add additional text
            matching param.

diff --git a/gdb/testsuite/gdb.server/sysroot.c b/gdb/testsuite/gdb.server/sysroot.c
new file mode 100644
index 0000000000..7db1a138d1
--- /dev/null
+++ b/gdb/testsuite/gdb.server/sysroot.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 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/>.  */
+
+#include <stdio.h>
+
+int
+main ()
+{
+  printf ("Hello World!\n");
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.server/sysroot.exp b/gdb/testsuite/gdb.server/sysroot.exp
new file mode 100644
index 0000000000..dbd548ba2b
--- /dev/null
+++ b/gdb/testsuite/gdb.server/sysroot.exp
@@ -0,0 +1,79 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2019 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/>.
+
+# Test that GDB can correctly read the binary and shared libraries
+# with different sysroot setups: local and "target:".
+
+load_lib gdbserver-support.exp
+
+if { [skip_gdbserver_tests] } {
+    verbose "skipping gdbserver tests"
+    return -1
+}
+
+standard_testfile
+if {[build_executable "failed to prepare" $testfile $srcfile "additional_flags=--no-builtin"] == -1} {
+    return -1
+}
+
+# Run once with sysroot set to the local filesystem and once set to the remote
+# target.
+foreach_with_prefix sysroot { "local" "remote" } {
+    global srcdir
+    global subdir
+    global binfile
+
+    if { $sysroot == "local" } {
+       set sysroot_command "/"
+       set reading_symbols "Reading symbols from $binfile..."
+    } else {
+       set sysroot_command "target:"
+       set reading_symbols "Reading $binfile from remote target..."
+    }
+
+    # Restart GDB
+    gdb_exit
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+
+    # Make sure we're disconnected, in case we're testing with an
+    # extended-remote board, therefore already connected.
+    gdb_test "disconnect" ".*"
+
+    # Start GDBserver.
+    set res [gdbserver_start "" $binfile]
+    set gdbserver_protocol [lindex $res 0]
+    set gdbserver_gdbport [lindex $res 1]
+
+    # Set the sysroot.
+    gdb_test_no_output "set sysroot $sysroot_command"
+
+    # Connect to gdbsever, making sure GDB reads in the binary correctly.
+    set test "connect to remote and read binary"
+    if {[gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport $reading_symbols] == 0} {
+       pass $test
+    } else {
+       fail $test
+    }
+
+    gdb_breakpoint main
+    gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
+
+    # Test that we can stop inside a library.
+    gdb_breakpoint printf
+    gdb_test "continue" "Breakpoint $decimal.* printf .*" "continue to printf"
+}
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index dbd885aa22..1343169488 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -42,9 +42,11 @@

 #
 # gdb_target_cmd
-# Send gdb the "target" command
+# Send gdb the "target" command. Returns 0 on success, 1 on failure.
+# If specified, then additional_text must match the text which must comes after
+# the connection message in order for procedure to succeed.
 #
-proc gdb_target_cmd { targetname serialport } {
+proc gdb_target_cmd { targetname serialport {additional_text ""} } {
     global gdb_prompt

     set serialport_re [string_to_regexp $serialport]
@@ -61,23 +63,23 @@ proc gdb_target_cmd { targetname serialport } {
            -re "Couldn't establish connection to remote.*$gdb_prompt $" {
                verbose "Connection failed"
            }
-           -re "Remote MIPS debugging.*$gdb_prompt" {
+           -re "Remote MIPS debugging.*$additional_text.*$gdb_prompt" {
                verbose "Set target to $targetname"
                return 0
            }
-           -re "Remote debugging using .*$serialport_re.*$gdb_prompt $" {
+           -re "Remote debugging using .*$serialport_re.*$additional_text.*$gdb_prompt $" {
                verbose "Set target to $targetname"
                return 0
            }
-           -re "Remote debugging using stdio.*$gdb_prompt $" {
+           -re "Remote debugging using stdio.*$additional_text.*$gdb_prompt $" {
                verbose "Set target to $targetname"
                return 0
            }
-           -re "Remote target $targetname connected to.*$gdb_prompt $" {
+           -re "Remote target $targetname connected to.*$additional_text.*$gdb_prompt $" {
                verbose "Set target to $targetname"
                return 0
            }
-           -re "Connected to.*$gdb_prompt $" {
+           -re "Connected to.*$additional_text.*$gdb_prompt $" {
                verbose "Set target to $targetname"
                return 0
            }

Re: [PATCH] Testsuite: Add gdbserver sysroot test

[Pedro Alves]

On 4/10/19 1:10 PM, Alan Hayward wrote:

>> Fix for that posted:
>>  https://sourceware.org/ml/gdb-patches/2019-04/msg00141.html
> 
> Excellent. I would give it a review, but don’t know anything in that area.

Review is often an excellent way to learn about new areas.  :-)

>>
>> Hit M-q in your emacs here.
> 
> Done (assuming you just meant to wrap before column 80).

Right.
>> Use clean_restart, and add period:
>>
>>    # Restart GDB.
>>    clean_restart
> 
> This was intentional to avoid the “file” command at the end of clean_restart.

The "file" command at the end is only run if you pass an argument to clean_restart.

    if { [llength $args] >= 1 } {
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	set executable [lindex $args 0]
	set binfile [standard_output_file ${executable}]
	gdb_load ${binfile}
    }

Please do add the all-important period.  :-)

> +
> +    # Set the sysroot.
> +    gdb_test_no_output "set sysroot $sysroot_command"
> +
> +    # Connect to gdbsever, making sure GDB reads in the binary correctly.

type: "gdbsever"

> +    set test "connect to remote and read binary"

> 
>  #
>  # gdb_target_cmd
> -# Send gdb the "target" command
> +# Send gdb the "target" command. Returns 0 on success, 1 on failure.

Double space after period.

> +# If specified, then additional_text must match the text which must comes after

Uppercase ADDITIONAL_TEXT.

s/which must comes/that comes/ ?

> +# the connection message in order for procedure to succeed.

s/for procedure/for the procedure/.

OK with the issues mentioned above fixed.

Thanks,
Pedro Alves

Re: [PATCH] Testsuite: Add gdbserver sysroot test

[Alan Hayward]

> On 11 Apr 2019, at 17:11, Pedro Alves <palves@redhat.com> wrote:
> 
> On 4/10/19 1:10 PM, Alan Hayward wrote:
> 
>>> Fix for that posted:
>>> https://sourceware.org/ml/gdb-patches/2019-04/msg00141.html
>> 
>> Excellent. I would give it a review, but don’t know anything in that area.
> 
> Review is often an excellent way to learn about new areas.  :-)

True :)

> 
>>> 
>>> Hit M-q in your emacs here.
>> 
>> Done (assuming you just meant to wrap before column 80).
> 
> Right.
>>> Use clean_restart, and add period:
>>> 
>>>   # Restart GDB.
>>>   clean_restart
>> 
>> This was intentional to avoid the “file” command at the end of clean_restart.
> 
> The "file" command at the end is only run if you pass an argument to clean_restart.
> 
>    if { [llength $args] >= 1 } {
>    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 	set executable [lindex $args 0]
> 	set binfile [standard_output_file ${executable}]
> 	gdb_load ${binfile}
>    }

Yes! Done.

> Please do add the all-important period.  :-)

And done.

> 
>> +
>> +    # Set the sysroot.
>> +    gdb_test_no_output "set sysroot $sysroot_command"
>> +
>> +    # Connect to gdbsever, making sure GDB reads in the binary correctly.
> 
> type: “gdbsever"

Done.

> 
>> +    set test "connect to remote and read binary"
> 
>> 
>> #
>> # gdb_target_cmd
>> -# Send gdb the "target" command
>> +# Send gdb the "target" command. Returns 0 on success, 1 on failure.
> 
> Double space after period.
> 

Done.

>> +# If specified, then additional_text must match the text which must comes after
> 
> Uppercase ADDITIONAL_TEXT.
> 
> s/which must comes/that comes/ ?

Done.

> 
>> +# the connection message in order for procedure to succeed.
> 
> s/for procedure/for the procedure/.


Done.

> 
> OK with the issues mentioned above fixed.
> 
> Thanks,
> Pedro Alves

Pushed the following:


diff --git a/gdb/testsuite/gdb.server/sysroot.c b/gdb/testsuite/gdb.server/sysroot.c
new file mode 100644
index 0000000000..7db1a138d1
--- /dev/null
+++ b/gdb/testsuite/gdb.server/sysroot.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 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/>.  */
+
+#include <stdio.h>
+
+int
+main ()
+{
+  printf ("Hello World!\n");
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.server/sysroot.exp b/gdb/testsuite/gdb.server/sysroot.exp
new file mode 100644
index 0000000000..4b95fdf087
--- /dev/null
+++ b/gdb/testsuite/gdb.server/sysroot.exp
@@ -0,0 +1,77 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2019 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/>.
+
+# Test that GDB can correctly read the binary and shared libraries
+# with different sysroot setups: local and "target:".
+
+load_lib gdbserver-support.exp
+
+if { [skip_gdbserver_tests] } {
+    verbose "skipping gdbserver tests"
+    return -1
+}
+
+standard_testfile
+if {[build_executable "failed to prepare" $testfile $srcfile "additional_flags=--no-builtin"] == -1} {
+    return -1
+}
+
+# Run once with sysroot set to the local filesystem and once set to the remote
+# target.
+foreach_with_prefix sysroot { "local" "remote" } {
+    global srcdir
+    global subdir
+    global binfile
+
+    if { $sysroot == "local" } {
+       set sysroot_command "/"
+       set reading_symbols "Reading symbols from $binfile..."
+    } else {
+       set sysroot_command "target:"
+       set reading_symbols "Reading $binfile from remote target..."
+    }
+
+    # Restart GDB.
+    clean_restart
+
+    # Make sure we're disconnected, in case we're testing with an
+    # extended-remote board, therefore already connected.
+    gdb_test "disconnect" ".*"
+
+    # Start GDBserver.
+    set res [gdbserver_start "" $binfile]
+    set gdbserver_protocol [lindex $res 0]
+    set gdbserver_gdbport [lindex $res 1]
+
+    # Set the sysroot.
+    gdb_test_no_output "set sysroot $sysroot_command"
+
+    # Connect to gdbserver, making sure GDB reads in the binary correctly.
+    set test "connect to remote and read binary"
+    if {[gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport $reading_symbols] == 0} {
+       pass $test
+    } else {
+       fail $test
+    }
+
+    gdb_breakpoint main
+    gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
+
+    # Test that we can stop inside a library.
+    gdb_breakpoint printf
+    gdb_test "continue" "Breakpoint $decimal.* printf .*" "continue to printf"
+}
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index dbd885aa22..2cb64f7d2f 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -42,9 +42,11 @@

 #
 # gdb_target_cmd
-# Send gdb the "target" command
+# Send gdb the "target" command.  Returns 0 on success, 1 on failure.
+# If specified, then ADDITIONAL_TEXT must match the text that comes after
+# the connection message in order for the procedure to succeed.
 #
-proc gdb_target_cmd { targetname serialport } {
+proc gdb_target_cmd { targetname serialport {additional_text ""} } {
     global gdb_prompt

     set serialport_re [string_to_regexp $serialport]
@@ -61,23 +63,23 @@ proc gdb_target_cmd { targetname serialport } {
            -re "Couldn't establish connection to remote.*$gdb_prompt $" {
                verbose "Connection failed"
            }
-           -re "Remote MIPS debugging.*$gdb_prompt" {
+           -re "Remote MIPS debugging.*$additional_text.*$gdb_prompt" {
                verbose "Set target to $targetname"
                return 0
            }
-           -re "Remote debugging using .*$serialport_re.*$gdb_prompt $" {
+           -re "Remote debugging using .*$serialport_re.*$additional_text.*$gdb_prompt $" {
                verbose "Set target to $targetname"
                return 0
            }
-           -re "Remote debugging using stdio.*$gdb_prompt $" {
+           -re "Remote debugging using stdio.*$additional_text.*$gdb_prompt $" {
                verbose "Set target to $targetname"
                return 0
            }
-           -re "Remote target $targetname connected to.*$gdb_prompt $" {
+           -re "Remote target $targetname connected to.*$additional_text.*$gdb_prompt $" {
                verbose "Set target to $targetname"
                return 0
            }
-           -re "Connected to.*$gdb_prompt $" {
+           -re "Connected to.*$additional_text.*$gdb_prompt $" {
                verbose "Set target to $targetname"
                return 0
            }