[PATCH 0/7] introduce get_runtime_path

[PATCH 0/7] introduce get_runtime_path

[Ivan Tetyushkin]

Hi. I tried to run a testsuite using a remote host test setup on QEMU
(RISC-V) with Linux.
Some tests that passed on local machine are failed due to hardcoded pathes in tests.
For example, see gdb.base/print-file-var.exp. This test has SHLIB_NAME
that is an absolute path on local machine, and of course this file does not
exist on remote machine. To fix this, a function get_runtime_path was created.

These patches introduce this function and some tests that could be fixed
by changing an absolute path on local machine to a runtime path.

Ivan Tetyushkin (7):
  [gdb/testsuite] Adding function to find runtime path to support remote
    execution for testsuite
  [gdb/testsuite] fix test gdb.base/print-file-var.exp for remote
    execution
  [gdb/testsuite] fix test gdb.base/jit-reader-exec.exp for remote
    execution
  [gdb/testsuite] fix test gdb.base/solib-vanish.exp for remote
    execution
  [gdb/testsuite] fix test gdb.base/infcall-exec.exp for remote
    execution
  [gdb/testsuite] fix test gdb.base/info-shared.exp for remote execution
  [gdb/testsuite] fix test gdb.base/jit-elf-so.exp for remote execution

 gdb/testsuite/gdb.base/infcall-exec.exp    |  7 +++++--
 gdb/testsuite/gdb.base/info-shared.exp     |  9 +++++++--
 gdb/testsuite/gdb.base/jit-elf-so.exp      |  6 ++++--
 gdb/testsuite/gdb.base/jit-reader-exec.exp |  5 ++++-
 gdb/testsuite/gdb.base/print-file-var.exp  |  4 +++-
 gdb/testsuite/gdb.base/solib-vanish.exp    |  7 ++++++-
 gdb/testsuite/lib/gdb.exp                  | 12 ++++++++++++
 7 files changed, 41 insertions(+), 9 deletions(-)

-- 
2.38.1

[PATCH 1/7] [gdb/testsuite] Adding function to find runtime path to support remote execution for testsuite

[Ivan Tetyushkin]

---
 gdb/testsuite/lib/gdb.exp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index e2cda30b95a..ee25daadac5 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -6200,6 +6200,18 @@ proc standard_output_file {basename} {
     return [file join $dir $basename]
 }
 
+# Turns a full file name in a name for runtime context.
+# This is needed as on remote folder structure is different
+proc get_runtime_file {fullfile} {
+    if [is_remote target] {
+        set remotedir [board_info target remotedir]
+        set filename [file tail $fullfile]
+        return [file join $remotedir $filename]
+    } else {
+        return $fullfile
+    }
+}
+
 # Turn BASENAME into a full file name in the standard output directory.  If
 # GDB has been launched more than once then append the count, starting with
 # a ".1" postfix.
-- 
2.38.1

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

[Ivan Tetyushkin]

---
 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}"
-- 
2.38.1

[PATCH 3/7] [gdb/testsuite] fix test gdb.base/jit-reader-exec.exp for remote execution

[Ivan Tetyushkin]

---
 gdb/testsuite/gdb.base/jit-reader-exec.exp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/jit-reader-exec.exp b/gdb/testsuite/gdb.base/jit-reader-exec.exp
index c5ee8ef89a3..8b03b24f1e6 100644
--- a/gdb/testsuite/gdb.base/jit-reader-exec.exp
+++ b/gdb/testsuite/gdb.base/jit-reader-exec.exp
@@ -26,8 +26,9 @@ standard_testfile jit-reader-exec.c
 set testfile2 "jit-reader-execd"
 set srcfile2 ${testfile2}.c
 set binfile2 [standard_output_file ${testfile2}]
+set runtimefile2 [get_runtime_file $binfile2]
 
-set compile_options [list debug additional_flags=-DPROGRAM=\"$binfile2\"]
+set compile_options [list debug additional_flags=-DPROGRAM=\"$runtimefile2\"]
 
 if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
 	 executable $compile_options] != ""} {
@@ -43,6 +44,8 @@ if {[gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" \
 
 clean_restart $binfile
 
+gdb_remote_download target $binfile2
+
 if {![runto_main]} {
     return
 }
-- 
2.38.1

[PATCH 4/7] [gdb/testsuite] fix test gdb.base/solib-vanish.exp for remote execution

[Ivan Tetyushkin]

---
 gdb/testsuite/gdb.base/solib-vanish.exp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/solib-vanish.exp b/gdb/testsuite/gdb.base/solib-vanish.exp
index 5f47f2a80fd..9116f1914a2 100644
--- a/gdb/testsuite/gdb.base/solib-vanish.exp
+++ b/gdb/testsuite/gdb.base/solib-vanish.exp
@@ -67,6 +67,7 @@ set lib2_flags {debug}
 set lib1name "solib-vanish-lib1"
 set srcfile_lib1 ${srcdir}/${subdir}/${lib1name}.c
 set binfile_lib1 [standard_output_file ${lib1name}.so]
+set runtimebinfile_lib1 [get_runtime_file $binfile_lib1]
 set lib1_flags [list debug shlib=${binfile_lib2}]
 
 # Main program
@@ -74,7 +75,7 @@ set testfile "solib-vanish-main"
 set srcfile ${srcdir}/${subdir}/${testfile}.c
 set executable ${testfile}
 set binfile [standard_output_file ${executable}]
-set bin_flags [list debug shlib_load additional_flags=-DVANISH_LIB=\"${binfile_lib1}\"]
+set bin_flags [list debug shlib_load additional_flags=-DVANISH_LIB=\"${runtimebinfile_lib1}\"]
 
 if { [gdb_compile_shlib ${srcfile_lib2} ${binfile_lib2} $lib2_flags] != ""
      || [gdb_compile_shlib ${srcfile_lib1} ${binfile_lib1} $lib1_flags] != ""
@@ -85,6 +86,10 @@ if { [gdb_compile_shlib ${srcfile_lib2} ${binfile_lib2} $lib2_flags] != ""
 
 clean_restart $testfile
 
+# load shlib to target
+gdb_load_shlib $binfile_lib1
+gdb_load_shlib $binfile_lib2
+
 if { ![runto_main] } {
     return
 }
-- 
2.38.1

[PATCH 5/7] [gdb/testsuite] fix test gdb.base/infcall-exec.exp for remote execution

[Ivan Tetyushkin]

---
 gdb/testsuite/gdb.base/infcall-exec.exp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/infcall-exec.exp b/gdb/testsuite/gdb.base/infcall-exec.exp
index e8f6a218c36..514dc70a4d9 100644
--- a/gdb/testsuite/gdb.base/infcall-exec.exp
+++ b/gdb/testsuite/gdb.base/infcall-exec.exp
@@ -21,6 +21,7 @@ standard_testfile
 set testfile2 "infcall-exec2"
 set srcfile2 "${testfile2}.c"
 set binfile2 [standard_output_file $testfile2]
+set runtimebinfile2 [get_runtime_file $binfile2]
 
 # Build the two executables for the test.
 if {[gdb_compile $srcdir/$subdir/$srcfile $binfile executable debug] != ""} {
@@ -33,15 +34,17 @@ if {[gdb_compile $srcdir/$subdir/$srcfile2 $binfile2 executable debug] != ""} {
 }
 
 clean_restart $binfile
+gdb_remote_download target $binfile
+gdb_remote_download target $binfile2
 
 if {![runto_main]} {
     return -1
 }
 
-set expected_result "process $decimal is executing new program: $binfile2"
+set expected_result "process $decimal is executing new program: $runtimebinfile2"
 append expected_result "\[\r\n\]+.*"
 append expected_result "Breakpoint 1, main .*at .*$srcfile2:$decimal"
 append expected_result ".*"
 
-gdb_test "call (int) execlp \(\"$binfile2\", \"$binfile2\", \(char \*\)0\)" \
+gdb_test "call (int) execlp \(\"$runtimebinfile2\", \"$runtimebinfile2\", \(char \*\)0\)" \
     $expected_result "call execlp"
-- 
2.38.1

[PATCH 6/7] [gdb/testsuite] fix test gdb.base/info-shared.exp for remote execution

[Ivan Tetyushkin]

---
 gdb/testsuite/gdb.base/info-shared.exp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/info-shared.exp b/gdb/testsuite/gdb.base/info-shared.exp
index 290ae5793dc..9b400e07649 100644
--- a/gdb/testsuite/gdb.base/info-shared.exp
+++ b/gdb/testsuite/gdb.base/info-shared.exp
@@ -22,12 +22,14 @@ standard_testfile
 set lib1name $testfile-solib1
 set srcfile_lib1 $srcdir/$subdir/$lib1name.c
 set binfile_lib1 [standard_output_file $lib1name.so]
-set define1 -DSHLIB1_NAME=\"$binfile_lib1\"
+set runtime_binfile_lib1 [get_runtime_file $binfile_lib1]
+set define1 -DSHLIB1_NAME=\"$runtime_binfile_lib1"
 
 set lib2name $testfile-solib2
 set srcfile_lib2 $srcdir/$subdir/$lib2name.c
 set binfile_lib2 [standard_output_file $lib2name.so]
-set define2 -DSHLIB2_NAME=\"$binfile_lib2\"
+set runtime_binfile_lib2 [get_runtime_file $binfile_lib2]
+set define2 -DSHLIB2_NAME=\"$runtime_binfile_lib2"
 
 if { [gdb_compile_shlib $srcfile_lib1 $binfile_lib1 {}] != "" } {
     untested "failed to compile shared library 1"
@@ -45,6 +47,9 @@ if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
     return -1
 }
 
+gdb_load_shlib $binfile_lib1
+gdb_load_shlib $binfile_lib2
+
 # Run "info sharedlibrary" and check for the presence or absence of
 # our libraries.
 proc check_info_shared { test expect1 expect2 } {
-- 
2.38.1

[PATCH 7/7] [gdb/testsuite] fix test gdb.base/jit-elf-so.exp for remote execution

[Ivan Tetyushkin]

---
 gdb/testsuite/gdb.base/jit-elf-so.exp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/jit-elf-so.exp b/gdb/testsuite/gdb.base/jit-elf-so.exp
index dcc9afc36b9..566e9ef0673 100644
--- a/gdb/testsuite/gdb.base/jit-elf-so.exp
+++ b/gdb/testsuite/gdb.base/jit-elf-so.exp
@@ -90,7 +90,8 @@ proc one_jit_test {solib_binfiles_target match_str} {
 	gdb_breakpoint [gdb_get_line_number "break here before-dlopen" \
 			    $main_loader_srcfile]
 	gdb_continue_to_breakpoint "break here before-dlopen"
-	gdb_test_no_output "set var jit_libname = \"$main_solib_binfile\"" \
+	set runtime_main_solib_binfile [get_runtime_file $main_solib_binfile]
+	gdb_test_no_output "set var jit_libname = \"$runtime_main_solib_binfile\"" \
 	    "setting library name"
 
 	gdb_breakpoint [gdb_get_line_number "break here after-dlopen" \
@@ -159,8 +160,9 @@ foreach solib $jit_solibs_target {
     # We don't intend to load the .so as a JIT debuginfo reader, but we
     # need some handy file name for a completion test.
     set input [string range $solib 0 [expr { [string length $solib] - 2 }]]
+    set runtime_solib [get_runtime_file $input]
     gdb_test \
 	"complete jit-reader-load [standard_output_file $input]" \
-	"jit-reader-load $solib" \
+	"jit-reader-load $runtime_solib" \
 	"test jit-reader-load filename completion [file tail $solib]"
 }
-- 
2.38.1

Re: [PATCH 0/7] introduce get_runtime_path

[Andrew Burgess]

Ivan Tetyushkin <ivan.tetyushkin@syntacore.com> writes:

> Hi. I tried to run a testsuite using a remote host test setup on QEMU
> (RISC-V) with Linux.

Thanks for looking at these problems.  I suspect the setup you are using
is not one that is used very often.

Could you give more details about your setup so that others might be
able to reproduce the issues you are seeing, and validate your fixes?

Thanks,
Andrew


> Some tests that passed on local machine are failed due to hardcoded pathes in tests.
> For example, see gdb.base/print-file-var.exp. This test has SHLIB_NAME
> that is an absolute path on local machine, and of course this file does not
> exist on remote machine. To fix this, a function get_runtime_path was created.
>
> These patches introduce this function and some tests that could be fixed
> by changing an absolute path on local machine to a runtime path.
>
> Ivan Tetyushkin (7):
>   [gdb/testsuite] Adding function to find runtime path to support remote
>     execution for testsuite
>   [gdb/testsuite] fix test gdb.base/print-file-var.exp for remote
>     execution
>   [gdb/testsuite] fix test gdb.base/jit-reader-exec.exp for remote
>     execution
>   [gdb/testsuite] fix test gdb.base/solib-vanish.exp for remote
>     execution
>   [gdb/testsuite] fix test gdb.base/infcall-exec.exp for remote
>     execution
>   [gdb/testsuite] fix test gdb.base/info-shared.exp for remote execution
>   [gdb/testsuite] fix test gdb.base/jit-elf-so.exp for remote execution
>
>  gdb/testsuite/gdb.base/infcall-exec.exp    |  7 +++++--
>  gdb/testsuite/gdb.base/info-shared.exp     |  9 +++++++--
>  gdb/testsuite/gdb.base/jit-elf-so.exp      |  6 ++++--
>  gdb/testsuite/gdb.base/jit-reader-exec.exp |  5 ++++-
>  gdb/testsuite/gdb.base/print-file-var.exp  |  4 +++-
>  gdb/testsuite/gdb.base/solib-vanish.exp    |  7 ++++++-
>  gdb/testsuite/lib/gdb.exp                  | 12 ++++++++++++
>  7 files changed, 41 insertions(+), 9 deletions(-)
>
> -- 
> 2.38.1

Re: [PATCH 0/7] introduce get_runtime_path

[Ivan Tetyushkin]

Hi

> Could you give more details about your setup so that others might be
> able to reproduce the issues you are seeing, and validate your fixes?
To reproduce my setup please:

0) Setup environment
0.1) Determine QEMU-related variables:
# QEMU_SSH_PORT - port is used for no-password access with ssh.
# QEMU_WORK_PORT - port used by gdb to send commands to gdbserver.

export QEMU_WORK_PORT=1111
export QEMU_SSH_PORT=2222

# HOSTNAME - name, used for connection with ssh.
export HOSTNAME=debian

# QEMU_ROOT_USER, QEMU_ROOT_PASS, QEMU_USER, QEMU_PASS - name and password for use in QEMU
# In suggested to download image:
export QEMU_ROOT_USER=root
export QEMU_ROOT_PASS=root
export QEMU_USER=debian
export QEMU_PASS=debian

0.2) Determine paths:
# for easier setup, you may want to use:
# BASEDIR - default parent directory for all paths here
export BASEDIR=~/BASEDIR

# QEMU_DIR - full path to folder where you want to have QEMU-related files.
export QEMU_DIR=$BASEDIR/qemu_dir
# BUILDDIR - full path to place where you want configure & build to be done
export BUILDDIR=$BASEDIR/gdb-build
# INSTALL_DIR - full path to install gdb
export INSTALL_DIR=$BASEDIR/gdb-install
# gdb_source - full path to binutils-gdb repo with configure
export gdb_source=$BASEDIR/binutils-gdb

# Optional
# dejagnu_dir - full path where you want to have dejagnu, if you do not have it already
export dejagnu_dir=$BASEDIR

0.3) Make sure that all directories exist.
mkdir --parents $BASEDIR
mkdir --parents $QEMU_DIR
mkdir --parents $BUILDDIR

0.4) Make sure you have required apps in your path.
apt update
apt-get install -y gcc-riscv64-linux-gnu
apt-get install -y g++-riscv64-linux-gnu
apt-get install -y qemu-system-riscv64
apt-get install -y u-boot-qemu opensbi


1) Prepare QEMU with Linux.
1.1) Download Debian-based image for RISCV QEMU:
url: https://gitlab.com/api/v4/projects/giomasce%2Fdqib/jobs/artifacts/master/download?job=convert_riscv64-virt

wget "https://gitlab.com/api/v4/projects/giomasce%2Fdqib/jobs/artifacts/master/download?job=convert_riscv64-virt" -O "$QEMU_DIR/debian-rv64.zip"

1.2) Unpack & prepare QEMU files for work:

unzip "$QEMU_DIR/debian-rv64.zip" -d "$QEMU_DIR/debian_tmp"
mv "$QEMU_DIR/debian_tmp/artifacts" "$QEMU_DIR/debian-rv64"

1.3) Create image overlay file (overlay.qcow2) for QEMU:

qemu-img create -o backing_file="$QEMU_DIR/debian-rv64/image.qcow2",backing_fmt=qcow2 -f qcow2 "$QEMU_DIR/overlay.qcow2"

1.4) Run QEMU:

qemu-system-riscv64 \
-machine virt \
-smp 8 \
-cpu rv64 \
-m 8G \
-device virtio-blk-device,drive=hd \
-drive file=$QEMU_DIR/overlay.qcow2,if=none,id=hd \
-device virtio-net-device,netdev=net \
-netdev user,id=net,hostfwd=tcp::$QEMU_SSH_PORT-:22,hostfwd=tcp::$QEMU_WORK_PORT-:1111 \
-bios /usr/lib/riscv64-linux-gnu/opensbi/generic/fw_jump.elf \
-kernel /usr/lib/u-boot/qemu-riscv64_smode/uboot.elf \
-object rng-random,filename=/dev/urandom,id=rng \
-device virtio-rng-device,rng=rng \
-daemonize

1.5) Add ssh config to be able to connect without password:

To your .ssh/config write (if you changed earlier environment variables, please, change there too):
Host debian
  HostName 127.0.0.1
  User debian
  Port 2222

check that ssh connection works:
# adding to the list of known hosts (may need to be done twice due to reset of connection).
ssh $HOSTNAME

If you do not have ssh keys, you need to generate them:
ssh-keygen -t rsa -N '' -f ~/.ssh/id_rsa <<< y;

Copy you ssh public keys to QEMU:
sshpass -p $QEMU_PASS ssh $HOSTNAME "mkdir --parents /home/$QEMU_USER/.ssh"
sshpass -p $QEMU_PASS scp ~/.ssh/id_rsa.pub $HOSTNAME:/home/$QEMU_USER/.ssh/authorized_keys

Now you should be able to connect to QEMU without password:
ssh $HOSTNAME

1.6) Install gdbserver on QEMU:

# adding to the list of known hosts. No need to put password
ssh $QEMU_ROOT_USER@localhost -p $QEMU_SSH_PORT

sshpass -p $QEMU_ROOT_PASS ssh $QEMU_ROOT_USER@localhost -p $QEMU_SSH_PORT
# in QEMU
apt update
apt install gdbserver
exit

2) Prepare binutils-gdb

2.0) If you want clean repo and used $BASEDIR structure, you may want to clone it from the sourceware repo to $BASEDIR.
cd $BASEDIR; git clone https://sourceware.org/git/binutils-gdb.git

2.1) Create board settings file. Setup usage riscv64-linux-gnu-gcc and remote execution.

Create new file NEW_BOARD.exp in $gdb_source/gdb/testsuite/boards (if you changed HOSTNAME or QEMU_PORT, change here as well):

load_generic_config "gdbserver"
load_board_description "gdbserver-base"

process_multilib_options ""

set_board_info compiler riscv64-linux-gnu-gcc
set_board_info c++compiler riscv64-linux-gnu-g++

set GDBSERVER "/usr/bin/gdbserver"
set_board_info gdb_server_prog "/usr/bin/gdbserver"

set_board_info hostname debian

set_board_info gdb,socketport 1111
set_board_info portnum 1111


set_board_info rsh_prog /usr/bin/ssh
set_board_info rcp_prog /usr/bin/scp
set_board_info file_transfer ftp
set_board_info protocol standard

set_board_info gdb_protocol "remote"

set_board_info use_gdb_stub 1
set_board_info gdb,do_reload_on_run 1
set_board_info noargs 1
set_board_info gdb,noinferiorio 1
set_board_info gdb,no_hardware_watchpoints 1

2.2) call binutils-gdb configure

cd $BUILDDIR; $gdb_source/configure \
--prefix=$INSTALL_DIR \
--with-expat=yes \
--target=riscv64-linux-gnu

2.3) build gdb
cd $BUILDDIR; make all-gdb


3) Download and run tests to reproduce the problem.
3.1) If you do not have dejagnu in the path, you need to download it and add to the path:
cd $dejagnu_dir
git clone  https://git.savannah.gnu.org/git/dejagnu.git
export PATH=$PATH:$(realpath ./)/dejagnu

3.2) Run test and observe failure without my patches
cd $BUILDDIR/gdb
make check RUNTESTFLAGS="--target_board=NEW_BOARD gdb.base/print-file-var.exp"

--
Ivan

Re: [PATCH 0/7] introduce get_runtime_path

[Tom de Vries]

On 10/29/22 11:20, Andrew Burgess via Gdb-patches wrote:
> Ivan Tetyushkin <ivan.tetyushkin@syntacore.com> writes:
> 
>> Hi. I tried to run a testsuite using a remote host test setup on QEMU
>> (RISC-V) with Linux.
> 
> Thanks for looking at these problems.  I suspect the setup you are using
> is not one that is used very often.
> 
> Could you give more details about your setup so that others might be
> able to reproduce the issues you are seeing, and validate your fixes?
> 

Hi,

I found a fairly easy way to reproduce this (after reading the response 
to Andrews question and realizing that given the use of --target_board 
rather than --host_board, from the point of view of dejagnu, this is not 
a remote host, but a remote target setup).

Add a local test account remote-target, and put a remote-target entry in 
the .ssh/config:
...
Host remote-target
      HostName  127.0.0.1
      User remote-target
      IdentityFile <some rsa file>
      IdentitiesOnly yes
      PreferredAuthentications publickey
...
such that we can access it without password:
...
$ ssh remote-target
Last login: Sat Nov  5 15:12:34 2022 from 127.0.0.1
Have a lot of fun...
remote-target@127.0.0.1:~>
...
and likewise for remote-target@remote-target.

Edit remote-gdbserver-on-localhost.exp to use the remote-target account:
...
  set_board_info rcp_prog "/usr/bin/scp"
  set_board_info rsh_prog "/usr/bin/ssh"
  set_board_info protocol standard
-set_board_info username $env(USER)
-set_board_info hostname localhost
+set_board_info username remote-target
+set_board_info hostname remote-target

  proc ${board}_spawn { board cmd } {
      global board_info
...

Run test-case gdb.base/print-file-var.exp with target board 
remote-gdbserver-on-localhost.exp, and watch it pass.

Now make sure the remote-target account cannot access files from the 
$USER account (well, as far as the testsuite output dir is concerned):
...
$ chmod go-rx build/gdb/testsuite/outputs/
...
mimicking an actual remote target machine not being able to access the 
files on the build/host machine.

Run the test again, and watch it fail.

Now apply the patch series, and watch it pass again.

Thanks,
- Tom

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

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 3416 bytes --]

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

[-- Attachment #2: 0003-gdb-testsuite-Fix-gdb.base-print-file-var.exp-for-re.patch --]
[-- Type: text/x-patch, Size: 3455 bytes --]

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
 
-    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

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

[Andrew Burgess]

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

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

[Ivan Tetyushkin]

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

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

[Simon Marchi]

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

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

[Tom de Vries]

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

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

[Tom de Vries]

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

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

[Tom de Vries]

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

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

[Tom de Vries]

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