[PATCH v2] Use "output-radix" setting to format function offsets

[PATCH v2] Use "output-radix" setting to format function offsets

[Zander Work]

Updates since v1:

* Addressed feedback in the test suite by more robustly handling the
  GDB I/O and improved the regex patterns used to match on the offset
  values.
* Updated the accompanying test program to have a minimum level of
  complexity to avoid compiler optimizations trimming out too much
  function body, and fixed the formatting and copyright date
* Added a comment on the `format_pc_offset()` implementation and
  addressed comments in that function.

I believe the only open items are:

* Should MI consumers have the string radix-formatted offset value, or
  continue having an int value (this is the current impl in the patch)?
* I still need to do an FSF copyright assignment.

Please let me know if I missed anything else to address. Thanks!
---
 gdb/disasm.c                     | 18 ++++++++++---
 gdb/printcmd.c                   |  2 +-
 gdb/testsuite/gdb.base/radix.c   | 35 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/radix.exp | 43 ++++++++++++++++++++++++++++++++
 gdb/valprint.c                   | 13 ++++++++++
 gdb/valprint.h                   |  6 +++++
 6 files changed, 112 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);
+	  }
+	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..8a4d1287208
--- /dev/null
+++ b/gdb/testsuite/gdb.base/radix.c
@@ -0,0 +1,35 @@
+/* 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/>.  */
+
+#include <unistd.h>
+#include <stdio.h>
+
+static int v;
+
+int
+main (void)
+{
+  v = 0;
+
+  puts ("hello world");
+
+  printf ("this is another string\n");
+
+  v += 3;
+
+  return v;
+}
diff --git a/gdb/testsuite/gdb.base/radix.exp b/gdb/testsuite/gdb.base/radix.exp
index 7a4320bbf36..4b8e2d74b49 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,41 @@ 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/i with output-radix $oradix"
+
+      gdb_test_multiple "x/i \$pc" "$test 1" {
+	-re -wrap "<main\\+$offset_re>:.*" {
+	   pass $gdb_test_name
+	}
+      }
+
+      gdb_test "ni 3" "\[0-9\]+.*" "Next instruction for radix $oradix"
+
+      gdb_test_multiple "x/i \$pc" "$test 2" {
+	-re -wrap "<main\\+$offset_re>:.*" {
+	   pass $gdb_test_name
+	}
+      }
+    }
+
+    test_pc_offset_radix 8  {0[0-7]+}
+    test_pc_offset_radix 10 {[1-9][0-9]*}
+    test_pc_offset_radix 16 {0x[0-9a-f]+}
+
+    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..d0d0472ca7a 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -171,6 +171,19 @@ show_output_radix (struct ui_file *file, int from_tty,
 	      value);
 }
 
+/* See valprint.h.  */
+
+std::string
+format_pc_offset (int offset)
+{
+  std::string sign = (offset < 0) ? "-" : "+";
+
+  std::string body = int_string (offset < 0 ? -offset : offset, output_radix,
+                                 0, 0, 1);
+
+  return 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.  */
 
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.
-- 
2.54.0.563.g4f69b47b94-goog

Re: [PATCH v2] Use "output-radix" setting to format function offsets

[Guinevere Larsen]

On 5/9/26 1:45 AM, Zander Work wrote:
> Updates since v1:
>
> * Addressed feedback in the test suite by more robustly handling the
>    GDB I/O and improved the regex patterns used to match on the offset
>    values.
> * Updated the accompanying test program to have a minimum level of
>    complexity to avoid compiler optimizations trimming out too much
>    function body, and fixed the formatting and copyright date
> * Added a comment on the `format_pc_offset()` implementation and
>    addressed comments in that function.
>
> I believe the only open items are:
>
> * Should MI consumers have the string radix-formatted offset value, or
>    continue having an int value (this is the current impl in the patch)?
> * I still need to do an FSF copyright assignment.
>
> Please let me know if I missed anything else to address. Thanks!

Hi Zander!

Thanks for the quick v2 for this patch. When sending another version of 
a patch, we also add the commit message that will be in the git repo, so 
that it can receive comments without needing to find the v1.

For other reviewers' convenience, here's the original email: 
https://inbox.sourceware.org/gdb-patches/CAB3ousCKmytOaWJMNC4kBm8oBqmUWOywg_bEnTutmm9M6vty_Q@mail.gmail.com/T/#m20c344a2b51c35bcb4f678a209f5c5eb986fbb1b

I only have one formatting nit I didn't notice on v1, but there's no 
need to send a v3 just for that. Feel free to add my review tag to the 
end of the commit message!

Reviewed-By: Guinevere Larsen <guinevere@redhat.com>

This isn't enough to push the commit, I hope a global maintainer 
approves this patch soon.

> ---
>   gdb/disasm.c                     | 18 ++++++++++---
>   gdb/printcmd.c                   |  2 +-
>   gdb/testsuite/gdb.base/radix.c   | 35 ++++++++++++++++++++++++++
>   gdb/testsuite/gdb.base/radix.exp | 43 ++++++++++++++++++++++++++++++++
>   gdb/valprint.c                   | 13 ++++++++++
>   gdb/valprint.h                   |  6 +++++
>   6 files changed, 112 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'.  */
Small nit that I didn't notice in the first version, but this is not 
lined up correctly, it should be 1 tab and 3 spaces.
> +	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 ());
I'm going to add this for reference for other reviewers: It seems that 
radix is respected in other commands, like print. So I think it would be 
reasonable to make it respected in the disas command as well, but since 
I don't know how MI consumers work, I also don't know if something would 
break here... so yeah, I think it might be fine, don't really know 
enough but the possibility of MI consumers breaking makes me cautious.
> +	  }
>   	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..8a4d1287208
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/radix.c
> @@ -0,0 +1,35 @@
> +/* 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/>.  */
> +
> +#include <unistd.h>
> +#include <stdio.h>
> +
> +static int v;
> +
> +int
> +main (void)
> +{
> +  v = 0;
> +
> +  puts ("hello world");
> +
> +  printf ("this is another string\n");
> +
> +  v += 3;
> +
> +  return v;
> +}
> diff --git a/gdb/testsuite/gdb.base/radix.exp b/gdb/testsuite/gdb.base/radix.exp
> index 7a4320bbf36..4b8e2d74b49 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,41 @@ 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/i with output-radix $oradix"
> +
> +      gdb_test_multiple "x/i \$pc" "$test 1" {
> +	-re -wrap "<main\\+$offset_re>:.*" {
> +	   pass $gdb_test_name
> +	}
> +      }
> +
> +      gdb_test "ni 3" "\[0-9\]+.*" "Next instruction for radix $oradix"
> +
> +      gdb_test_multiple "x/i \$pc" "$test 2" {
> +	-re -wrap "<main\\+$offset_re>:.*" {
> +	   pass $gdb_test_name
> +	}
> +      }
> +    }
> +
> +    test_pc_offset_radix 8  {0[0-7]+}
> +    test_pc_offset_radix 10 {[1-9][0-9]*}
> +    test_pc_offset_radix 16 {0x[0-9a-f]+}
> +
> +    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..d0d0472ca7a 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -171,6 +171,19 @@ show_output_radix (struct ui_file *file, int from_tty,
>   	      value);
>   }
>   
> +/* See valprint.h.  */
> +
> +std::string
> +format_pc_offset (int offset)
> +{
> +  std::string sign = (offset < 0) ? "-" : "+";
> +
> +  std::string body = int_string (offset < 0 ? -offset : offset, output_radix,
> +                                 0, 0, 1);
> +
> +  return 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.  */
>   
> 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

[PATCH v2] Use "output-radix" setting to format function offsets

[Zander Work]

Thank you for the review Guinevere! I fixed the formatting issue you
flagged, and added your review tag.

Global maintainers: I haven't done a FSF copyright assignment, please
let me know how I should do that.

Below is the full original patch context.

~~~~~

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

Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
---
 gdb/disasm.c                     | 18 ++++++++++---
 gdb/printcmd.c                   |  2 +-
 gdb/testsuite/gdb.base/radix.c   | 35 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/radix.exp | 43 ++++++++++++++++++++++++++++++++
 gdb/valprint.c                   | 13 ++++++++++
 gdb/valprint.h                   |  6 +++++
 6 files changed, 112 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/radix.c

diff --git a/gdb/disasm.c b/gdb/disasm.c
index 81c466c188a..64a9289e070 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);
+	  }
+	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..8a4d1287208
--- /dev/null
+++ b/gdb/testsuite/gdb.base/radix.c
@@ -0,0 +1,35 @@
+/* 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/>.  */
+
+#include <unistd.h>
+#include <stdio.h>
+
+static int v;
+
+int
+main (void)
+{
+  v = 0;
+
+  puts ("hello world");
+
+  printf ("this is another string\n");
+
+  v += 3;
+
+  return v;
+}
diff --git a/gdb/testsuite/gdb.base/radix.exp b/gdb/testsuite/gdb.base/radix.exp
index 7a4320bbf36..4b8e2d74b49 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,41 @@ 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/i with output-radix $oradix"
+
+      gdb_test_multiple "x/i \$pc" "$test 1" {
+	-re -wrap "<main\\+$offset_re>:.*" {
+	   pass $gdb_test_name
+	}
+      }
+
+      gdb_test "ni 3" "\[0-9\]+.*" "Next instruction for radix $oradix"
+
+      gdb_test_multiple "x/i \$pc" "$test 2" {
+	-re -wrap "<main\\+$offset_re>:.*" {
+	   pass $gdb_test_name
+	}
+      }
+    }
+
+    test_pc_offset_radix 8  {0[0-7]+}
+    test_pc_offset_radix 10 {[1-9][0-9]*}
+    test_pc_offset_radix 16 {0x[0-9a-f]+}
+
+    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..d0d0472ca7a 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -171,6 +171,19 @@ show_output_radix (struct ui_file *file, int from_tty,
 	      value);
 }
 
+/* See valprint.h.  */
+
+std::string
+format_pc_offset (int offset)
+{
+  std::string sign = (offset < 0) ? "-" : "+";
+
+  std::string body = int_string (offset < 0 ? -offset : offset, output_radix,
+                                 0, 0, 1);
+
+  return 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.  */
 
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.
-- 
2.54.0.563.g4f69b47b94-goog

Re: [PATCH v2] Use "output-radix" setting to format function offsets

[Eli Zaretskii]

> From: Zander Work <zdw@google.com>
> Cc: guinevere@redhat.com,
> 	tom@tromey.com,
> 	Zander Work <zdw@google.com>
> Date: Mon, 11 May 2026 18:32:38 -0700
> 
> Thank you for the review Guinevere! I fixed the formatting issue you
> flagged, and added your review tag.
> 
> Global maintainers: I haven't done a FSF copyright assignment, please
> let me know how I should do that.

If you are an employee of Google, I think they have a blanket
assignment covering all of their employees?  Tom, am I right?

Failing that, we can send you the form to fill and the instructions to
go with it, so you could start your assignment paperwork rolling.

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

Yes, this needs an entry in NEWS.  I think the descriptions of
output-radix and of the "disassemble" command in the GDB manual should
also be updated to reflect the change.

Thanks.

Re: [PATCH v2] Use "output-radix" setting to format function offsets

[Zander Work]

On Tue, May 12, 2026 at 4:33 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Zander Work <zdw@google.com>
> > Cc: guinevere@redhat.com,
> >       tom@tromey.com,
> >       Zander Work <zdw@google.com>
> > Date: Mon, 11 May 2026 18:32:38 -0700
> >
> > Thank you for the review Guinevere! I fixed the formatting issue you
> > flagged, and added your review tag.
> >
> > Global maintainers: I haven't done a FSF copyright assignment, please
> > let me know how I should do that.
>
> If you are an employee of Google, I think they have a blanket
> assignment covering all of their employees?  Tom, am I right?

Our internal documentation says that Google has a CLA on file with
FSF, but the red warning on the Contributor Guide section 6 [1] makes
me think that isn't sufficient

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

>
> Failing that, we can send you the form to fill and the instructions to
> go with it, so you could start your assignment paperwork rolling.
>
> > 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.
>
> Yes, this needs an entry in NEWS.  I think the descriptions of
> output-radix and of the "disassemble" command in the GDB manual should
> also be updated to reflect the change.

Sure thing, will update this patch with that later today.

>
> Thanks.

Re: [PATCH v2] Use "output-radix" setting to format function offsets

[Eli Zaretskii]

> From: Zander Work <zdw@google.com>
> Date: Tue, 12 May 2026 06:05:21 -0700
> Cc: gdb-patches@sourceware.org, guinevere@redhat.com, tom@tromey.com
> 
> On Tue, May 12, 2026 at 4:33 AM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > From: Zander Work <zdw@google.com>
> > > Cc: guinevere@redhat.com,
> > >       tom@tromey.com,
> > >       Zander Work <zdw@google.com>
> > > Date: Mon, 11 May 2026 18:32:38 -0700
> > >
> > > Thank you for the review Guinevere! I fixed the formatting issue you
> > > flagged, and added your review tag.
> > >
> > > Global maintainers: I haven't done a FSF copyright assignment, please
> > > let me know how I should do that.
> >
> > If you are an employee of Google, I think they have a blanket
> > assignment covering all of their employees?  Tom, am I right?
> 
> Our internal documentation says that Google has a CLA on file with
> FSF, but the red warning on the Contributor Guide section 6 [1] makes
> me think that isn't sufficient
> 
> [1] https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment

I sent the form off-list, just in case.

Re: [PATCH v2] Use "output-radix" setting to format function offsets

[Simon Marchi]

On 2026-05-12 09:05, Zander Work wrote:
> On Tue, May 12, 2026 at 4:33 AM Eli Zaretskii <eliz@gnu.org> wrote:
>>
>>> From: Zander Work <zdw@google.com>
>>> Cc: guinevere@redhat.com,
>>>       tom@tromey.com,
>>>       Zander Work <zdw@google.com>
>>> Date: Mon, 11 May 2026 18:32:38 -0700
>>>
>>> Thank you for the review Guinevere! I fixed the formatting issue you
>>> flagged, and added your review tag.
>>>
>>> Global maintainers: I haven't done a FSF copyright assignment, please
>>> let me know how I should do that.
>>
>> If you are an employee of Google, I think they have a blanket
>> assignment covering all of their employees?  Tom, am I right?
> 
> Our internal documentation says that Google has a CLA on file with
> FSF, but the red warning on the Contributor Guide section 6 [1] makes
> me think that isn't sufficient
> 
> [1] https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment

That is strange, because many of us (me included) contribute under a blanket assignment.

Unless it has been revoked for some reason, I also remember Google
having a blanket assigment for all GNU tools projects.  I see an entry
for that in the copyright assignment file we have access to.  Can you
ask the FSF clerk to confirm that?

Simon

Re: [PATCH v2] Use "output-radix" setting to format function offsets

[Zander Work]

On Thu, May 14, 2026 at 10:23 AM Simon Marchi <simark@simark.ca> wrote:
>
>
>
> On 2026-05-12 09:05, Zander Work wrote:
> > On Tue, May 12, 2026 at 4:33 AM Eli Zaretskii <eliz@gnu.org> wrote:
> >>
> >>> From: Zander Work <zdw@google.com>
> >>> Cc: guinevere@redhat.com,
> >>>       tom@tromey.com,
> >>>       Zander Work <zdw@google.com>
> >>> Date: Mon, 11 May 2026 18:32:38 -0700
> >>>
> >>> Thank you for the review Guinevere! I fixed the formatting issue you
> >>> flagged, and added your review tag.
> >>>
> >>> Global maintainers: I haven't done a FSF copyright assignment, please
> >>> let me know how I should do that.
> >>
> >> If you are an employee of Google, I think they have a blanket
> >> assignment covering all of their employees?  Tom, am I right?
> >
> > Our internal documentation says that Google has a CLA on file with
> > FSF, but the red warning on the Contributor Guide section 6 [1] makes
> > me think that isn't sufficient
> >
> > [1] https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment
>
> That is strange, because many of us (me included) contribute under a blanket assignment.
>
> Unless it has been revoked for some reason, I also remember Google
> having a blanket assigment for all GNU tools projects.  I see an entry
> for that in the copyright assignment file we have access to.  Can you
> ask the FSF clerk to confirm that?

I will try and get a conclusive answer on that and will follow up
here, thanks for the additional info Simon

>
> Simon

Re: [PATCH v2] Use "output-radix" setting to format function offsets

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> If you are an employee of Google, I think they have a blanket
Eli> assignment covering all of their employees?  Tom, am I right?

I do not know.

Tom

Re: [PATCH v2] Use "output-radix" setting to format function offsets

[Zander Work]

Thanks for the feedback Tom. I sent the initial information Eli sent
me off-list to assign@gnu.org yesterday.

If the consensus from folks is that a different approach should be
taken to this, happy to make adjustments. I did work on a docs update
yesterday but ran into issues trying to actually get the documentation
compiled so haven't sent that yet.

On Wed, May 13, 2026 at 12:38 PM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
>
> Eli> If you are an employee of Google, I think they have a blanket
> Eli> assignment covering all of their employees?  Tom, am I right?
>
> I do not know.
>
> Tom

Re: [PATCH v2] Use "output-radix" setting to format function offsets

[Tom Tromey]

>>>>> Zander Work <zdw@google.com> writes:

> Updates since v1:
[...]

It's normal in gdb for the commit message to describe the purpose of the
patch.

> +std::string
> +format_pc_offset (int offset)
> +{
> +  std::string sign = (offset < 0) ? "-" : "+";
> +
> +  std::string body = int_string (offset < 0 ? -offset : offset, output_radix,
> +                                 0, 0, 1);

I don't have any objection to this but I wanted to note that using the
output radix is kind of heavy, since it affects all sorts of output.

That is, I tend to think a new option would be preferable.  Though,
unfortunately, that would be more work since it would involve more
documentation at least.

Another possibility is that nobody really even wants decimal here and
that using hex always would be fine.  I mean -- that would be fine by me
but we should see if anyone feels differently.

Finally, I think print_address_symbolic should probably do the same
thing, i.e., it needs an update:

  if (offset != 0)
    gdb_printf (stream, "%+d", offset);

thanks,
Tom