Re: [PATCH v3 2/2] `add-symbol-file`: add test for mixing -o and -s

[Keith Seitz <keiths@redhat.com>]

Hi,

Thank you for the test case! See my comments below.

On 9/23/26 8:51 PM, Dragorn421 wrote:
> diff --git a/gdb/testsuite/gdb.base/relocate_linked.c b/gdb/testsuite/gdb.base/relocate_linked.c
> new file mode 100644
> index 00000000000..ac48ea28dc0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/relocate_linked.c
> @@ -0,0 +1,28 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2026 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +int
> +text ()
> +{
> +  return 0;
> +}
> +
> +int data = 1;
> +
> +const int rodata = 2;
> +
> +int bss;
> diff --git a/gdb/testsuite/gdb.base/relocate_linked.exp b/gdb/testsuite/gdb.base/relocate_linked.exp
> new file mode 100644
> index 00000000000..64c25dfab7b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/relocate_linked.exp
> @@ -0,0 +1,58 @@
> +# Copyright 2026 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +# relocate_linked.exp -- Expect script to test loading symbols from linked
> +#		  relocatable object files (also known as partially linked files).
> +
> +standard_testfile .c
> +
> +# plf stands for "partially linked file"
> +remote_exec build "rm -f ${binfile}.o ${binfile}.plf"
> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}.o" object {debug}] != "" } {
> +    untested "failed to compile"
> +    return
> +}
> +if { [target_link "${binfile}.o" "${binfile}.plf" --relocatable] != "" } {
> +    untested "failed to partially link"
> +    return
> +}
> +

Unfortunately, using target_link is problematic. It invokes bare ld
while ignoring build flags such as -m32. When running the test suite
in 32-bit mode:

$ make check RUNTESTFLAGS="--target_board unix/-m32" 
TESTS=gdb.base/relocate_linked.exp
...
ld: relocatable linking with relocations from format elf32-i386
(...) to format elf64-x86-64 (...) is not supported
UNTESTED: gdb.base/relocate_linked.exp: failed to partially link

It would be better to drive this through gdb_compile so that any
target multilib flags are applied. Unfortunately, this gets a little
complicated because of linking with the math library. I think it best
to follow gdb.base/nostdlib.exp. Something like this on top of your
patch seems a safer (and broader) approach:

diff --git a/gdb/testsuite/gdb.base/relocate_linked.exp 
b/gdb/testsuite/gdb.base/relocate_linked.exp
index 64c25dfab7b..69d715c86eb 100644
--- a/gdb/testsuite/gdb.base/relocate_linked.exp
+++ b/gdb/testsuite/gdb.base/relocate_linked.exp
@@ -24,7 +24,21 @@ if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" 
"${binfile}.o" object {debug}
      untested "failed to compile"
      return
  }
-if { [target_link "${binfile}.o" "${binfile}.plf" --relocatable] != "" } {
+
+set board [target_info name]
+if {[board_info $board exists mathlib]} {
+    set saved_mathlib [board_info $board mathlib]
+    set_board_info mathlib ""
+    set err [gdb_compile "${binfile}.o" "${binfile}.plf" executable \
+		 {additional_flags=-nostdlib ldflags=-Wl,-r}]
+    set_board_info mathlib $saved_mathlib
+} else {
+    set_board_info mathlib ""
+    set err [gdb_compile "${binfile}.o" "${binfile}.plf" executable \
+		 {additional_flags=-nostdlib ldflags=-Wl,-r}]
+    unset_board_info mathlib
+}
+if { $err != "" } {
      untested "failed to partially link"
      return
  }

[I just copied that from nostdlib.exp.]

> +# Check that the -s address is respected when -o is used.
> +clean_restart
> +set offset 0xff000000
> +set offset_re 0x0*ff000000
> +set data 0x12340
> +set data_re 0x0*12340
> +set saw_data_section_with_correct_address 0
> +gdb_test_multiple "add-symbol-file $binfile.plf -o $offset -s .data $data" \
> +    "add-symbol-file with offset and explicit data section address" {
> +    -re "add symbol table from file .*${testfile}\\.plf.*at\[ \\t\\r\\n\]+\\.data_addr = ${data_re}\[\\r\\n\]+with other sections offset by ${offset_re}\[\\r\\n\]+\\(y or n\\) " {
> +	send_gdb "y\n"
> +	exp_continue
> +    }
> +    -re "Reading symbols from .*\\.\\.\\.\\r\\n$gdb_prompt " {
> +	send_gdb "maint info sections -all-objects\n"
> +	exp_continue
> +    }
> +    -re "$data_re->$hex .*: \\.data" {
> +	set saw_data_section_with_correct_address 1
> +	exp_continue
> +    }
> +    -re -wrap "" {
> +	gdb_assert { $saw_data_section_with_correct_address } \
> +	    "explicit .data section address is preserved"
> +	pass $gdb_test_name
> +    }
> +}

This final 'pass' emits a misleading PASS. On an unpatched gdb, for
example, we will mark both a FAIL and a PASS. gdb_assert will issue
PASS/FAIL messages on its own, so I don't think this "pass
$gdb_test_name" is needed.

Keith