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

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

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.