Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Fix `add-symbol-file -o ... -s ...` address mapping bug
@ 2026-09-24  3:51 Dragorn421
  2026-09-24  3:51 ` [PATCH v3 1/2] " Dragorn421
  2026-09-24  3:51 ` [PATCH v3 2/2] `add-symbol-file`: add test for mixing -o and -s Dragorn421
  0 siblings, 2 replies; 7+ messages in thread
From: Dragorn421 @ 2026-09-24  3:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: Dragorn421

Hi!

This fixes a bug when mixing -o and -s in the add-symbol-file command,
and adds a corresponding test.

Initial patch thread here: https://sourceware.org/pipermail/gdb-patches/2026-September/230131.html

v3 over v2 fixes the added test case.

This is my first time contributing so my apologies if I'm doing anything
not properly, please let me know.

Dragorn421 (2):
  Fix `add-symbol-file -o ... -s ...` address mapping bug
  `add-symbol-file`: add test for mixing -o and -s

 gdb/symfile.c                              |  6 +--
 gdb/testsuite/gdb.base/relocate_linked.c   | 28 +++++++++++
 gdb/testsuite/gdb.base/relocate_linked.exp | 58 ++++++++++++++++++++++
 3 files changed, 89 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/relocate_linked.c
 create mode 100644 gdb/testsuite/gdb.base/relocate_linked.exp

-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/2] Fix `add-symbol-file -o ... -s ...` address mapping bug
  2026-09-24  3:51 [PATCH v3 0/2] Fix `add-symbol-file -o ... -s ...` address mapping bug Dragorn421
@ 2026-09-24  3:51 ` Dragorn421
  2026-09-24 18:54   ` Keith Seitz
  2026-09-24  3:51 ` [PATCH v3 2/2] `add-symbol-file`: add test for mixing -o and -s Dragorn421
  1 sibling, 1 reply; 7+ messages in thread
From: Dragorn421 @ 2026-09-24  3:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: Dragorn421

The 'add-symbol-file' command allows users to map sections explicitly
("-s SECTION ADDR") and apply an offset ("-o OFFSET"). The documentation
explains:

   If an optional @var{offset} is specified, it is added to the start
   address of each section, except those for which the address was
   specified explicitly.

But those two options currently may not be supplied together in the
same command invocation without encountering buggy behavior.

For example if mapping the .text section from main.o at 0x1234:

```
(gdb) add-symbol-file main.o -s .text 0x1234
add symbol table from file "main.o" at
          .text_addr = 0x1234
(y or n) y
Reading symbols from main.o...
(No debugging symbols found in main.o)
(gdb) info files
...
          0x0000000000001030 - 0x0000000000001040 is .plt.got
          0x0000000000001234 - 0x000000000000132c is .text
          0x0000000000001138 - 0x0000000000001145 is .fini
...
```

`info files` does then report the expect address for .text

However this breaks when combining with the -o "set offset for other
unspecified sections" option:

```
(gdb) add-symbol-file main.o -o 0xFF000000 -s .text 0x1234
add symbol table from file "main.o" at
          .text_addr = 0x1234
with other sections offset by 0xff000000
(y or n) y
Reading symbols from main.o...
(No debugging symbols found in main.o)
(gdb) info files
...
          0x00000000ff001030 - 0x00000000ff001040 is .plt.got
          0x0000000000001040 - 0x0000000000001138 is .text
          0x00000000ff001138 - 0x00000000ff001145 is .fini
...
```

We notice here the -o option was correctly used per the addresses of
e.g. the `.fini` section, but the .text section address is now wrong.

This behavior boils down to the `set_objfile_default_section_offset`
function assuming it can pass 0 as `objfile_relocate`'s `offsets`
entries to not modify the `-s` mappings previously set by the
`symbol_file_add` call in `add_symbol_file_command`. When in fact
`objfile_relocate` does not check for 0, it only checks for the new
offset being the same as the current one.

The fix is to change `set_objfile_default_section_offset` to pass
the current offset instead of 0 for sections that should be unaltered.
---
 gdb/symfile.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 017f7a49d8d..5691c2a46d9 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2139,8 +2139,8 @@ set_objfile_default_section_offset (struct objfile *objf,
     = addrs_section_sort (objf_addrs);
 
   /* Walk the BFD section list, and if a matching section is found in
-     ADDRS_SORTED_LIST, set its offset to zero to keep its address
-     unchanged.
+     ADDRS_SORTED_LIST, set its offset to its current offset to keep
+     its address unchanged.
 
      Note that both lists may contain multiple sections with the same
      name, and then the sections from ADDRS are matched in BFD order
@@ -2163,7 +2163,7 @@ set_objfile_default_section_offset (struct objfile *objf,
 	}
 
       if (cmp == 0)
-	offsets[objf_sect->sectindex] = 0;
+	offsets[objf_sect->sectindex] = objf->section_offsets[objf_sect->sectindex];
     }
 
   /* Apply the new section offsets.  */
-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 2/2] `add-symbol-file`: add test for mixing -o and -s
  2026-09-24  3:51 [PATCH v3 0/2] Fix `add-symbol-file -o ... -s ...` address mapping bug Dragorn421
  2026-09-24  3:51 ` [PATCH v3 1/2] " Dragorn421
@ 2026-09-24  3:51 ` Dragorn421
  2026-09-24 18:57   ` Keith Seitz
  1 sibling, 1 reply; 7+ messages in thread
From: Dragorn421 @ 2026-09-24  3:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: Dragorn421

The test makes use of a built .plf (partially linked file) file,
which is needed to exhibit the previously fixed bug, as unlinked
.o object files only have 0 as address for their sections.
---
 gdb/testsuite/gdb.base/relocate_linked.c   | 28 +++++++++++
 gdb/testsuite/gdb.base/relocate_linked.exp | 58 ++++++++++++++++++++++
 2 files changed, 86 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/relocate_linked.c
 create mode 100644 gdb/testsuite/gdb.base/relocate_linked.exp

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
+}
+
+# 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
+    }
+}
-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] Fix `add-symbol-file -o ... -s ...` address mapping bug
  2026-09-24  3:51 ` [PATCH v3 1/2] " Dragorn421
@ 2026-09-24 18:54   ` Keith Seitz
  2026-09-25  7:15     ` Dragorn421
  0 siblings, 1 reply; 7+ messages in thread
From: Keith Seitz @ 2026-09-24 18:54 UTC (permalink / raw)
  To: Dragorn421, gdb-patches

Hi,

Thank you for the revision. This looks good, I just note one
tiny nit.

On 9/23/26 8:51 PM, Dragorn421 wrote:
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 017f7a49d8d..5691c2a46d9 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -2163,7 +2163,7 @@ set_objfile_default_section_offset (struct objfile *objf,
>   	}
>   
>         if (cmp == 0)
> -	offsets[objf_sect->sectindex] = 0;
> +	offsets[objf_sect->sectindex] = objf->section_offsets[objf_sect->sectindex];

This line is now too long. Suggest moving "= ..." to the next line as
per our usual coding convention. No need to repost this patch.

Reviewed-By: Keith Seitz <keiths@redhat.com>

Keith

>       }
>   
>     /* Apply the new section offsets.  */


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] `add-symbol-file`: add test for mixing -o and -s
  2026-09-24  3:51 ` [PATCH v3 2/2] `add-symbol-file`: add test for mixing -o and -s Dragorn421
@ 2026-09-24 18:57   ` Keith Seitz
  0 siblings, 0 replies; 7+ messages in thread
From: Keith Seitz @ 2026-09-24 18:57 UTC (permalink / raw)
  To: Dragorn421, gdb-patches

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] Fix `add-symbol-file -o ... -s ...` address mapping bug
  2026-09-24 18:54   ` Keith Seitz
@ 2026-09-25  7:15     ` Dragorn421
  2026-09-25 17:06       ` Keith Seitz
  0 siblings, 1 reply; 7+ messages in thread
From: Dragorn421 @ 2026-09-25  7:15 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches

On 9/24/26 20:54, Keith Seitz wrote:
> Hi,
>
> Thank you for the revision. This looks good, I just note one
> tiny nit.
>
> On 9/23/26 8:51 PM, Dragorn421 wrote:
>> diff --git a/gdb/symfile.c b/gdb/symfile.c
>> index 017f7a49d8d..5691c2a46d9 100644
>> --- a/gdb/symfile.c
>> +++ b/gdb/symfile.c
>> @@ -2163,7 +2163,7 @@ set_objfile_default_section_offset (struct 
>> objfile *objf,
>>       }
>>           if (cmp == 0)
>> -    offsets[objf_sect->sectindex] = 0;
>> +    offsets[objf_sect->sectindex] = 
>> objf->section_offsets[objf_sect->sectindex];
>
> This line is now too long. Suggest moving "= ..." to the next line as
> per our usual coding convention. No need to repost this patch. 


Thanks for the review.

What do you mean by "No need to repost this patch."? Should I just send 
the other (test-adding) patch on its own from now on?

So will a maintainer possibly take care of formatting that too-long line 
after applying the patch?


> Reviewed-By: Keith Seitz <keiths@redhat.com>
> Keith
>
>>       }
>>       /* Apply the new section offsets.  */
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] Fix `add-symbol-file -o ... -s ...` address mapping bug
  2026-09-25  7:15     ` Dragorn421
@ 2026-09-25 17:06       ` Keith Seitz
  0 siblings, 0 replies; 7+ messages in thread
From: Keith Seitz @ 2026-09-25 17:06 UTC (permalink / raw)
  To: Dragorn421, gdb-patches

On 9/25/26 12:15 AM, Dragorn421 wrote:
> On 9/24/26 20:54, Keith Seitz wrote:
>> Hi,
>>
>> Thank you for the revision. This looks good, I just note one
>> tiny nit.
>>
>> On 9/23/26 8:51 PM, Dragorn421 wrote:
>>> diff --git a/gdb/symfile.c b/gdb/symfile.c
>>> index 017f7a49d8d..5691c2a46d9 100644
>>> --- a/gdb/symfile.c
>>> +++ b/gdb/symfile.c
>>> @@ -2163,7 +2163,7 @@ set_objfile_default_section_offset (struct 
>>> objfile *objf,
>>>       }
>>>           if (cmp == 0)
>>> -    offsets[objf_sect->sectindex] = 0;
>>> +    offsets[objf_sect->sectindex] = objf->section_offsets[objf_sect- 
>>> >sectindex];
>>
>> This line is now too long. Suggest moving "= ..." to the next line as
>> per our usual coding convention. No need to repost this patch. 
> 
> 
> Thanks for the review.
> 
> What do you mean by "No need to repost this patch."? Should I just send 
> the other (test-adding) patch on its own from now on?

That's what I would do, yes, but you do not have to if it is easier for
you to track this. You could simply continue to post both patches.

> So will a maintainer possibly take care of formatting that too-long line 
> after applying the patch?
You would make the change locally, pushing that when a maintainer
approved the whole series.

My apologies for not keeping this simpler for first time contributors.

Keith


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-09-25 17:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-24  3:51 [PATCH v3 0/2] Fix `add-symbol-file -o ... -s ...` address mapping bug Dragorn421
2026-09-24  3:51 ` [PATCH v3 1/2] " Dragorn421
2026-09-24 18:54   ` Keith Seitz
2026-09-25  7:15     ` Dragorn421
2026-09-25 17:06       ` Keith Seitz
2026-09-24  3:51 ` [PATCH v3 2/2] `add-symbol-file`: add test for mixing -o and -s Dragorn421
2026-09-24 18:57   ` Keith Seitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox