Testsuite: Add gdbserver sysroot test
Commit Message
> 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.
Comments
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
new file mode 100644
@@ -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;
+}
new file mode 100644
@@ -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"
+}
@@ -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
}