Re: [PATCH] Testsuite: Add gdbserver sysroot test

[Pedro Alves <palves at redhat dot com>]

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