[2/7,gdb/testsuite] fix test gdb.base/print-file-var.exp for remote execution

  ---
 gdb/testsuite/gdb.base/print-file-var.exp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
  

On 10/25/22 18:29, Ivan Tetyushkin wrote:
> ---
>   gdb/testsuite/gdb.base/print-file-var.exp | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
> index 9abe87d7758..73137630fed 100644
> --- a/gdb/testsuite/gdb.base/print-file-var.exp
> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
> @@ -42,6 +42,8 @@ proc test {hidden dlopen version_id_main lang} {
>       set libobj1 [standard_output_file ${lib1}$suffix.so]
>       set libobj2 [standard_output_file ${lib2}$suffix.so]
>   
> +    set runtimelibobj2 [get_runtime_file $libobj2]
> +
>       set lib_opts { debug $lang }
>       lappend lib_opts "additional_flags=-DHIDDEN=$hidden"
>   
> @@ -60,7 +62,7 @@ proc test {hidden dlopen version_id_main lang} {
>       set link_opts [list debug shlib=${libobj1}]
>   
>       if {$dlopen} {
> -	lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
> +	lappend main_opts "additional_flags=-DSHLIB_NAME=\"$runtimelibobj2\""
>   	lappend link_opts "shlib_load"
>       } else {
>   	lappend link_opts "shlib=${libobj2}"

I get this test-case passing by avoiding to use an absolute file name:
...
diff --git a/gdb/testsuite/gdb.base/print-file-var.exp 
b/gdb/testsuite/gdb.base/print-file-
var.exp
index 9abe87d7758..338840cb05e 100644
--- a/gdb/testsuite/gdb.base/print-file-var.exp
+++ b/gdb/testsuite/gdb.base/print-file-var.exp
@@ -60,7 +60,7 @@ proc test {hidden dlopen version_id_main lang} {
      set link_opts [list debug shlib=${libobj1}]

      if {$dlopen} {
-       lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
+       lappend main_opts "additional_flags=-DSHLIB_NAME=\"[file tail 
$libobj2]\""
         lappend link_opts "shlib_load"
      } else {
         lappend link_opts "shlib=${libobj2}"
...
which means that the file is found relative to $ORIGIN, as is the case 
for $dlopen == 0.

I'm not entirely happy with this fix, because what we'd rather want is 
to use the name as returned by remote_download, but that's done in 
gdb_load_shlib later on (which also handles shlib_target_file, so 
actually, the name as returned by this proc is the one we really want), 
so it's not available yet at the time we do the compilation.

I've thought about allowing gdb_load_shlib to be called without gdb 
instance, and for cases where there's no instance, registering the 
solib-search-path setting to be done at gdb start.  This will allow us 
to move the gdb_load_shlib calls to before the compilation, such that we 
can use the result to set -DSHLIB_NAME.  But I think it's a solution 
bound to cause confusion because things happen under the hood.

Alternatively, we can try to not pass in the name into compilation 
(working around only some of the problems related to remote host 
testing, see https://sourceware.org/bugzilla/show_bug.cgi?id=16947), but 
that also has its limitations: we need to be able to patch target memory 
or the ability to use argv.

Perhaps splitting up the functionality in gdb_load_shlib makes the most 
sense.  By default, it'll do the same as before, but by calling it 
twice, once with -only-download, and once with -no-download can we get 
the solib name before starting gdb.

I've given this a try in patch attached below.

Ivan, could you check if the patch fixes the gdb.base/print-file-var.exp 
test-case for you?

Thanks,
- Tom
  

Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

> On 10/25/22 18:29, Ivan Tetyushkin wrote:
>> ---
>>   gdb/testsuite/gdb.base/print-file-var.exp | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
>> index 9abe87d7758..73137630fed 100644
>> --- a/gdb/testsuite/gdb.base/print-file-var.exp
>> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
>> @@ -42,6 +42,8 @@ proc test {hidden dlopen version_id_main lang} {
>>       set libobj1 [standard_output_file ${lib1}$suffix.so]
>>       set libobj2 [standard_output_file ${lib2}$suffix.so]
>>   
>> +    set runtimelibobj2 [get_runtime_file $libobj2]
>> +
>>       set lib_opts { debug $lang }
>>       lappend lib_opts "additional_flags=-DHIDDEN=$hidden"
>>   
>> @@ -60,7 +62,7 @@ proc test {hidden dlopen version_id_main lang} {
>>       set link_opts [list debug shlib=${libobj1}]
>>   
>>       if {$dlopen} {
>> -	lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
>> +	lappend main_opts "additional_flags=-DSHLIB_NAME=\"$runtimelibobj2\""
>>   	lappend link_opts "shlib_load"
>>       } else {
>>   	lappend link_opts "shlib=${libobj2}"
>
> I get this test-case passing by avoiding to use an absolute file name:
> ...
> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp 
> b/gdb/testsuite/gdb.base/print-file-
> var.exp
> index 9abe87d7758..338840cb05e 100644
> --- a/gdb/testsuite/gdb.base/print-file-var.exp
> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
> @@ -60,7 +60,7 @@ proc test {hidden dlopen version_id_main lang} {
>       set link_opts [list debug shlib=${libobj1}]
>
>       if {$dlopen} {
> -       lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
> +       lappend main_opts "additional_flags=-DSHLIB_NAME=\"[file tail 
> $libobj2]\""
>          lappend link_opts "shlib_load"
>       } else {
>          lappend link_opts "shlib=${libobj2}"
> ...
> which means that the file is found relative to $ORIGIN, as is the case 
> for $dlopen == 0.
>
> I'm not entirely happy with this fix, because what we'd rather want is 
> to use the name as returned by remote_download, but that's done in 
> gdb_load_shlib later on (which also handles shlib_target_file, so 
> actually, the name as returned by this proc is the one we really want), 
> so it's not available yet at the time we do the compilation.
>
> I've thought about allowing gdb_load_shlib to be called without gdb 
> instance, and for cases where there's no instance, registering the 
> solib-search-path setting to be done at gdb start.  This will allow us 
> to move the gdb_load_shlib calls to before the compilation, such that we 
> can use the result to set -DSHLIB_NAME.  But I think it's a solution 
> bound to cause confusion because things happen under the hood.
>
> Alternatively, we can try to not pass in the name into compilation 
> (working around only some of the problems related to remote host 
> testing, see https://sourceware.org/bugzilla/show_bug.cgi?id=16947), but 
> that also has its limitations: we need to be able to patch target memory 
> or the ability to use argv.
>
> Perhaps splitting up the functionality in gdb_load_shlib makes the most 
> sense.  By default, it'll do the same as before, but by calling it 
> twice, once with -only-download, and once with -no-download can we get 
> the solib name before starting gdb.
>
> I've given this a try in patch attached below.

I also prefer the approach that Tom has suggested here.  I gave the
patch a go and it fixed the problems in print_file-var.exp for me using
a local/remote type setup.

>
> Ivan, could you check if the patch fixes the gdb.base/print-file-var.exp 
> test-case for you?
>
> Thanks,
> - Tom
> From b072645433f83d62a4b7cf857d3d50f0dd7e0c75 Mon Sep 17 00:00:00 2001
> From: Tom de Vries <tdevries@suse.de>
> Date: Sun, 6 Nov 2022 11:26:23 +0100
> Subject: [PATCH 3/3] [gdb/testsuite] Fix gdb.base/print-file-var.exp for
>  remote target
>
> ---
>  gdb/testsuite/gdb.base/print-file-var.exp |  6 ++-
>  gdb/testsuite/lib/gdb.exp                 | 48 +++++++++++++++++------
>  2 files changed, 39 insertions(+), 15 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
> index 9abe87d7758..abdbbbfdce4 100644
> --- a/gdb/testsuite/gdb.base/print-file-var.exp
> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
> @@ -59,8 +59,10 @@ proc test {hidden dlopen version_id_main lang} {
>      set main_opts [list debug $lang]
>      set link_opts [list debug shlib=${libobj1}]
>  
> +    set target_libobj2 [gdb_load_shlib $libobj2 -only-download]
> +
>      if {$dlopen} {
> -	lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
> +	lappend main_opts "additional_flags=-DSHLIB_NAME=\"$target_libobj2\""
>  	lappend link_opts "shlib_load"
>      } else {
>  	lappend link_opts "shlib=${libobj2}"
> @@ -79,7 +81,7 @@ proc test {hidden dlopen version_id_main lang} {
>  
>      clean_restart $executable
>      gdb_load_shlib $libobj1
> -    gdb_load_shlib $libobj2
> +    gdb_load_shlib $libobj2 -no-download
>  
>      if ![runto_main] {
>  	return -1
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index e2cda30b95a..5328afb27e8 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -5873,24 +5873,46 @@ proc gdb_remote_download {dest fromfile {tofile {}}} {
>  #
>  # Copy the listed library to the target.
>  
> -proc gdb_load_shlib { file } {
> +proc gdb_load_shlib { file args } {
>      global gdb_spawn_id

The comments on this function will need updating for a final version of
this patch.

Otherwise, looks good.

Thanks,
Andrew

>  
> -    if ![info exists gdb_spawn_id] {
> -	perror "gdb_load_shlib: GDB is not running"
> +    set download 1
> +    set solib-search-path 1
> +    parse_args {
> +	{no-download}
> +	{only-download}
> +    }
> +    if { ${no-download} && ${only-download} } {
> +	perror \
> +	    "gdb_load_shlib: Cannot use both -no-download and -only-download"
> +    }
> +    if { ${no-download} } {
> +	set download 0
> +    }
> +    if { ${only-download} } {
> +	set solib-search-path 0
>      }
>  
> -    set dest [gdb_remote_download target [shlib_target_file $file]]
> +    set dest ""
> +    if { $download } {
> +	set dest [gdb_remote_download target [shlib_target_file $file]]
> +    }
>  
> -    if {[is_remote target]} {
> -	# If the target is remote, we need to tell gdb where to find the
> -	# libraries.
> -	#
> -	# We could set this even when not testing remotely, but a user
> -	# generally won't set it unless necessary.  In order to make the tests
> -	# more like the real-life scenarios, we don't set it for local testing.
> -	gdb_test "set solib-search-path [file dirname $file]" "" \
> -	    "set solib-search-path for [file tail $file]"
> +    if { ${solib-search-path} } {
> +	if ![info exists gdb_spawn_id] {
> +	    perror "gdb_load_shlib: GDB is not running"
> +	}
> +
> +	if {[is_remote target]} {
> +	    # If the target is remote, we need to tell gdb where to find the
> +	    # libraries.
> +	    #
> +	    # We could set this even when not testing remotely, but a user
> +	    # generally won't set it unless necessary.  In order to make the tests
> +	    # more like the real-life scenarios, we don't set it for local testing.
> +	    gdb_test "set solib-search-path [file dirname $file]" "" \
> +		"set solib-search-path for [file tail $file]"
> +	}
>      }
>  
>      return $dest
> -- 
> 2.35.3
  

Hi
Yes, this patch fixes this test. However, it will not fix gdb.base/infcall-exec.exp from the patch 5/7 of this thread.
As I can understand now, this test uses absolute paths for checking, but relative paths are used during compilation. We can change regexp itself and check only filename, but this could lead to false positive result.
Also, I thought these tests are created to work with absolute paths to ignore solib-search-path.

--
Ivan
  

On 11/6/22 05:54, Tom de Vries via Gdb-patches wrote:
> On 10/25/22 18:29, Ivan Tetyushkin wrote:
>> ---
>>   gdb/testsuite/gdb.base/print-file-var.exp | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
>> index 9abe87d7758..73137630fed 100644
>> --- a/gdb/testsuite/gdb.base/print-file-var.exp
>> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
>> @@ -42,6 +42,8 @@ proc test {hidden dlopen version_id_main lang} {
>>       set libobj1 [standard_output_file ${lib1}$suffix.so]
>>       set libobj2 [standard_output_file ${lib2}$suffix.so]
>>   +    set runtimelibobj2 [get_runtime_file $libobj2]
>> +
>>       set lib_opts { debug $lang }
>>       lappend lib_opts "additional_flags=-DHIDDEN=$hidden"
>>   @@ -60,7 +62,7 @@ proc test {hidden dlopen version_id_main lang} {
>>       set link_opts [list debug shlib=${libobj1}]
>>         if {$dlopen} {
>> -    lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
>> +    lappend main_opts "additional_flags=-DSHLIB_NAME=\"$runtimelibobj2\""
>>       lappend link_opts "shlib_load"
>>       } else {
>>       lappend link_opts "shlib=${libobj2}"
> 
> I get this test-case passing by avoiding to use an absolute file name:
> ...
> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-
> var.exp
> index 9abe87d7758..338840cb05e 100644
> --- a/gdb/testsuite/gdb.base/print-file-var.exp
> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
> @@ -60,7 +60,7 @@ proc test {hidden dlopen version_id_main lang} {
>      set link_opts [list debug shlib=${libobj1}]
> 
>      if {$dlopen} {
> -       lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
> +       lappend main_opts "additional_flags=-DSHLIB_NAME=\"[file tail $libobj2]\""
>         lappend link_opts "shlib_load"
>      } else {
>         lappend link_opts "shlib=${libobj2}"
> ...
> which means that the file is found relative to $ORIGIN, as is the case for $dlopen == 0.
> 
> I'm not entirely happy with this fix, because what we'd rather want is to use the name as returned by remote_download, but that's done in gdb_load_shlib later on (which also handles shlib_target_file, so actually, the name as returned by this proc is the one we really want), so it's not available yet at the time we do the compilation.
> 
> I've thought about allowing gdb_load_shlib to be called without gdb instance, and for cases where there's no instance, registering the solib-search-path setting to be done at gdb start.  This will allow us to move the gdb_load_shlib calls to before the compilation, such that we can use the result to set -DSHLIB_NAME.  But I think it's a solution bound to cause confusion because things happen under the hood.
> 
> Alternatively, we can try to not pass in the name into compilation (working around only some of the problems related to remote host testing, see https://sourceware.org/bugzilla/show_bug.cgi?id=16947), but that also has its limitations: we need to be able to patch target memory or the ability to use argv.
> 
> Perhaps splitting up the functionality in gdb_load_shlib makes the most sense.  By default, it'll do the same as before, but by calling it twice, once with -only-download, and once with -no-download can we get the solib name before starting gdb.
> 
> I've given this a try in patch attached below.
> 
> Ivan, could you check if the patch fixes the gdb.base/print-file-var.exp test-case for you?

I just gave it a quick look and I'm not familiar with the entire
problem.  But it sounds to me like it would be clearer to just make two
procs for the two steps, instead of one proc with flags to control its
behavior.  And perhaps a third proc that does both steps, for
convenience (which would be called gdb_load_shlib for backwards
compatibility, I guess).

Simon
  

On 11/7/22 14:55, Simon Marchi wrote:
> 
> 
> On 11/6/22 05:54, Tom de Vries via Gdb-patches wrote:
>> On 10/25/22 18:29, Ivan Tetyushkin wrote:
>>> ---
>>>    gdb/testsuite/gdb.base/print-file-var.exp | 4 +++-
>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
>>> index 9abe87d7758..73137630fed 100644
>>> --- a/gdb/testsuite/gdb.base/print-file-var.exp
>>> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
>>> @@ -42,6 +42,8 @@ proc test {hidden dlopen version_id_main lang} {
>>>        set libobj1 [standard_output_file ${lib1}$suffix.so]
>>>        set libobj2 [standard_output_file ${lib2}$suffix.so]
>>>    +    set runtimelibobj2 [get_runtime_file $libobj2]
>>> +
>>>        set lib_opts { debug $lang }
>>>        lappend lib_opts "additional_flags=-DHIDDEN=$hidden"
>>>    @@ -60,7 +62,7 @@ proc test {hidden dlopen version_id_main lang} {
>>>        set link_opts [list debug shlib=${libobj1}]
>>>          if {$dlopen} {
>>> -    lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
>>> +    lappend main_opts "additional_flags=-DSHLIB_NAME=\"$runtimelibobj2\""
>>>        lappend link_opts "shlib_load"
>>>        } else {
>>>        lappend link_opts "shlib=${libobj2}"
>>
>> I get this test-case passing by avoiding to use an absolute file name:
>> ...
>> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-
>> var.exp
>> index 9abe87d7758..338840cb05e 100644
>> --- a/gdb/testsuite/gdb.base/print-file-var.exp
>> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
>> @@ -60,7 +60,7 @@ proc test {hidden dlopen version_id_main lang} {
>>       set link_opts [list debug shlib=${libobj1}]
>>
>>       if {$dlopen} {
>> -       lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
>> +       lappend main_opts "additional_flags=-DSHLIB_NAME=\"[file tail $libobj2]\""
>>          lappend link_opts "shlib_load"
>>       } else {
>>          lappend link_opts "shlib=${libobj2}"
>> ...
>> which means that the file is found relative to $ORIGIN, as is the case for $dlopen == 0.
>>
>> I'm not entirely happy with this fix, because what we'd rather want is to use the name as returned by remote_download, but that's done in gdb_load_shlib later on (which also handles shlib_target_file, so actually, the name as returned by this proc is the one we really want), so it's not available yet at the time we do the compilation.
>>
>> I've thought about allowing gdb_load_shlib to be called without gdb instance, and for cases where there's no instance, registering the solib-search-path setting to be done at gdb start.  This will allow us to move the gdb_load_shlib calls to before the compilation, such that we can use the result to set -DSHLIB_NAME.  But I think it's a solution bound to cause confusion because things happen under the hood.
>>
>> Alternatively, we can try to not pass in the name into compilation (working around only some of the problems related to remote host testing, see https://sourceware.org/bugzilla/show_bug.cgi?id=16947), but that also has its limitations: we need to be able to patch target memory or the ability to use argv.
>>
>> Perhaps splitting up the functionality in gdb_load_shlib makes the most sense.  By default, it'll do the same as before, but by calling it twice, once with -only-download, and once with -no-download can we get the solib name before starting gdb.
>>
>> I've given this a try in patch attached below.
>>
>> Ivan, could you check if the patch fixes the gdb.base/print-file-var.exp test-case for you?
> 
> I just gave it a quick look and I'm not familiar with the entire
> problem.  But it sounds to me like it would be clearer to just make two
> procs for the two steps, instead of one proc with flags to control its
> behavior.  And perhaps a third proc that does both steps, for
> convenience (which would be called gdb_load_shlib for backwards
> compatibility, I guess).

Thanks for the review.

Two-proc approach proposed here ( 
https://sourceware.org/pipermail/gdb-patches/2022-November/193522.html ).

Thanks,
- Tom
  

On 11/7/22 14:32, Andrew Burgess wrote:
> Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>> On 10/25/22 18:29, Ivan Tetyushkin wrote:
>>> ---
>>>    gdb/testsuite/gdb.base/print-file-var.exp | 4 +++-
>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
>>> index 9abe87d7758..73137630fed 100644
>>> --- a/gdb/testsuite/gdb.base/print-file-var.exp
>>> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
>>> @@ -42,6 +42,8 @@ proc test {hidden dlopen version_id_main lang} {
>>>        set libobj1 [standard_output_file ${lib1}$suffix.so]
>>>        set libobj2 [standard_output_file ${lib2}$suffix.so]
>>>    
>>> +    set runtimelibobj2 [get_runtime_file $libobj2]
>>> +
>>>        set lib_opts { debug $lang }
>>>        lappend lib_opts "additional_flags=-DHIDDEN=$hidden"
>>>    
>>> @@ -60,7 +62,7 @@ proc test {hidden dlopen version_id_main lang} {
>>>        set link_opts [list debug shlib=${libobj1}]
>>>    
>>>        if {$dlopen} {
>>> -	lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
>>> +	lappend main_opts "additional_flags=-DSHLIB_NAME=\"$runtimelibobj2\""
>>>    	lappend link_opts "shlib_load"
>>>        } else {
>>>    	lappend link_opts "shlib=${libobj2}"
>>
>> I get this test-case passing by avoiding to use an absolute file name:
>> ...
>> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp
>> b/gdb/testsuite/gdb.base/print-file-
>> var.exp
>> index 9abe87d7758..338840cb05e 100644
>> --- a/gdb/testsuite/gdb.base/print-file-var.exp
>> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
>> @@ -60,7 +60,7 @@ proc test {hidden dlopen version_id_main lang} {
>>        set link_opts [list debug shlib=${libobj1}]
>>
>>        if {$dlopen} {
>> -       lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
>> +       lappend main_opts "additional_flags=-DSHLIB_NAME=\"[file tail
>> $libobj2]\""
>>           lappend link_opts "shlib_load"
>>        } else {
>>           lappend link_opts "shlib=${libobj2}"
>> ...
>> which means that the file is found relative to $ORIGIN, as is the case
>> for $dlopen == 0.
>>
>> I'm not entirely happy with this fix, because what we'd rather want is
>> to use the name as returned by remote_download, but that's done in
>> gdb_load_shlib later on (which also handles shlib_target_file, so
>> actually, the name as returned by this proc is the one we really want),
>> so it's not available yet at the time we do the compilation.
>>
>> I've thought about allowing gdb_load_shlib to be called without gdb
>> instance, and for cases where there's no instance, registering the
>> solib-search-path setting to be done at gdb start.  This will allow us
>> to move the gdb_load_shlib calls to before the compilation, such that we
>> can use the result to set -DSHLIB_NAME.  But I think it's a solution
>> bound to cause confusion because things happen under the hood.
>>
>> Alternatively, we can try to not pass in the name into compilation
>> (working around only some of the problems related to remote host
>> testing, see https://sourceware.org/bugzilla/show_bug.cgi?id=16947), but
>> that also has its limitations: we need to be able to patch target memory
>> or the ability to use argv.
>>
>> Perhaps splitting up the functionality in gdb_load_shlib makes the most
>> sense.  By default, it'll do the same as before, but by calling it
>> twice, once with -only-download, and once with -no-download can we get
>> the solib name before starting gdb.
>>
>> I've given this a try in patch attached below.
> 
> I also prefer the approach that Tom has suggested here.  I gave the
> patch a go and it fixed the problems in print_file-var.exp for me using
> a local/remote type setup.
> 

Thanks for confirming.

>>
>> Ivan, could you check if the patch fixes the gdb.base/print-file-var.exp
>> test-case for you?
>>
>> Thanks,
>> - Tom
>>  From b072645433f83d62a4b7cf857d3d50f0dd7e0c75 Mon Sep 17 00:00:00 2001
>> From: Tom de Vries <tdevries@suse.de>
>> Date: Sun, 6 Nov 2022 11:26:23 +0100
>> Subject: [PATCH 3/3] [gdb/testsuite] Fix gdb.base/print-file-var.exp for
>>   remote target
>>
>> ---
>>   gdb/testsuite/gdb.base/print-file-var.exp |  6 ++-
>>   gdb/testsuite/lib/gdb.exp                 | 48 +++++++++++++++++------
>>   2 files changed, 39 insertions(+), 15 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
>> index 9abe87d7758..abdbbbfdce4 100644
>> --- a/gdb/testsuite/gdb.base/print-file-var.exp
>> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
>> @@ -59,8 +59,10 @@ proc test {hidden dlopen version_id_main lang} {
>>       set main_opts [list debug $lang]
>>       set link_opts [list debug shlib=${libobj1}]
>>   
>> +    set target_libobj2 [gdb_load_shlib $libobj2 -only-download]
>> +
>>       if {$dlopen} {
>> -	lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
>> +	lappend main_opts "additional_flags=-DSHLIB_NAME=\"$target_libobj2\""
>>   	lappend link_opts "shlib_load"
>>       } else {
>>   	lappend link_opts "shlib=${libobj2}"
>> @@ -79,7 +81,7 @@ proc test {hidden dlopen version_id_main lang} {
>>   
>>       clean_restart $executable
>>       gdb_load_shlib $libobj1
>> -    gdb_load_shlib $libobj2
>> +    gdb_load_shlib $libobj2 -no-download
>>   
>>       if ![runto_main] {
>>   	return -1
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index e2cda30b95a..5328afb27e8 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -5873,24 +5873,46 @@ proc gdb_remote_download {dest fromfile {tofile {}}} {
>>   #
>>   # Copy the listed library to the target.
>>   
>> -proc gdb_load_shlib { file } {
>> +proc gdb_load_shlib { file args } {
>>       global gdb_spawn_id
> 
> The comments on this function will need updating for a final version of
> this patch.
> 
> Otherwise, looks good.

Ack, and thanks for the review.

Thanks,
- Tom
  

On 11/7/22 14:49, Ivan Tetyushkin wrote:
> Hi
> Yes, this patch fixes this test. However, it will not fix gdb.base/infcall-exec.exp from the patch 5/7 of this thread.

Hi Ivan,

thanks for reporting the problem and proposing fixes.

I went ahead and proposed a series, borrowing from yours, submitted here 
( https://sourceware.org/pipermail/gdb-patches/2022-November/193518.html ).

That test-case is also addressed there.

> As I can understand now, this test uses absolute paths for checking, but relative paths are used during compilation. We can change regexp itself and check only filename, but this could lead to false positive result.

The gdb.base/infcall-exec.exp test-case checks that an inferior function 
call which execs a new program is handled ok.  This is the main function 
of the test, and absolute vs relative paths is not an interesting aspect 
to check.

> Also, I thought these tests are created to work with absolute paths to ignore solib-search-path.

Maybe with local target, but for remote target we do use solib-search-path.

The overall problem is that there's no reliable way to do construct an 
absolute path on a remote target (which could f.i. be using \ instead of 
/ as path separator), or at least not an easy one.

We could do something like using remote_exec for "echo \"puts [file 
normalize .$file]\" | tclsh " but that requires tcl on the target.

So AFAIU, until dejagnu decided to support that natively, we just have 
to use the filenames as returned by remote_download.

On a different topic, do you have a copyright assignment?

Assuming you don't, with this submission you've probably used up your 
legally non-significant contribution amount, and for a new submission 
you'd need a copyright assignment (and unfortunately, it can take some 
time to get it done).

Thanks,
- Tom
  

On 11/7/22 17:44, Tom de Vries wrote:
> On a different topic, do you have a copyright assignment?
> 
> Assuming you don't, with this submission you've probably used up your 
> legally non-significant contribution amount, and for a new submission 
> you'd need a copyright assignment (and unfortunately, it can take some 
> time to get it done).

Hi Ivan,

I've committed my series.

any update on the copyright assignment? You already have one? You need 
help getting one?

Thanks,
- Tom