Re: [PATCH] Make ftrace tests work with remote targets

[Simon Marchi <simon dot marchi at ericsson dot com>]

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.