Make ftrace tests work with remote targets

  When we build a shared library for testing, it is built differently
whether it is meant for the local system or a remote one.  When it is
for the local system, the library is built with no SONAME.  So when the
executable is built, roughly in this way:

  $ gcc testfile.c /path/to/library.so

the executable will contain an absolute reference to the library.  For
example:

  $ readelf -a testsuite/gdb.python/py-shared | grep NEEDED
   0x0000000000000001 (NEEDED)             Shared library: [/home/emaisin/build/binutils-gdb/gdb/testsuite/gdb.python/py-shared-sl.sl]

When testing is done remotely, the absolute path obviously doesn't work.
Therefore, we build the library with an SONAME:

  $ readelf -a testsuite/gdb.python/py-shared-sl.sl | grep SONAME
   0x000000000000000e (SONAME)             Library soname: [py-shared-sl.sl]

which ends up in the executable's NEEDED field:

  $ readelf -a testsuite/gdb.python/py-shared | grep NEEDED
   0x0000000000000001 (NEEDED)             Shared library: [py-shared-sl.sl]

The executable and the library are then uploaded side-by-side on the
remote system.  To allow the dynamic linker to find the shared library,
we have to add the special RPATH value $ORIGIN, which tells it to search
in the executable's directory:

  $ readelf -a testsuite/gdb.python/py-shared | grep ORIGIN
   0x000000000000000f (RPATH)              Library rpath: [$ORIGIN]

The problem with the IPA library is that it doesn't have an SONAME,
making it very difficult to do testing on a remote board.  When a
test executable is linked with it, it contains an absolute reference to
the library path.  Therefore, unless the paths on the target are the
same as on the build system, it won't work.

To make it possible for tests using the IPA library to run test on
remote boards, I suggest adding dding an SONAME to libinproctrace.so.  I
don't think it should be a big problem for users.  All the libraries
installed on my system have an SONAME, so it should be fine if
libinproctrace.so does too.

As a consequence, native testing does not work anymore, since
executables do not contain the absolute path to the library anymore.  To
keep them working, we can have gdb_load_shlibs copy the library to the
test directory when testing natively.  That's done by modifying
gdb_load_shlibs.  We also have to add RPATH=$ORIGIN to executables, even
when testing natively.

I think it's a good change in general, as it reduces the differences
between testing a native and a remote target.  To further reduce those
differences, we can also always build test shared libraries with an
SONAME.

ftrace.exp and ftrace-lock.exp need to be modified slightly.  The code
checks that the IPA library is loaded using the absolute path on the
build machine.  That obviously doesn't work if the test is done
remotely, as the path will be different.  I changed the tests to only
search for the library basename (e.g. libinproctrace.so).

gdb/gdbserver/ChangeLog:

	* Makefile.in ($(IPA_LIB)): Set SONAME of the IPA lib.

gdb/testsuite/ChangeLog:

	* gdb.trace/ftrace-lock.exp: Check for IPA basename instead of
	absolute.
	* gdb.trace/ftrace.exp: Likewise.
	* lib/gdb.exp (gdb_compile): Set rpath $ORIGIN for non-remote
	targets as well.
	(gdb_compile_shlib): Set SONAME for non-remote targets as well.
	(gdb_load_shlibs): Copy libraries to test directory when testing
	natively.
---
 gdb/gdbserver/Makefile.in               |  2 +-
 gdb/testsuite/gdb.trace/ftrace-lock.exp |  2 +-
 gdb/testsuite/gdb.trace/ftrace.exp      |  2 +-
 gdb/testsuite/lib/gdb.exp               | 27 ++++++++++++++++++++-------
 4 files changed, 23 insertions(+), 10 deletions(-)
  

On 16-03-03 04:22 PM, Simon Marchi wrote:
> When we build a shared library for testing, it is built differently
> whether it is meant for the local system or a remote one.  When it is
> for the local system, the library is built with no SONAME.  So when the
> executable is built, roughly in this way:
> 
>   $ gcc testfile.c /path/to/library.so
> 
> the executable will contain an absolute reference to the library.  For
> example:
> 
>   $ readelf -a testsuite/gdb.python/py-shared | grep NEEDED
>    0x0000000000000001 (NEEDED)             Shared library: [/home/emaisin/build/binutils-gdb/gdb/testsuite/gdb.python/py-shared-sl.sl]
> 
> When testing is done remotely, the absolute path obviously doesn't work.
> Therefore, we build the library with an SONAME:
> 
>   $ readelf -a testsuite/gdb.python/py-shared-sl.sl | grep SONAME
>    0x000000000000000e (SONAME)             Library soname: [py-shared-sl.sl]
> 
> which ends up in the executable's NEEDED field:
> 
>   $ readelf -a testsuite/gdb.python/py-shared | grep NEEDED
>    0x0000000000000001 (NEEDED)             Shared library: [py-shared-sl.sl]
> 
> The executable and the library are then uploaded side-by-side on the
> remote system.  To allow the dynamic linker to find the shared library,
> we have to add the special RPATH value $ORIGIN, which tells it to search
> in the executable's directory:
> 
>   $ readelf -a testsuite/gdb.python/py-shared | grep ORIGIN
>    0x000000000000000f (RPATH)              Library rpath: [$ORIGIN]
> 
> The problem with the IPA library is that it doesn't have an SONAME,
> making it very difficult to do testing on a remote board.  When a
> test executable is linked with it, it contains an absolute reference to
> the library path.  Therefore, unless the paths on the target are the
> same as on the build system, it won't work.
> 
> To make it possible for tests using the IPA library to run test on
> remote boards, I suggest adding dding an SONAME to libinproctrace.so.  I
> don't think it should be a big problem for users.  All the libraries
> installed on my system have an SONAME, so it should be fine if
> libinproctrace.so does too.
> 
> As a consequence, native testing does not work anymore, since
> executables do not contain the absolute path to the library anymore.  To
> keep them working, we can have gdb_load_shlibs copy the library to the
> test directory when testing natively.  That's done by modifying
> gdb_load_shlibs.  We also have to add RPATH=$ORIGIN to executables, even
> when testing natively.
> 
> I think it's a good change in general, as it reduces the differences
> between testing a native and a remote target.  To further reduce those
> differences, we can also always build test shared libraries with an
> SONAME.
> 
> ftrace.exp and ftrace-lock.exp need to be modified slightly.  The code
> checks that the IPA library is loaded using the absolute path on the
> build machine.  That obviously doesn't work if the test is done
> remotely, as the path will be different.  I changed the tests to only
> search for the library basename (e.g. libinproctrace.so).
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* Makefile.in ($(IPA_LIB)): Set SONAME of the IPA lib.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.trace/ftrace-lock.exp: Check for IPA basename instead of
> 	absolute.
> 	* gdb.trace/ftrace.exp: Likewise.
> 	* lib/gdb.exp (gdb_compile): Set rpath $ORIGIN for non-remote
> 	targets as well.
> 	(gdb_compile_shlib): Set SONAME for non-remote targets as well.
> 	(gdb_load_shlibs): Copy libraries to test directory when testing
> 	natively.
> ---
>  gdb/gdbserver/Makefile.in               |  2 +-
>  gdb/testsuite/gdb.trace/ftrace-lock.exp |  2 +-
>  gdb/testsuite/gdb.trace/ftrace.exp      |  2 +-
>  gdb/testsuite/lib/gdb.exp               | 27 ++++++++++++++++++++-------
>  4 files changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
> index 1e874e3..c4324e3 100644
> --- a/gdb/gdbserver/Makefile.in
> +++ b/gdb/gdbserver/Makefile.in
> @@ -327,7 +327,7 @@ IPA_LIB=libinproctrace.so
>  
>  $(IPA_LIB): $(IPA_OBJS) ${ADD_DEPS} ${CDEPS}
>  	rm -f $(IPA_LIB)
> -	$(CC_LD) -shared -fPIC -Wl,--no-undefined $(INTERNAL_CFLAGS) \
> +	$(CC_LD) -shared -fPIC -Wl,--soname=$(IPA_LIB) -Wl,--no-undefined $(INTERNAL_CFLAGS) \
>  	$(INTERNAL_LDFLAGS) -o $(IPA_LIB) ${IPA_OBJS} -ldl -pthread
>  
>  # Put the proper machine-specific files first, so M-. on a machine
> diff --git a/gdb/testsuite/gdb.trace/ftrace-lock.exp b/gdb/testsuite/gdb.trace/ftrace-lock.exp
> index 0b73086..077a261 100644
> --- a/gdb/testsuite/gdb.trace/ftrace-lock.exp
> +++ b/gdb/testsuite/gdb.trace/ftrace-lock.exp
> @@ -64,7 +64,7 @@ if ![runto_main] {
>      return -1
>  }
>  
> -if { [gdb_test "info sharedlibrary" ".*${libipa}.*" "IPA loaded"] != 0 } {
> +if { [gdb_test "info sharedlibrary" ".*[file tail ${libipa}].*" "IPA loaded"] != 0 } {
>      untested "Could not find IPA lib loaded"
>      return 1
>  }
> diff --git a/gdb/testsuite/gdb.trace/ftrace.exp b/gdb/testsuite/gdb.trace/ftrace.exp
> index 15ad7e7..ce6ac27 100644
> --- a/gdb/testsuite/gdb.trace/ftrace.exp
> +++ b/gdb/testsuite/gdb.trace/ftrace.exp
> @@ -217,7 +217,7 @@ proc test_ftrace_condition { condexp var list } \
>  
>  gdb_reinitialize_dir $srcdir/$subdir
>  
> -if { [gdb_test "info sharedlibrary" ".*${libipa}.*" "IPA loaded"] != 0 } {
> +if { [gdb_test "info sharedlibrary" ".*[file tail ${libipa}].*" "IPA loaded"] != 0 } {
>      untested "Could not find IPA lib loaded"
>      return 1
>  }
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 1fb05c4..d6a9ead 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -3374,7 +3374,7 @@ proc gdb_compile {source dest type options} {
>      # dynamically load one by basename, we must specify rpath.  If we
>      # are using a remote host, DejaGNU will link to the shared library
>      # using a relative path, so again we must specify an rpath.
> -    if { $shlib_load || ($shlib_found && [is_remote target]) } {
> +    if { $shlib_load || $shlib_found } {
>  	if { ([istarget "*-*-mingw*"]
>  	      || [istarget *-*-cygwin*]
>  	      || [istarget *-*-pe*]) } {
> @@ -3585,7 +3585,7 @@ proc gdb_compile_shlib {sources dest options} {
>  		set name ${dest}
>  	    }
>  	    lappend link_options "additional_flags=-Wl,--out-implib,${name}.a"
> -	} elseif [is_remote target] {
> +	} else {
>  	    # By default, we do not set the soname.  This causes the linker
>  	    # on ELF systems to create a DT_NEEDED entry in the executable
>  	    # refering to the full path name of the library.  This is a
> @@ -4218,14 +4218,27 @@ proc gdb_download { filename } {
>  # Copy the listed libraries to the target.
>  
>  proc gdb_load_shlibs { args } {
> -    if {![is_remote target]} {
> -	return
> -    }
> +    if {[is_remote target]} {
> +	foreach file $args {
> +	    # When the target is remote, we simply send the file to the target.
> +	    gdb_download [shlib_target_file $file]
> +	}
> +    } else {
> +	foreach from $args {
> +	    # When the target is native, we copy the files to the test directory
> +	    # (next to the executable), except if that's already where it is.
> +	    set to [standard_output_file [file tail $from]]
>  
> -    foreach file $args {
> -	gdb_download [shlib_target_file $file]
> +	    set from [file normalize $from]
> +	    set to [file normalize $to]
> +
> +	    if {"$from" != "$to"} {
> +		file copy -force $from $to
> +	    }
> +	}
>      }
>  
> +
>      # Even if the target supplies full paths for shared libraries,
>      # they may not be paths for this system.
>      gdb_test "set solib-search-path [file dirname [lindex $args 0]]" "" ""
> 


Ping.
  

On 16-03-11 12:30 PM, Simon Marchi wrote:
> On 16-03-03 04:22 PM, Simon Marchi wrote:
>> When we build a shared library for testing, it is built differently
>> whether it is meant for the local system or a remote one.  When it is
>> for the local system, the library is built with no SONAME.  So when the
>> executable is built, roughly in this way:
>>
>>   $ gcc testfile.c /path/to/library.so
>>
>> the executable will contain an absolute reference to the library.  For
>> example:
>>
>>   $ readelf -a testsuite/gdb.python/py-shared | grep NEEDED
>>    0x0000000000000001 (NEEDED)             Shared library: [/home/emaisin/build/binutils-gdb/gdb/testsuite/gdb.python/py-shared-sl.sl]
>>
>> When testing is done remotely, the absolute path obviously doesn't work.
>> Therefore, we build the library with an SONAME:
>>
>>   $ readelf -a testsuite/gdb.python/py-shared-sl.sl | grep SONAME
>>    0x000000000000000e (SONAME)             Library soname: [py-shared-sl.sl]
>>
>> which ends up in the executable's NEEDED field:
>>
>>   $ readelf -a testsuite/gdb.python/py-shared | grep NEEDED
>>    0x0000000000000001 (NEEDED)             Shared library: [py-shared-sl.sl]
>>
>> The executable and the library are then uploaded side-by-side on the
>> remote system.  To allow the dynamic linker to find the shared library,
>> we have to add the special RPATH value $ORIGIN, which tells it to search
>> in the executable's directory:
>>
>>   $ readelf -a testsuite/gdb.python/py-shared | grep ORIGIN
>>    0x000000000000000f (RPATH)              Library rpath: [$ORIGIN]
>>
>> The problem with the IPA library is that it doesn't have an SONAME,
>> making it very difficult to do testing on a remote board.  When a
>> test executable is linked with it, it contains an absolute reference to
>> the library path.  Therefore, unless the paths on the target are the
>> same as on the build system, it won't work.
>>
>> To make it possible for tests using the IPA library to run test on
>> remote boards, I suggest adding dding an SONAME to libinproctrace.so.  I
>> don't think it should be a big problem for users.  All the libraries
>> installed on my system have an SONAME, so it should be fine if
>> libinproctrace.so does too.
>>
>> As a consequence, native testing does not work anymore, since
>> executables do not contain the absolute path to the library anymore.  To
>> keep them working, we can have gdb_load_shlibs copy the library to the
>> test directory when testing natively.  That's done by modifying
>> gdb_load_shlibs.  We also have to add RPATH=$ORIGIN to executables, even
>> when testing natively.
>>
>> I think it's a good change in general, as it reduces the differences
>> between testing a native and a remote target.  To further reduce those
>> differences, we can also always build test shared libraries with an
>> SONAME.
>>
>> ftrace.exp and ftrace-lock.exp need to be modified slightly.  The code
>> checks that the IPA library is loaded using the absolute path on the
>> build machine.  That obviously doesn't work if the test is done
>> remotely, as the path will be different.  I changed the tests to only
>> search for the library basename (e.g. libinproctrace.so).
>>
>> gdb/gdbserver/ChangeLog:
>>
>> 	* Makefile.in ($(IPA_LIB)): Set SONAME of the IPA lib.
>>
>> gdb/testsuite/ChangeLog:
>>
>> 	* gdb.trace/ftrace-lock.exp: Check for IPA basename instead of
>> 	absolute.
>> 	* gdb.trace/ftrace.exp: Likewise.
>> 	* lib/gdb.exp (gdb_compile): Set rpath $ORIGIN for non-remote
>> 	targets as well.
>> 	(gdb_compile_shlib): Set SONAME for non-remote targets as well.
>> 	(gdb_load_shlibs): Copy libraries to test directory when testing
>> 	natively.
>> ---
>>  gdb/gdbserver/Makefile.in               |  2 +-
>>  gdb/testsuite/gdb.trace/ftrace-lock.exp |  2 +-
>>  gdb/testsuite/gdb.trace/ftrace.exp      |  2 +-
>>  gdb/testsuite/lib/gdb.exp               | 27 ++++++++++++++++++++-------
>>  4 files changed, 23 insertions(+), 10 deletions(-)
>>
>> diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
>> index 1e874e3..c4324e3 100644
>> --- a/gdb/gdbserver/Makefile.in
>> +++ b/gdb/gdbserver/Makefile.in
>> @@ -327,7 +327,7 @@ IPA_LIB=libinproctrace.so
>>  
>>  $(IPA_LIB): $(IPA_OBJS) ${ADD_DEPS} ${CDEPS}
>>  	rm -f $(IPA_LIB)
>> -	$(CC_LD) -shared -fPIC -Wl,--no-undefined $(INTERNAL_CFLAGS) \
>> +	$(CC_LD) -shared -fPIC -Wl,--soname=$(IPA_LIB) -Wl,--no-undefined $(INTERNAL_CFLAGS) \
>>  	$(INTERNAL_LDFLAGS) -o $(IPA_LIB) ${IPA_OBJS} -ldl -pthread
>>  
>>  # Put the proper machine-specific files first, so M-. on a machine
>> diff --git a/gdb/testsuite/gdb.trace/ftrace-lock.exp b/gdb/testsuite/gdb.trace/ftrace-lock.exp
>> index 0b73086..077a261 100644
>> --- a/gdb/testsuite/gdb.trace/ftrace-lock.exp
>> +++ b/gdb/testsuite/gdb.trace/ftrace-lock.exp
>> @@ -64,7 +64,7 @@ if ![runto_main] {
>>      return -1
>>  }
>>  
>> -if { [gdb_test "info sharedlibrary" ".*${libipa}.*" "IPA loaded"] != 0 } {
>> +if { [gdb_test "info sharedlibrary" ".*[file tail ${libipa}].*" "IPA loaded"] != 0 } {
>>      untested "Could not find IPA lib loaded"
>>      return 1
>>  }
>> diff --git a/gdb/testsuite/gdb.trace/ftrace.exp b/gdb/testsuite/gdb.trace/ftrace.exp
>> index 15ad7e7..ce6ac27 100644
>> --- a/gdb/testsuite/gdb.trace/ftrace.exp
>> +++ b/gdb/testsuite/gdb.trace/ftrace.exp
>> @@ -217,7 +217,7 @@ proc test_ftrace_condition { condexp var list } \
>>  
>>  gdb_reinitialize_dir $srcdir/$subdir
>>  
>> -if { [gdb_test "info sharedlibrary" ".*${libipa}.*" "IPA loaded"] != 0 } {
>> +if { [gdb_test "info sharedlibrary" ".*[file tail ${libipa}].*" "IPA loaded"] != 0 } {
>>      untested "Could not find IPA lib loaded"
>>      return 1
>>  }
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index 1fb05c4..d6a9ead 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -3374,7 +3374,7 @@ proc gdb_compile {source dest type options} {
>>      # dynamically load one by basename, we must specify rpath.  If we
>>      # are using a remote host, DejaGNU will link to the shared library
>>      # using a relative path, so again we must specify an rpath.
>> -    if { $shlib_load || ($shlib_found && [is_remote target]) } {
>> +    if { $shlib_load || $shlib_found } {
>>  	if { ([istarget "*-*-mingw*"]
>>  	      || [istarget *-*-cygwin*]
>>  	      || [istarget *-*-pe*]) } {
>> @@ -3585,7 +3585,7 @@ proc gdb_compile_shlib {sources dest options} {
>>  		set name ${dest}
>>  	    }
>>  	    lappend link_options "additional_flags=-Wl,--out-implib,${name}.a"
>> -	} elseif [is_remote target] {
>> +	} else {
>>  	    # By default, we do not set the soname.  This causes the linker
>>  	    # on ELF systems to create a DT_NEEDED entry in the executable
>>  	    # refering to the full path name of the library.  This is a
>> @@ -4218,14 +4218,27 @@ proc gdb_download { filename } {
>>  # Copy the listed libraries to the target.
>>  
>>  proc gdb_load_shlibs { args } {
>> -    if {![is_remote target]} {
>> -	return
>> -    }
>> +    if {[is_remote target]} {
>> +	foreach file $args {
>> +	    # When the target is remote, we simply send the file to the target.
>> +	    gdb_download [shlib_target_file $file]
>> +	}
>> +    } else {
>> +	foreach from $args {
>> +	    # When the target is native, we copy the files to the test directory
>> +	    # (next to the executable), except if that's already where it is.
>> +	    set to [standard_output_file [file tail $from]]
>>  
>> -    foreach file $args {
>> -	gdb_download [shlib_target_file $file]
>> +	    set from [file normalize $from]
>> +	    set to [file normalize $to]
>> +
>> +	    if {"$from" != "$to"} {
>> +		file copy -force $from $to
>> +	    }
>> +	}
>>      }
>>  
>> +
>>      # Even if the target supplies full paths for shared libraries,
>>      # they may not be paths for this system.
>>      gdb_test "set solib-search-path [file dirname [lindex $args 0]]" "" ""
>>
> 
> 
> Ping.

Ping.
  

On 03/03/2016 09:22 PM, Simon Marchi wrote:
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -3374,7 +3374,7 @@ proc gdb_compile {source dest type options} {
>      # dynamically load one by basename, we must specify rpath.  If we
>      # are using a remote host, DejaGNU will link to the shared library
>      # using a relative path, so again we must specify an rpath.
> -    if { $shlib_load || ($shlib_found && [is_remote target]) } {
> +    if { $shlib_load || $shlib_found } {

I think the comment above needs updating.

>  	    lappend link_options "additional_flags=-Wl,--out-implib,${name}.a"
> -	} elseif [is_remote target] {
> +	} else {
>  	    # By default, we do not set the soname.  This causes the linker

Likewise.


>  proc gdb_load_shlibs { args } {
> -    if {![is_remote target]} {
> -	return
> -    }
> +    if {[is_remote target]} {
> +	foreach file $args {
> +	    # When the target is remote, we simply send the file to the target.
> +	    gdb_download [shlib_target_file $file]
> +	}
> +    } else {
> +	foreach from $args {
> +	    # When the target is native, we copy the files to the test directory
> +	    # (next to the executable), except if that's already where it is.
> +	    set to [standard_output_file [file tail $from]]

I'd think it better to make the gdb_download path work for native
testing as well.  WDYT?  E.g., make shlib_target_file default to
return the test directory path?

> # Even if the target supplies full paths for shared libraries,
> # they may not be paths for this system.
> gdb_test "set solib-search-path [file dirname [lindex $args 0]]" "" ""

Can we skip this command on native testing?

I'm worried that that command might paper over, or cause, issues with
sysroot / dso path lookup.  Normal native debugging usage will not specify
that command, so if we could avoid it, I'd prefer it, on grounds of
testing what users normally use.  

Thanks,
Pedro Alves
  

On 16-03-29 06:46 PM, Pedro Alves wrote:
> On 03/03/2016 09:22 PM, Simon Marchi wrote:
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -3374,7 +3374,7 @@ proc gdb_compile {source dest type options} {
>>      # dynamically load one by basename, we must specify rpath.  If we
>>      # are using a remote host, DejaGNU will link to the shared library
>>      # using a relative path, so again we must specify an rpath.
>> -    if { $shlib_load || ($shlib_found && [is_remote target]) } {
>> +    if { $shlib_load || $shlib_found } {
> 
> I think the comment above needs updating.

What about:

     # Because we link with libraries using their basename, we may need
     # (depending on the platform) to set a special rpath value, to allow
     # the executable to find the libraries it depends on.

>>  	    lappend link_options "additional_flags=-Wl,--out-implib,${name}.a"
>> -	} elseif [is_remote target] {
>> +	} else {
>>  	    # By default, we do not set the soname.  This causes the linker
> 
> Likewise.

What about:

             # Set the soname of the library.  This causes the linker on ELF
             # systems to create the DT_NEEDED entry in the executable referring
             # to the soname of the library, and not its absolute path.  This
             # (using the absolute path) would be problem when testing on a
             # remote target.
             #
             # In conjunction with setting the soname, we add the special
             # rpath=$ORIGIN value when building the executable, so that it's
             # able to find the library in its own directory.


>>  proc gdb_load_shlibs { args } {
>> -    if {![is_remote target]} {
>> -	return
>> -    }
>> +    if {[is_remote target]} {
>> +	foreach file $args {
>> +	    # When the target is remote, we simply send the file to the target.
>> +	    gdb_download [shlib_target_file $file]
>> +	}
>> +    } else {
>> +	foreach from $args {
>> +	    # When the target is native, we copy the files to the test directory
>> +	    # (next to the executable), except if that's already where it is.
>> +	    set to [standard_output_file [file tail $from]]
> 
> I'd think it better to make the gdb_download path work for native
> testing as well.  WDYT?  E.g., make shlib_target_file default to
> return the test directory path?

If I understand correctly, gdb_download would take a local file path and take
care of transferring that file to the "runtime test directory", whether it's
local or remote.  If the target is local, it checks whether $src == $dest, and
copies the file if not.  Basically, the same as my implementation of
gdb_load_shlibs, just not limited to shlibs.

I am not sure if "make shlib_target_file default to return the test directory path?"
would be ok.  I think it's meant to return the local file path to download to the
target.  So gdb_load_shlibs would then become:

proc gdb_load_shlibs { args } {
    foreach file $args {
        gdb_download [shlib_target_file $file]
    }
}

I just tested it quickly and it seems to work fine.  But I also noticed that there
might be some overlap in feature between gdb_download and gdb_remote_download, so
perhaps I can try to merge them, WDYT?

>> # Even if the target supplies full paths for shared libraries,
>> # they may not be paths for this system.
>> gdb_test "set solib-search-path [file dirname [lindex $args 0]]" "" ""
> 
> Can we skip this command on native testing?
> 
> I'm worried that that command might paper over, or cause, issues with
> sysroot / dso path lookup.  Normal native debugging usage will not specify
> that command, so if we could avoid it, I'd prefer it, on grounds of
> testing what users normally use.  

Makes sense.  I'll add that, with a comment explaining why.

Thanks!

Simon
  

On 03/31/2016 09:24 PM, Simon Marchi wrote:
> On 16-03-29 06:46 PM, Pedro Alves wrote:
>> On 03/03/2016 09:22 PM, Simon Marchi wrote:
>>> --- a/gdb/testsuite/lib/gdb.exp
>>> +++ b/gdb/testsuite/lib/gdb.exp
>>> @@ -3374,7 +3374,7 @@ proc gdb_compile {source dest type options} {
>>>      # dynamically load one by basename, we must specify rpath.  If we
>>>      # are using a remote host, DejaGNU will link to the shared library
>>>      # using a relative path, so again we must specify an rpath.
>>> -    if { $shlib_load || ($shlib_found && [is_remote target]) } {
>>> +    if { $shlib_load || $shlib_found } {
>>
>> I think the comment above needs updating.
> 
> What about:
> 
>      # Because we link with libraries using their basename, we may need
>      # (depending on the platform) to set a special rpath value, to allow
>      # the executable to find the libraries it depends on.

That's great.

> 
>>>  	    lappend link_options "additional_flags=-Wl,--out-implib,${name}.a"
>>> -	} elseif [is_remote target] {
>>> +	} else {
>>>  	    # By default, we do not set the soname.  This causes the linker
>>
>> Likewise.
> 
> What about:
> 
>              # Set the soname of the library.  This causes the linker on ELF
>              # systems to create the DT_NEEDED entry in the executable referring
>              # to the soname of the library, and not its absolute path.  This
>              # (using the absolute path) would be problem when testing on a
>              # remote target.
>              #
>              # In conjunction with setting the soname, we add the special
>              # rpath=$ORIGIN value when building the executable, so that it's
>              # able to find the library in its own directory.

Perfect.

> 
> 
>>>  proc gdb_load_shlibs { args } {
>>> -    if {![is_remote target]} {
>>> -	return
>>> -    }
>>> +    if {[is_remote target]} {
>>> +	foreach file $args {
>>> +	    # When the target is remote, we simply send the file to the target.
>>> +	    gdb_download [shlib_target_file $file]
>>> +	}
>>> +    } else {
>>> +	foreach from $args {
>>> +	    # When the target is native, we copy the files to the test directory
>>> +	    # (next to the executable), except if that's already where it is.
>>> +	    set to [standard_output_file [file tail $from]]
>>
>> I'd think it better to make the gdb_download path work for native
>> testing as well.  WDYT?  E.g., make shlib_target_file default to
>> return the test directory path?
> 
> If I understand correctly, gdb_download would take a local file path and take
> care of transferring that file to the "runtime test directory", whether it's
> local or remote.

Right.  I'm not sure whether it's the right level or not.

  If the target is local, it checks whether $src == $dest, and
> copies the file if not.  Basically, the same as my implementation of
> gdb_load_shlibs, just not limited to shlibs.

*nod*

Now that I grep for gdb_download, sounds like we'll need to
change mi_load_shlibs similarly.  Not sure offhand why the jit*.so
hits don't use gdb_load_shlibs instead or gdb_download 

> 
> I am not sure if "make shlib_target_file default to return the test directory path?"
> would be ok.  I think it's meant to return the local file path to download to the
> target.

You're right.  I added those for SymbianOS years ago, since that target 
had a post link step to convert elf to pe/dll, and it'd be the latter that'd
be copied to the target.  IIRC.


>  So gdb_load_shlibs would then become:
> 
> proc gdb_load_shlibs { args } {
>     foreach file $args {
>         gdb_download [shlib_target_file $file]
>     }
> }
> 
> I just tested it quickly and it seems to work fine.  But I also noticed that there
> might be some overlap in feature between gdb_download and gdb_remote_download, so
> perhaps I can try to merge them, WDYT?
> 

Indeed.

>>> # Even if the target supplies full paths for shared libraries,
>>> # they may not be paths for this system.
>>> gdb_test "set solib-search-path [file dirname [lindex $args 0]]" "" ""
>>
>> Can we skip this command on native testing?
>>
>> I'm worried that that command might paper over, or cause, issues with
>> sysroot / dso path lookup.  Normal native debugging usage will not specify
>> that command, so if we could avoid it, I'd prefer it, on grounds of
>> testing what users normally use.  
> 
> Makes sense.  I'll add that, with a comment explaining why.
> 
> Thanks!
> 

Thank you!
  

On 16-03-31 07:18 PM, Pedro Alves wrote:
> On 03/31/2016 09:24 PM, Simon Marchi wrote:
>> On 16-03-29 06:46 PM, Pedro Alves wrote:
>>> On 03/03/2016 09:22 PM, Simon Marchi wrote:
>>>> --- a/gdb/testsuite/lib/gdb.exp
>>>> +++ b/gdb/testsuite/lib/gdb.exp
>>>> @@ -3374,7 +3374,7 @@ proc gdb_compile {source dest type options} {
>>>>      # dynamically load one by basename, we must specify rpath.  If we
>>>>      # are using a remote host, DejaGNU will link to the shared library
>>>>      # using a relative path, so again we must specify an rpath.
>>>> -    if { $shlib_load || ($shlib_found && [is_remote target]) } {
>>>> +    if { $shlib_load || $shlib_found } {
>>>
>>> I think the comment above needs updating.
>>
>> What about:
>>
>>      # Because we link with libraries using their basename, we may need
>>      # (depending on the platform) to set a special rpath value, to allow
>>      # the executable to find the libraries it depends on.
> 
> That's great.
> 
>>
>>>>  	    lappend link_options "additional_flags=-Wl,--out-implib,${name}.a"
>>>> -	} elseif [is_remote target] {
>>>> +	} else {
>>>>  	    # By default, we do not set the soname.  This causes the linker
>>>
>>> Likewise.
>>
>> What about:
>>
>>              # Set the soname of the library.  This causes the linker on ELF
>>              # systems to create the DT_NEEDED entry in the executable referring
>>              # to the soname of the library, and not its absolute path.  This
>>              # (using the absolute path) would be problem when testing on a
>>              # remote target.
>>              #
>>              # In conjunction with setting the soname, we add the special
>>              # rpath=$ORIGIN value when building the executable, so that it's
>>              # able to find the library in its own directory.
> 
> Perfect.
> 
>>
>>
>>>>  proc gdb_load_shlibs { args } {
>>>> -    if {![is_remote target]} {
>>>> -	return
>>>> -    }
>>>> +    if {[is_remote target]} {
>>>> +	foreach file $args {
>>>> +	    # When the target is remote, we simply send the file to the target.
>>>> +	    gdb_download [shlib_target_file $file]
>>>> +	}
>>>> +    } else {
>>>> +	foreach from $args {
>>>> +	    # When the target is native, we copy the files to the test directory
>>>> +	    # (next to the executable), except if that's already where it is.
>>>> +	    set to [standard_output_file [file tail $from]]
>>>
>>> I'd think it better to make the gdb_download path work for native
>>> testing as well.  WDYT?  E.g., make shlib_target_file default to
>>> return the test directory path?
>>
>> If I understand correctly, gdb_download would take a local file path and take
>> care of transferring that file to the "runtime test directory", whether it's
>> local or remote.
> 
> Right.  I'm not sure whether it's the right level or not.
> 
>   If the target is local, it checks whether $src == $dest, and
>> copies the file if not.  Basically, the same as my implementation of
>> gdb_load_shlibs, just not limited to shlibs.
> 
> *nod*
> 
> Now that I grep for gdb_download, sounds like we'll need to
> change mi_load_shlibs similarly.  Not sure offhand why the jit*.so
> hits don't use gdb_load_shlibs instead or gdb_download 
> 
>>
>> I am not sure if "make shlib_target_file default to return the test directory path?"
>> would be ok.  I think it's meant to return the local file path to download to the
>> target.
> 
> You're right.  I added those for SymbianOS years ago, since that target 
> had a post link step to convert elf to pe/dll, and it'd be the latter that'd
> be copied to the target.  IIRC.
> 
> 
>>  So gdb_load_shlibs would then become:
>>
>> proc gdb_load_shlibs { args } {
>>     foreach file $args {
>>         gdb_download [shlib_target_file $file]
>>     }
>> }
>>
>> I just tested it quickly and it seems to work fine.  But I also noticed that there
>> might be some overlap in feature between gdb_download and gdb_remote_download, so
>> perhaps I can try to merge them, WDYT?
>>
> 
> Indeed.
> 
>>>> # Even if the target supplies full paths for shared libraries,
>>>> # they may not be paths for this system.
>>>> gdb_test "set solib-search-path [file dirname [lindex $args 0]]" "" ""
>>>
>>> Can we skip this command on native testing?
>>>
>>> I'm worried that that command might paper over, or cause, issues with
>>> sysroot / dso path lookup.  Normal native debugging usage will not specify
>>> that command, so if we could avoid it, I'd prefer it, on grounds of
>>> testing what users normally use.  
>>
>> Makes sense.  I'll add that, with a comment explaining why.
>>
>> Thanks!
>>
> 
> Thank you!
> 

V2 posted here: https://sourceware.org/ml/gdb-patches/2016-04/msg00047.html