Testsuite: Add gdbserver sysroot test

Message ID C32017F7-001A-4A5D-93EB-2E51B6C68605@arm.com
State New, archived
Headers

Commit Message

Alan Hayward April 9, 2019, 11:04 a.m. UTC
  > 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:
  

Comments

Pedro Alves April 9, 2019, 2:35 p.m. UTC | #1
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
  

Patch

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"
+}