Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* Fix `add-symbol-file -o ... -s ...` address mapping bug
@ 2026-09-05 10:16 Dragorn421
  2026-09-21  2:02 ` [PING] " Dragorn421
  0 siblings, 1 reply; 3+ messages in thread
From: Dragorn421 @ 2026-09-05 10:16 UTC (permalink / raw)
  To: gdb-patches

Hello,

I encountered an issue in GDB and am hereby submitting a patch for 
fixing it.

First, let me explain the problem: it has to do with the 
`add-symbol-file` command that is used to map elf files to arbitrary 
addresses.

For example if I want to map 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 proposed fix is to change `set_objfile_default_section_offset` to 
pass the current offset instead of 0 for sections that should be unaltered:

```diff
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.  */
```

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

Sincerely,
Dragorn421


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

* [PING] Fix `add-symbol-file -o ... -s ...` address mapping bug
  2026-09-05 10:16 Fix `add-symbol-file -o ... -s ...` address mapping bug Dragorn421
@ 2026-09-21  2:02 ` Dragorn421
  2026-09-21 19:45   ` Keith Seitz
  0 siblings, 1 reply; 3+ messages in thread
From: Dragorn421 @ 2026-09-21  2:02 UTC (permalink / raw)
  To: gdb-patches

Ping :)

---------- Forwarded message ---------
De : Dragorn421 <dragorn421@gmail.com>
Date: sam. 5 sept. 2026 à 12:16
Subject: Fix `add-symbol-file -o ... -s ...` address mapping bug
To: <gdb-patches@sourceware.org>


Hello,

I encountered an issue in GDB and am hereby submitting a patch for
fixing it.

First, let me explain the problem: it has to do with the
`add-symbol-file` command that is used to map elf files to arbitrary
addresses.

For example if I want to map 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 proposed fix is to change `set_objfile_default_section_offset` to
pass the current offset instead of 0 for sections that should be unaltered:

```diff
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.  */
```

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

Sincerely,
Dragorn421

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

* Re: [PING] Fix `add-symbol-file -o ... -s ...` address mapping bug
  2026-09-21  2:02 ` [PING] " Dragorn421
@ 2026-09-21 19:45   ` Keith Seitz
  0 siblings, 0 replies; 3+ messages in thread
From: Keith Seitz @ 2026-09-21 19:45 UTC (permalink / raw)
  To: Dragorn421, gdb-patches

Hi,

Thank you for submitting a patch and fixing a bug! It is
very appreciated.

Overall, your patch looks good! I just have a few (very) minor
comments to clean it up a little.

On 9/20/26 7:02 PM, Dragorn421 wrote:
 > ---------- Forwarded message ---------> De : Dragorn421 
<dragorn421@gmail.com>
> Date: sam. 5 sept. 2026 à 12:16
> Subject: Fix `add-symbol-file -o ... -s ...` address mapping bug
> To: <gdb-patches@sourceware.org>
> 
> 
> Hello,
> 
> I encountered an issue in GDB and am hereby submitting a patch for
> fixing it.

In gdb land, we typically submit patches ready to commit. That is,
we use "git send-email" to send patches to the list which, once
approved, can be applied directly to the tree.

You've started well here: you're subject line is perfect.
[I don't know if you have any interest in submitting future patches
to gdb -- I hope you do! If you don't, just ignore me. :-)]

Now let's fix-up your body text a little:

> 
> First, let me explain the problem: it has to do with the
> `add-symbol-file` command that is used to map elf files to arbitrary
> addresses.

I simply recommend changing this slightly to read:

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.

Those two options are currently may not be supplied together in the
same command invocation.

[then keep all the below]

> For example if I want to map 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 proposed fix is to change `set_objfile_default_section_offset` to
> pass the current offset instead of 0 for sections that should be unaltered:

And there you go! Perfect commit log!

The only real additional ask, per the contributions checklist[1], would
be a test case for this to make sure this never regresses. Are you able
to do that? You could modify or "copy" gdb.base/relocate.exp to exercise
this specific  use case.

[1] https://www.sourceware.org/gdb/wiki/ContributionChecklist

> ```diff
> 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.  */
> ```

> This is my first time contributing so my apologies if I'm doing anything
> not properly, please let me know.
Your fix looks good to me. If you'd like to "address" my review comments
and submit a v2, please do! Otherwise, we'll have to await final say by
an approving maintainer.

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

Thank you!

Keith


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

end of thread, other threads:[~2026-09-21 19:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-05 10:16 Fix `add-symbol-file -o ... -s ...` address mapping bug Dragorn421
2026-09-21  2:02 ` [PING] " Dragorn421
2026-09-21 19:45   ` Keith Seitz

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