Use "output-radix" setting to format function offsets
Commit Message
This is a patch for a discussion [1] I had previously where it was
determined that it was a bug for GDB to not use the "output-radix"
setting for function offsets.
This patch includes updates to the radix.exp tests, and I verified that
there were no new breakages added when running the full gdb testsuite
with this patch.
I didn't make any changes to NEWS or a /gdb/ Changelog entry for this,
if I should please let me know.
I also have not personally completed a FSF copyright assignment form.
Sample output with this patch:
```
(gdb) disas main
Dump of assembler code for function main:
0x0000000000001149 <+0>: endbr64
0x000000000000114d <+4>: push %rbp
0x000000000000114e <+5>: mov %rsp,%rbp
0x0000000000001151 <+8>: lea 0xeac(%rip),%rax # 0x2004
0x0000000000001158 <+15>: mov %rax,%rdi
0x000000000000115b <+18>: mov $0x0,%eax
0x0000000000001160 <+23>: call 0x1050 <printf@plt>
0x0000000000001165 <+28>: mov $0x0,%eax
0x000000000000116a <+33>: pop %rbp
0x000000000000116b <+34>: ret
End of assembler dump.
(gdb) set radix 0x10
Input and output radices now set to decimal 16, hex 10, octal 20.
(gdb) disas main
Dump of assembler code for function main:
0x0000000000001149 <+0x0>: endbr64
0x000000000000114d <+0x4>: push %rbp
0x000000000000114e <+0x5>: mov %rsp,%rbp
0x0000000000001151 <+0x8>: lea 0xeac(%rip),%rax # 0x2004
0x0000000000001158 <+0xf>: mov %rax,%rdi
0x000000000000115b <+0x12>: mov $0x0,%eax
0x0000000000001160 <+0x17>: call 0x1050 <printf@plt>
0x0000000000001165 <+0x1c>: mov $0x0,%eax
0x000000000000116a <+0x21>: pop %rbp
0x000000000000116b <+0x22>: ret
End of assembler dump.
```
[1] https://sourceware.org/pipermail/gdb/2026-April/052170.html
---
gdb/disasm.c | 18 +++++++++++++----
gdb/printcmd.c | 2 +-
gdb/testsuite/gdb.base/radix.c | 31 +++++++++++++++++++++++++++++
gdb/testsuite/gdb.base/radix.exp | 34 ++++++++++++++++++++++++++++++++
gdb/valprint.c | 10 ++++++++++
gdb/valprint.h | 6 ++++++
6 files changed, 96 insertions(+), 5 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/radix.c
Comments
On 5/4/26 6:37 PM, Zander Work wrote:
> This is a patch for a discussion [1] I had previously where it was
> determined that it was a bug for GDB to not use the "output-radix"
> setting for function offsets.
>
> This patch includes updates to the radix.exp tests, and I verified that
> there were no new breakages added when running the full gdb testsuite
> with this patch.
>
> I didn't make any changes to NEWS or a /gdb/ Changelog entry for this,
> if I should please let me know.
Hi! Thank you for working on this!
Changelog entries are definitely no longer required, and I don't think
this would need a NEWS entry either.
I have some minor comments, mostly about styling or wondering about a
few choices, but in general I think this patch is in the right direction!
>
> I also have not personally completed a FSF copyright assignment form.
Yeah, for a patch this size, it will definitely be necessary. I'm adding
Tom Tromey in CC since I know he knows about the assignment process
(more than me, at least)
>
> Sample output with this patch:
>
> ```
> (gdb) disas main
> Dump of assembler code for function main:
> 0x0000000000001149 <+0>: endbr64
> 0x000000000000114d <+4>: push %rbp
> 0x000000000000114e <+5>: mov %rsp,%rbp
> 0x0000000000001151 <+8>: lea 0xeac(%rip),%rax # 0x2004
> 0x0000000000001158 <+15>: mov %rax,%rdi
> 0x000000000000115b <+18>: mov $0x0,%eax
> 0x0000000000001160 <+23>: call 0x1050 <printf@plt>
> 0x0000000000001165 <+28>: mov $0x0,%eax
> 0x000000000000116a <+33>: pop %rbp
> 0x000000000000116b <+34>: ret
> End of assembler dump.
> (gdb) set radix 0x10
> Input and output radices now set to decimal 16, hex 10, octal 20.
> (gdb) disas main
> Dump of assembler code for function main:
> 0x0000000000001149 <+0x0>: endbr64
> 0x000000000000114d <+0x4>: push %rbp
> 0x000000000000114e <+0x5>: mov %rsp,%rbp
> 0x0000000000001151 <+0x8>: lea 0xeac(%rip),%rax # 0x2004
> 0x0000000000001158 <+0xf>: mov %rax,%rdi
> 0x000000000000115b <+0x12>: mov $0x0,%eax
> 0x0000000000001160 <+0x17>: call 0x1050 <printf@plt>
> 0x0000000000001165 <+0x1c>: mov $0x0,%eax
> 0x000000000000116a <+0x21>: pop %rbp
> 0x000000000000116b <+0x22>: ret
> End of assembler dump.
> ```
>
> [1] https://sourceware.org/pipermail/gdb/2026-April/052170.html
> ---
> gdb/disasm.c | 18 +++++++++++++----
> gdb/printcmd.c | 2 +-
> gdb/testsuite/gdb.base/radix.c | 31 +++++++++++++++++++++++++++++
> gdb/testsuite/gdb.base/radix.exp | 34 ++++++++++++++++++++++++++++++++
> gdb/valprint.c | 10 ++++++++++
> gdb/valprint.h | 6 ++++++
> 6 files changed, 96 insertions(+), 5 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/radix.c
>
> diff --git a/gdb/disasm.c b/gdb/disasm.c
> index 81c466c188a..5d19dea72b6 100644
> --- a/gdb/disasm.c
> +++ b/gdb/disasm.c
> @@ -375,10 +375,20 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
> m_uiout->field_string ("func-name", name,
> function_name_style.style ());
> /* For negative offsets, avoid displaying them as +-N; the sign of
> - the offset takes the place of the "+" here. */
> - if (offset >= 0)
> - m_uiout->text ("+");
> - m_uiout->field_signed ("offset", offset);
> + the offset takes the place of the "+" here. For MI consumers,
> + emit the integer value; otherwise, print the formatted offset based
> + on the current 'output-radix'. */
> + if (m_uiout->is_mi_like_p ())
> + {
> + if (offset >= 0)
> + m_uiout->text ("+");
> + m_uiout->field_signed ("offset", offset);
I'm not familiar with the radix functionality, so this is a genuine
quesiton, but shouldn't the MI interface also honor the radix request?
It seems to me like it would make sense, but if other MI messages don't,
your approach here seems right.
> + }
> + else
> + {
> + std::string s = format_pc_offset (offset);
> + m_uiout->field_string ("offset", s.c_str ());
> + }
> m_uiout->text (">:\t");
> }
> else
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index ae498395436..5fb8c666447 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -567,7 +567,7 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr,
> gdb_puts ("<", stream);
> fputs_styled (name.c_str (), function_name_style.style (), stream);
> if (offset != 0)
> - gdb_printf (stream, "%+d", offset);
> + gdb_puts (format_pc_offset (offset).c_str (), stream);
>
> /* Append source filename and line number if desired. Give specific
> line # of this addr, if we have it; else line # of the nearest symbol. */
> diff --git a/gdb/testsuite/gdb.base/radix.c b/gdb/testsuite/gdb.base/radix.c
> new file mode 100644
> index 00000000000..191c374d466
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/radix.c
> @@ -0,0 +1,31 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2013-2026 Free Software Foundation, Inc.
The copyright year can be only 2026.
> +
> + 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/>. */
> +
> +#include <unistd.h>
> +#include <stdio.h>
> +
> +int v;
> +
> +int main()
Testsuite should also follow the coding style, so main should be in
column 0 of the next line and have a space between function name and
parenthesis.
> +{
> + puts("hello world");
> + /* Don't let the test case run forever. */
> + alarm (60);
This feels overly complex for what you want to test. Is there any reason
why a simple
int
main ()
{
return 0;
}
wouldn't be enough for the test?
> +
> + for (;;)
> + ;
> +}
> diff --git a/gdb/testsuite/gdb.base/radix.exp b/gdb/testsuite/gdb.base/radix.exp
> index 7a4320bbf36..99256b98df3 100644
> --- a/gdb/testsuite/gdb.base/radix.exp
> +++ b/gdb/testsuite/gdb.base/radix.exp
> @@ -17,6 +17,11 @@
> # This file was written by Fred Fish. (fnf@cygnus.com)
> # And rewritten by Michael Chastain (mec.gnu@mindspring.com)
>
> +standard_testfile
> +
> +if {[build_executable "failed to prepare" $testfile $srcfile debug]} {
> + return -1
> +}
>
> # Start with a fresh gdb.
>
> @@ -189,3 +194,32 @@ gdb_test "set radix 7" \
> gdb_test "show output-radix" \
> "Default output radix for printing of values is 10\\." \
> "output radix unchanged after rejection through set radix command"
> +
> +with_test_prefix "pc offset radix" {
> + clean_restart $testfile
> +
> + if { ![runto_main] } {
> + return -1
> + }
> +
> + proc test_pc_offset_radix { oradix offset_re } {
> + global gdb_prompt
> +
> + gdb_test "set output-radix $oradix" \
> + "Output radix now set to decimal $oradix.*\\."
> +
> + set test "x/10i main with output-radix $oradix"
> + gdb_test_multiple "x/10i main" $test {
I wonder if using x/i or x/2i would be enough. I think it's good to
minimize the amount of stuff emitted by GDB, so that we don't fill a
buffer on slow/overworked machines and get unreliable tests
> + -re "<main\\+$offset_re>:\[^\r\n\]*\r\n(?:\[^\r\n\]*\r\n)*$gdb_prompt $" {
This can be simplified in a few ways. First, you can use -wrap to wrap
your regular expression in the stuff that gdb_test adds around it, so
you won't need to add $gdb_prompt at the end and some stuff at the
start, and second, the whole "\[^\r\n\]*\r\n(?:\[^\r\n\]*\r\n)*" is just
"any number of lines with any amount of characters", so there's no
reason to not use a simple ".*" there. We just avoid .* when the exact
amount of lines, or that something is in the same line, is important,
which isn't the case in this test.
> + pass $gdb_test_name
> + }
> + }
> + }
> +
> + test_pc_offset_radix 8 {0[0-7]{2,}}
> + test_pc_offset_radix 10 {[1-9][0-9]+}
This regex fails if the number is exactly 0, but if you use multiple
instructions and -wrap, I don't think it is a big deal....
> + test_pc_offset_radix 16 {0x[0-9a-f]{2,}}
> +
> + gdb_test "set output-radix 10" "Output radix now set to decimal 10.*\\." \
> + "restore output-radix"
> +}
> diff --git a/gdb/valprint.c b/gdb/valprint.c
> index 62b1b33bb66..ea4bece0416 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -171,6 +171,16 @@ show_output_radix (struct ui_file *file, int from_tty,
> value);
> }
>
There should be a comment here like
/* See valprint.h. */
> +std::string
> +format_pc_offset (int offset)
> +{
> + const char *sign = (offset < 0) ? "-" : "+";
> + ULONGEST uoffset = (offset < 0) ? -(ULONGEST) offset : (ULONGEST) offset;
> +
> + std::string body = int_string (uoffset, output_radix, 0, 0, 1);
since uoffset is ULONGEST, you should use pulongest. However, I don't
even think you need this extra variable, you can just pass (offset < 0)
-offset : offset to the int_string call.
> + return std::string (sign) + body;
why not declare sign as an std::string? I think it would make things a
little more readable.
> +}
> +
> /* By default we print arrays without printing the index of each element in
> the array. This behavior can be changed by setting PRINT_ARRAY_INDEXES. */
>
> diff --git a/gdb/valprint.h b/gdb/valprint.h
> index 0ce3e0781f6..5511707cba3 100644
> --- a/gdb/valprint.h
> +++ b/gdb/valprint.h
> @@ -320,6 +320,12 @@ extern int build_address_symbolic (struct gdbarch *,
> int *line,
> int *unmapped);
>
> +/* Format OFFSET, the offset portion of a "<symbol+offset>" display, as
> + a string with an explicit sign prefix ("+" or "-"). The numeric
> + portion is rendered using the current "output-radix". */
> +
> +extern std::string format_pc_offset (int offset);
> +
> /* Check to see if RECURSE is greater than or equal to the allowed
> printing max-depth (see 'set print max-depth'). If it is then print an
> ellipsis expression to STREAM and return true, otherwise return false.
Thank you for the review! Replies inline, I will follow up (probably
next week) with a v2 of this patch.
-Zander
On Fri, May 8, 2026 at 6:45 AM Guinevere Larsen <guinevere@redhat.com> wrote:
>
> On 5/4/26 6:37 PM, Zander Work wrote:
> > This is a patch for a discussion [1] I had previously where it was
> > determined that it was a bug for GDB to not use the "output-radix"
> > setting for function offsets.
> >
> > This patch includes updates to the radix.exp tests, and I verified that
> > there were no new breakages added when running the full gdb testsuite
> > with this patch.
> >
> > I didn't make any changes to NEWS or a /gdb/ Changelog entry for this,
> > if I should please let me know.
>
> Hi! Thank you for working on this!
>
> Changelog entries are definitely no longer required, and I don't think
> this would need a NEWS entry either.
>
> I have some minor comments, mostly about styling or wondering about a
> few choices, but in general I think this patch is in the right direction!
>
> >
> > I also have not personally completed a FSF copyright assignment form.
> Yeah, for a patch this size, it will definitely be necessary. I'm adding
> Tom Tromey in CC since I know he knows about the assignment process
> (more than me, at least)
> >
> > Sample output with this patch:
> >
> > ```
> > (gdb) disas main
> > Dump of assembler code for function main:
> > 0x0000000000001149 <+0>: endbr64
> > 0x000000000000114d <+4>: push %rbp
> > 0x000000000000114e <+5>: mov %rsp,%rbp
> > 0x0000000000001151 <+8>: lea 0xeac(%rip),%rax # 0x2004
> > 0x0000000000001158 <+15>: mov %rax,%rdi
> > 0x000000000000115b <+18>: mov $0x0,%eax
> > 0x0000000000001160 <+23>: call 0x1050 <printf@plt>
> > 0x0000000000001165 <+28>: mov $0x0,%eax
> > 0x000000000000116a <+33>: pop %rbp
> > 0x000000000000116b <+34>: ret
> > End of assembler dump.
> > (gdb) set radix 0x10
> > Input and output radices now set to decimal 16, hex 10, octal 20.
> > (gdb) disas main
> > Dump of assembler code for function main:
> > 0x0000000000001149 <+0x0>: endbr64
> > 0x000000000000114d <+0x4>: push %rbp
> > 0x000000000000114e <+0x5>: mov %rsp,%rbp
> > 0x0000000000001151 <+0x8>: lea 0xeac(%rip),%rax # 0x2004
> > 0x0000000000001158 <+0xf>: mov %rax,%rdi
> > 0x000000000000115b <+0x12>: mov $0x0,%eax
> > 0x0000000000001160 <+0x17>: call 0x1050 <printf@plt>
> > 0x0000000000001165 <+0x1c>: mov $0x0,%eax
> > 0x000000000000116a <+0x21>: pop %rbp
> > 0x000000000000116b <+0x22>: ret
> > End of assembler dump.
> > ```
> >
> > [1] https://sourceware.org/pipermail/gdb/2026-April/052170.html
> > ---
> > gdb/disasm.c | 18 +++++++++++++----
> > gdb/printcmd.c | 2 +-
> > gdb/testsuite/gdb.base/radix.c | 31 +++++++++++++++++++++++++++++
> > gdb/testsuite/gdb.base/radix.exp | 34 ++++++++++++++++++++++++++++++++
> > gdb/valprint.c | 10 ++++++++++
> > gdb/valprint.h | 6 ++++++
> > 6 files changed, 96 insertions(+), 5 deletions(-)
> > create mode 100644 gdb/testsuite/gdb.base/radix.c
> >
> > diff --git a/gdb/disasm.c b/gdb/disasm.c
> > index 81c466c188a..5d19dea72b6 100644
> > --- a/gdb/disasm.c
> > +++ b/gdb/disasm.c
> > @@ -375,10 +375,20 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
> > m_uiout->field_string ("func-name", name,
> > function_name_style.style ());
> > /* For negative offsets, avoid displaying them as +-N; the sign of
> > - the offset takes the place of the "+" here. */
> > - if (offset >= 0)
> > - m_uiout->text ("+");
> > - m_uiout->field_signed ("offset", offset);
> > + the offset takes the place of the "+" here. For MI consumers,
> > + emit the integer value; otherwise, print the formatted offset based
> > + on the current 'output-radix'. */
> > + if (m_uiout->is_mi_like_p ())
> > + {
> > + if (offset >= 0)
> > + m_uiout->text ("+");
> > + m_uiout->field_signed ("offset", offset);
>
> I'm not familiar with the radix functionality, so this is a genuine
> quesiton, but shouldn't the MI interface also honor the radix request?
>
> It seems to me like it would make sense, but if other MI messages don't,
> your approach here seems right.
My assumption was that a change from an int-type to a string-type may
cause compatibility/API contract
issues, I'm not familiar enough with how MI is consumed to know for
sure. Happy to simplify this
to be the same output unconditionally if it makes sense.
>
> > + }
> > + else
> > + {
> > + std::string s = format_pc_offset (offset);
> > + m_uiout->field_string ("offset", s.c_str ());
> > + }
> > m_uiout->text (">:\t");
> > }
> > else
> > diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> > index ae498395436..5fb8c666447 100644
> > --- a/gdb/printcmd.c
> > +++ b/gdb/printcmd.c
> > @@ -567,7 +567,7 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr,
> > gdb_puts ("<", stream);
> > fputs_styled (name.c_str (), function_name_style.style (), stream);
> > if (offset != 0)
> > - gdb_printf (stream, "%+d", offset);
> > + gdb_puts (format_pc_offset (offset).c_str (), stream);
> >
> > /* Append source filename and line number if desired. Give specific
> > line # of this addr, if we have it; else line # of the nearest symbol. */
> > diff --git a/gdb/testsuite/gdb.base/radix.c b/gdb/testsuite/gdb.base/radix.c
> > new file mode 100644
> > index 00000000000..191c374d466
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/radix.c
> > @@ -0,0 +1,31 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > + Copyright 2013-2026 Free Software Foundation, Inc.
> The copyright year can be only 2026.
ack
> > +
> > + 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/>. */
> > +
> > +#include <unistd.h>
> > +#include <stdio.h>
> > +
> > +int v;
> > +
> > +int main()
> Testsuite should also follow the coding style, so main should be in
> column 0 of the next line and have a space between function name and
> parenthesis.
ack
> > +{
> > + puts("hello world");
> > + /* Don't let the test case run forever. */
> > + alarm (60);
>
> This feels overly complex for what you want to test. Is there any reason
> why a simple
>
> int
> main ()
> {
> return 0;
> }
>
> wouldn't be enough for the test?
I just duplicated and modified another test program right next to this
one, I will trim it down and
update the styling and copyright as requested. Just need to have
enough in the function for
there to be enough instructions for testing the offsets.
>
> > +
> > + for (;;)
> > + ;
> > +}
> > diff --git a/gdb/testsuite/gdb.base/radix.exp b/gdb/testsuite/gdb.base/radix.exp
> > index 7a4320bbf36..99256b98df3 100644
> > --- a/gdb/testsuite/gdb.base/radix.exp
> > +++ b/gdb/testsuite/gdb.base/radix.exp
> > @@ -17,6 +17,11 @@
> > # This file was written by Fred Fish. (fnf@cygnus.com)
> > # And rewritten by Michael Chastain (mec.gnu@mindspring.com)
> >
> > +standard_testfile
> > +
> > +if {[build_executable "failed to prepare" $testfile $srcfile debug]} {
> > + return -1
> > +}
> >
> > # Start with a fresh gdb.
> >
> > @@ -189,3 +194,32 @@ gdb_test "set radix 7" \
> > gdb_test "show output-radix" \
> > "Default output radix for printing of values is 10\\." \
> > "output radix unchanged after rejection through set radix command"
> > +
> > +with_test_prefix "pc offset radix" {
> > + clean_restart $testfile
> > +
> > + if { ![runto_main] } {
> > + return -1
> > + }
> > +
> > + proc test_pc_offset_radix { oradix offset_re } {
> > + global gdb_prompt
> > +
> > + gdb_test "set output-radix $oradix" \
> > + "Output radix now set to decimal $oradix.*\\."
> > +
> > + set test "x/10i main with output-radix $oradix"
> > + gdb_test_multiple "x/10i main" $test {
> I wonder if using x/i or x/2i would be enough. I think it's good to
> minimize the amount of stuff emitted by GDB, so that we don't fill a
> buffer on slow/overworked machines and get unreliable tests
I wanted to get enough output for the test to fully verify that the
right octal was being
used (rather than an (unlikely) formatting issue; ie, offsets >
0n10/0xa). I'll make a tweak here
to decrease the buffer size while still validating this behavior.
> > + -re "<main\\+$offset_re>:\[^\r\n\]*\r\n(?:\[^\r\n\]*\r\n)*$gdb_prompt $" {
>
> This can be simplified in a few ways. First, you can use -wrap to wrap
> your regular expression in the stuff that gdb_test adds around it, so
> you won't need to add $gdb_prompt at the end and some stuff at the
> start, and second, the whole "\[^\r\n\]*\r\n(?:\[^\r\n\]*\r\n)*" is just
> "any number of lines with any amount of characters", so there's no
> reason to not use a simple ".*" there. We just avoid .* when the exact
> amount of lines, or that something is in the same line, is important,
> which isn't the case in this test.
Ah perfect, thanks. Will adjust.
>
> > + pass $gdb_test_name
> > + }
> > + }
> > + }
> > +
> > + test_pc_offset_radix 8 {0[0-7]{2,}}
> > + test_pc_offset_radix 10 {[1-9][0-9]+}
> This regex fails if the number is exactly 0, but if you use multiple
> instructions and -wrap, I don't think it is a big deal....
Yep, I'll make some tweaks here.
> > + test_pc_offset_radix 16 {0x[0-9a-f]{2,}}
> > +
> > + gdb_test "set output-radix 10" "Output radix now set to decimal 10.*\\." \
> > + "restore output-radix"
> > +}
> > diff --git a/gdb/valprint.c b/gdb/valprint.c
> > index 62b1b33bb66..ea4bece0416 100644
> > --- a/gdb/valprint.c
> > +++ b/gdb/valprint.c
> > @@ -171,6 +171,16 @@ show_output_radix (struct ui_file *file, int from_tty,
> > value);
> > }
> >
>
> There should be a comment here like
>
> /* See valprint.h. */
ack
>
> > +std::string
> > +format_pc_offset (int offset)
> > +{
> > + const char *sign = (offset < 0) ? "-" : "+";
> > + ULONGEST uoffset = (offset < 0) ? -(ULONGEST) offset : (ULONGEST) offset;
> > +
> > + std::string body = int_string (uoffset, output_radix, 0, 0, 1);
>
> since uoffset is ULONGEST, you should use pulongest. However, I don't
> even think you need this extra variable, you can just pass (offset < 0)
> -offset : offset to the int_string call.
>
> > + return std::string (sign) + body;
> why not declare sign as an std::string? I think it would make things a
> little more readable.
> > +}
> > +
> > /* By default we print arrays without printing the index of each element in
> > the array. This behavior can be changed by setting PRINT_ARRAY_INDEXES. */
> >
> > diff --git a/gdb/valprint.h b/gdb/valprint.h
> > index 0ce3e0781f6..5511707cba3 100644
> > --- a/gdb/valprint.h
> > +++ b/gdb/valprint.h
> > @@ -320,6 +320,12 @@ extern int build_address_symbolic (struct gdbarch *,
> > int *line,
> > int *unmapped);
> >
> > +/* Format OFFSET, the offset portion of a "<symbol+offset>" display, as
> > + a string with an explicit sign prefix ("+" or "-"). The numeric
> > + portion is rendered using the current "output-radix". */
> > +
> > +extern std::string format_pc_offset (int offset);
> > +
> > /* Check to see if RECURSE is greater than or equal to the allowed
> > printing max-depth (see 'set print max-depth'). If it is then print an
> > ellipsis expression to STREAM and return true, otherwise return false.
>
>
> --
> Cheers,
> Guinevere Larsen
> It/she
>
>>>>> Guinevere Larsen <guinevere@redhat.com> writes:
>> I also have not personally completed a FSF copyright assignment form.
> Yeah, for a patch this size, it will definitely be necessary. I'm
> adding Tom Tromey in CC since I know he knows about the assignment
> process (more than me, at least)
If your company doesn't automatically cover you (I don't know), email
assign@gnu.org and say that you have patches for gdb that you want to
assign. They will get you started on the process.
Once everything is done, be sure to tell us, because we aren't
automatically notified.
thanks,
Tom
@@ -375,10 +375,20 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
m_uiout->field_string ("func-name", name,
function_name_style.style ());
/* For negative offsets, avoid displaying them as +-N; the sign of
- the offset takes the place of the "+" here. */
- if (offset >= 0)
- m_uiout->text ("+");
- m_uiout->field_signed ("offset", offset);
+ the offset takes the place of the "+" here. For MI consumers,
+ emit the integer value; otherwise, print the formatted offset based
+ on the current 'output-radix'. */
+ if (m_uiout->is_mi_like_p ())
+ {
+ if (offset >= 0)
+ m_uiout->text ("+");
+ m_uiout->field_signed ("offset", offset);
+ }
+ else
+ {
+ std::string s = format_pc_offset (offset);
+ m_uiout->field_string ("offset", s.c_str ());
+ }
m_uiout->text (">:\t");
}
else
@@ -567,7 +567,7 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr,
gdb_puts ("<", stream);
fputs_styled (name.c_str (), function_name_style.style (), stream);
if (offset != 0)
- gdb_printf (stream, "%+d", offset);
+ gdb_puts (format_pc_offset (offset).c_str (), stream);
/* Append source filename and line number if desired. Give specific
line # of this addr, if we have it; else line # of the nearest symbol. */
new file mode 100644
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013-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/>. */
+
+#include <unistd.h>
+#include <stdio.h>
+
+int v;
+
+int main()
+{
+ puts("hello world");
+ /* Don't let the test case run forever. */
+ alarm (60);
+
+ for (;;)
+ ;
+}
@@ -17,6 +17,11 @@
# This file was written by Fred Fish. (fnf@cygnus.com)
# And rewritten by Michael Chastain (mec.gnu@mindspring.com)
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug]} {
+ return -1
+}
# Start with a fresh gdb.
@@ -189,3 +194,32 @@ gdb_test "set radix 7" \
gdb_test "show output-radix" \
"Default output radix for printing of values is 10\\." \
"output radix unchanged after rejection through set radix command"
+
+with_test_prefix "pc offset radix" {
+ clean_restart $testfile
+
+ if { ![runto_main] } {
+ return -1
+ }
+
+ proc test_pc_offset_radix { oradix offset_re } {
+ global gdb_prompt
+
+ gdb_test "set output-radix $oradix" \
+ "Output radix now set to decimal $oradix.*\\."
+
+ set test "x/10i main with output-radix $oradix"
+ gdb_test_multiple "x/10i main" $test {
+ -re "<main\\+$offset_re>:\[^\r\n\]*\r\n(?:\[^\r\n\]*\r\n)*$gdb_prompt $" {
+ pass $gdb_test_name
+ }
+ }
+ }
+
+ test_pc_offset_radix 8 {0[0-7]{2,}}
+ test_pc_offset_radix 10 {[1-9][0-9]+}
+ test_pc_offset_radix 16 {0x[0-9a-f]{2,}}
+
+ gdb_test "set output-radix 10" "Output radix now set to decimal 10.*\\." \
+ "restore output-radix"
+}
@@ -171,6 +171,16 @@ show_output_radix (struct ui_file *file, int from_tty,
value);
}
+std::string
+format_pc_offset (int offset)
+{
+ const char *sign = (offset < 0) ? "-" : "+";
+ ULONGEST uoffset = (offset < 0) ? -(ULONGEST) offset : (ULONGEST) offset;
+
+ std::string body = int_string (uoffset, output_radix, 0, 0, 1);
+ return std::string (sign) + body;
+}
+
/* By default we print arrays without printing the index of each element in
the array. This behavior can be changed by setting PRINT_ARRAY_INDEXES. */
@@ -320,6 +320,12 @@ extern int build_address_symbolic (struct gdbarch *,
int *line,
int *unmapped);
+/* Format OFFSET, the offset portion of a "<symbol+offset>" display, as
+ a string with an explicit sign prefix ("+" or "-"). The numeric
+ portion is rendered using the current "output-radix". */
+
+extern std::string format_pc_offset (int offset);
+
/* Check to see if RECURSE is greater than or equal to the allowed
printing max-depth (see 'set print max-depth'). If it is then print an
ellipsis expression to STREAM and return true, otherwise return false.