[PATCH] gdbserver: Update require_int function to parse offset for pread packet

[PATCH] gdbserver: Update require_int function to parse offset for pread packet

[Kirill Radkin]

Currently gdbserver uses require_int() function to parse the requested offset
(in vFile::pread packet and the like). This function allows integers up to
0x7fffffff (to fit in 32-bit int), however the offset (for pread system call)
has an off_t type which can be larger than 32-bit.

This patch allows require_int() function to parse offset up to the maximum
value implied by the off_t type.
---
 gdb/testsuite/gdb.server/pread-offset-size.S  | 26 +++++++++++
 .../gdb.server/pread-offset-size.exp          | 45 +++++++++++++++++++
 gdbserver/hostio.cc                           | 18 +++++---
 3 files changed, 84 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/gdb.server/pread-offset-size.S
 create mode 100644 gdb/testsuite/gdb.server/pread-offset-size.exp

diff --git a/gdb/testsuite/gdb.server/pread-offset-size.S b/gdb/testsuite/gdb.server/pread-offset-size.S
new file mode 100644
index 00000000000..31748090ac3
--- /dev/null
+++ b/gdb/testsuite/gdb.server/pread-offset-size.S
@@ -0,0 +1,26 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023-2023 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/>.  */
+
+	.text
+	.globl	_start
+_start:
+	.skip 3742415472
+   ret
+	.globl	f
+	.type	f, @function
+f:
+   ret
diff --git a/gdb/testsuite/gdb.server/pread-offset-size.exp b/gdb/testsuite/gdb.server/pread-offset-size.exp
new file mode 100644
index 00000000000..c6acaa382b2
--- /dev/null
+++ b/gdb/testsuite/gdb.server/pread-offset-size.exp
@@ -0,0 +1,45 @@
+# Copyright (C) 2023-2023 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/>.
+
+load_lib gdbserver-support.exp
+
+require allow_gdbserver_tests
+
+standard_testfile .S
+
+if { [prepare_for_testing ${testfile}.exp $testfile \
+        $srcfile {debug additional_flags=-nostdlib} ] } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+
+gdb_test_no_output "set remote exec-file $binfile" \
+"set remote exec-file"
+
+# Make sure we're disconnected, in case we're testing with an
+# extended-remote board, therefore already connected.
+gdb_test "disconnect" ".*"
+
+set res [gdbserver_spawn ""]
+set gdbserver_protocol [lindex $res 0]
+set gdbserver_gdbport [lindex $res 1]
+
+gdb_test "target $gdbserver_protocol $gdbserver_gdbport" \
+"Remote debugging using .*" \
+"target $gdbserver_protocol $gdbserver_gdbport"
+
+gdb_test "break f" "Breakpoint 1.*"
diff --git a/gdbserver/hostio.cc b/gdbserver/hostio.cc
index 17b6179d8ca..2207804670b 100644
--- a/gdbserver/hostio.cc
+++ b/gdbserver/hostio.cc
@@ -89,12 +89,18 @@ require_filename (char **pp, char *filename)
   return 0;
 }
 
+template <typename T>
 static int
-require_int (char **pp, int *value)
+require_int (char **pp, T *value)
 {
+  constexpr bool is_signed = std::is_signed<T>::value;
+
   char *p;
   int count, firstdigit;
 
+  /* Max count of hexadecimal digits in off_t (1 hex digit is 4 bits) */
+  int max_count = sizeof(T) * CHAR_BIT / 4;
+
   p = *pp;
   *value = 0;
   count = 0;
@@ -111,9 +117,9 @@ require_int (char **pp, int *value)
 	firstdigit = nib;
 
       /* Don't allow overflow.  */
-      if (count >= 8 || (count == 7 && firstdigit >= 0x8))
+      if (count >= max_count
+	  || (is_signed && count == (max_count - 1) && firstdigit >= 0x8))
 	return -1;
-
       *value = *value * 16 + nib;
       p++;
       count++;
@@ -343,7 +349,8 @@ handle_open (char *own_buf)
 static void
 handle_pread (char *own_buf, int *new_packet_len)
 {
-  int fd, ret, len, offset, bytes_sent;
+  int fd, ret, len, bytes_sent;
+  off_t offset;
   char *p, *data;
   static int max_reply_size = -1;
 
@@ -410,7 +417,8 @@ handle_pread (char *own_buf, int *new_packet_len)
 static void
 handle_pwrite (char *own_buf, int packet_len)
 {
-  int fd, ret, len, offset;
+  int fd, ret, len;
+  off_t offset;
   char *p, *data;
 
   p = own_buf + strlen ("vFile:pwrite:");
-- 
2.43.0

Re: [PATCH] gdbserver: Update require_int function to parse offset for pread packet

[Kevin Buettner]

Hi Kirill,

Just a few nits...

On Mon, 21 Apr 2025 14:59:41 +0300
Kirill Radkin <kirill.radkin@syntacore.com> wrote:

> Currently gdbserver uses require_int() function to parse the requested offset
> (in vFile::pread packet and the like). This function allows integers up to
> 0x7fffffff (to fit in 32-bit int), however the offset (for pread system call)
> has an off_t type which can be larger than 32-bit.
> 
> This patch allows require_int() function to parse offset up to the maximum
> value implied by the off_t type.
> ---
>  gdb/testsuite/gdb.server/pread-offset-size.S  | 26 +++++++++++
>  .../gdb.server/pread-offset-size.exp          | 45 +++++++++++++++++++
>  gdbserver/hostio.cc                           | 18 +++++---
>  3 files changed, 84 insertions(+), 5 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.server/pread-offset-size.S
>  create mode 100644 gdb/testsuite/gdb.server/pread-offset-size.exp
> 
> diff --git a/gdb/testsuite/gdb.server/pread-offset-size.S b/gdb/testsuite/gdb.server/pread-offset-size.S
> new file mode 100644
> index 00000000000..31748090ac3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.server/pread-offset-size.S
> @@ -0,0 +1,26 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2023-2023 Free Software Foundation, Inc.

Please update the second 2023 to 2025.

(Do you even need the first 2023?)

> +
> +   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/>.  */
> +
> +	.text
> +	.globl	_start
> +_start:
> +	.skip 3742415472
> +   ret
> +	.globl	f
> +	.type	f, @function
> +f:
> +   ret
> diff --git a/gdb/testsuite/gdb.server/pread-offset-size.exp b/gdb/testsuite/gdb.server/pread-offset-size.exp
> new file mode 100644
> index 00000000000..c6acaa382b2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.server/pread-offset-size.exp
> @@ -0,0 +1,45 @@
> +# Copyright (C) 2023-2023 Free Software Foundation, Inc.

Likewise.

> +#
> +# 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/>.
> +
> +load_lib gdbserver-support.exp
> +
> +require allow_gdbserver_tests
> +
> +standard_testfile .S
> +
> +if { [prepare_for_testing ${testfile}.exp $testfile \
> +        $srcfile {debug additional_flags=-nostdlib} ] } {

git am complained of a whitespace problem here.  Basically, it
wants you to use a tab instead of the leading 8 spaces.

> +    return -1
> +}
> +
> +gdb_exit
> +gdb_start
> +
> +gdb_test_no_output "set remote exec-file $binfile" \
> +"set remote exec-file"
> +
> +# Make sure we're disconnected, in case we're testing with an
> +# extended-remote board, therefore already connected.
> +gdb_test "disconnect" ".*"
> +
> +set res [gdbserver_spawn ""]
> +set gdbserver_protocol [lindex $res 0]
> +set gdbserver_gdbport [lindex $res 1]
> +
> +gdb_test "target $gdbserver_protocol $gdbserver_gdbport" \
> +"Remote debugging using .*" \
> +"target $gdbserver_protocol $gdbserver_gdbport"
> +
> +gdb_test "break f" "Breakpoint 1.*"
> diff --git a/gdbserver/hostio.cc b/gdbserver/hostio.cc
> index 17b6179d8ca..2207804670b 100644
> --- a/gdbserver/hostio.cc
> +++ b/gdbserver/hostio.cc
> @@ -89,12 +89,18 @@ require_filename (char **pp, char *filename)
>    return 0;
>  }
>  
> +template <typename T>
>  static int
> -require_int (char **pp, int *value)
> +require_int (char **pp, T *value)
>  {
> +  constexpr bool is_signed = std::is_signed<T>::value;
> +
>    char *p;
>    int count, firstdigit;
>  
> +  /* Max count of hexadecimal digits in off_t (1 hex digit is 4 bits) */

Please place a period (.) at the end of the comment and make sure
that it's followed by 2 spaces.

> +  int max_count = sizeof(T) * CHAR_BIT / 4;
> +
>    p = *pp;
>    *value = 0;
>    count = 0;
> @@ -111,9 +117,9 @@ require_int (char **pp, int *value)
>  	firstdigit = nib;
>  
>        /* Don't allow overflow.  */
> -      if (count >= 8 || (count == 7 && firstdigit >= 0x8))
> +      if (count >= max_count
> +	  || (is_signed && count == (max_count - 1) && firstdigit >= 0x8))
>  	return -1;
> -
>        *value = *value * 16 + nib;
>        p++;
>        count++;
> @@ -343,7 +349,8 @@ handle_open (char *own_buf)
>  static void
>  handle_pread (char *own_buf, int *new_packet_len)
>  {
> -  int fd, ret, len, offset, bytes_sent;
> +  int fd, ret, len, bytes_sent;
> +  off_t offset;
>    char *p, *data;
>    static int max_reply_size = -1;
>  
> @@ -410,7 +417,8 @@ handle_pread (char *own_buf, int *new_packet_len)
>  static void
>  handle_pwrite (char *own_buf, int packet_len)
>  {
> -  int fd, ret, len, offset;
> +  int fd, ret, len;
> +  off_t offset;
>    char *p, *data;
>  
>    p = own_buf + strlen ("vFile:pwrite:");
> -- 
> 2.43.0
> 
> 

[PATCH] gdbserver: Update require_int function to parse offset for pread packet

[Kirill Radkin]

Currently gdbserver uses require_int() function to parse the requested offset
(in vFile::pread packet and the like). This function allows integers up to
0x7fffffff (to fit in 32-bit int), however the offset (for pread system call)
has an off_t type which can be larger than 32-bit.

This patch allows require_int() function to parse offset up to the maximum
value implied by the off_t type.
---
 gdb/testsuite/gdb.server/pread-offset-size.S  | 26 +++++++++++
 .../gdb.server/pread-offset-size.exp          | 45 +++++++++++++++++++
 gdbserver/hostio.cc                           | 18 +++++---
 3 files changed, 84 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/gdb.server/pread-offset-size.S
 create mode 100644 gdb/testsuite/gdb.server/pread-offset-size.exp

diff --git a/gdb/testsuite/gdb.server/pread-offset-size.S b/gdb/testsuite/gdb.server/pread-offset-size.S
new file mode 100644
index 00000000000..f43e059653d
--- /dev/null
+++ b/gdb/testsuite/gdb.server/pread-offset-size.S
@@ -0,0 +1,26 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 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/>.  */
+
+	.text
+	.globl	_start
+_start:
+	.skip 3742415472
+   ret
+	.globl	f
+	.type	f, @function
+f:
+   ret
diff --git a/gdb/testsuite/gdb.server/pread-offset-size.exp b/gdb/testsuite/gdb.server/pread-offset-size.exp
new file mode 100644
index 00000000000..5faf2162ef0
--- /dev/null
+++ b/gdb/testsuite/gdb.server/pread-offset-size.exp
@@ -0,0 +1,45 @@
+# Copyright (C) 2025 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/>.
+
+load_lib gdbserver-support.exp
+
+require allow_gdbserver_tests
+
+standard_testfile .S
+
+if { [prepare_for_testing ${testfile}.exp $testfile \
+	$srcfile {debug additional_flags=-nostdlib} ] } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+
+gdb_test_no_output "set remote exec-file $binfile" \
+"set remote exec-file"
+
+# Make sure we're disconnected, in case we're testing with an
+# extended-remote board, therefore already connected.
+gdb_test "disconnect" ".*"
+
+set res [gdbserver_spawn ""]
+set gdbserver_protocol [lindex $res 0]
+set gdbserver_gdbport [lindex $res 1]
+
+gdb_test "target $gdbserver_protocol $gdbserver_gdbport" \
+"Remote debugging using .*" \
+"target $gdbserver_protocol $gdbserver_gdbport"
+
+gdb_test "break f" "Breakpoint 1.*"
diff --git a/gdbserver/hostio.cc b/gdbserver/hostio.cc
index 17b6179d8ca..e88f8d79728 100644
--- a/gdbserver/hostio.cc
+++ b/gdbserver/hostio.cc
@@ -89,12 +89,18 @@ require_filename (char **pp, char *filename)
   return 0;
 }
 
+template <typename T>
 static int
-require_int (char **pp, int *value)
+require_int (char **pp, T *value)
 {
+  constexpr bool is_signed = std::is_signed<T>::value;
+
   char *p;
   int count, firstdigit;
 
+  /* Max count of hexadecimal digits in off_t (1 hex digit is 4 bits).  */
+  int max_count = sizeof(T) * CHAR_BIT / 4;
+
   p = *pp;
   *value = 0;
   count = 0;
@@ -111,9 +117,9 @@ require_int (char **pp, int *value)
 	firstdigit = nib;
 
       /* Don't allow overflow.  */
-      if (count >= 8 || (count == 7 && firstdigit >= 0x8))
+      if (count >= max_count
+	  || (is_signed && count == (max_count - 1) && firstdigit >= 0x8))
 	return -1;
-
       *value = *value * 16 + nib;
       p++;
       count++;
@@ -343,7 +349,8 @@ handle_open (char *own_buf)
 static void
 handle_pread (char *own_buf, int *new_packet_len)
 {
-  int fd, ret, len, offset, bytes_sent;
+  int fd, ret, len, bytes_sent;
+  off_t offset;
   char *p, *data;
   static int max_reply_size = -1;
 
@@ -410,7 +417,8 @@ handle_pread (char *own_buf, int *new_packet_len)
 static void
 handle_pwrite (char *own_buf, int packet_len)
 {
-  int fd, ret, len, offset;
+  int fd, ret, len;
+  off_t offset;
   char *p, *data;
 
   p = own_buf + strlen ("vFile:pwrite:");
-- 
2.43.0

Re: [PATCH] gdbserver: Update require_int function to parse offset for pread packet

[Pedro Alves]

On 2025-05-28 12:35, Kirill Radkin wrote:
> Currently gdbserver uses require_int() function to parse the requested offset
> (in vFile::pread packet and the like). This function allows integers up to
> 0x7fffffff (to fit in 32-bit int), however the offset (for pread system call)
> has an off_t type which can be larger than 32-bit.
> 
> This patch allows require_int() function to parse offset up to the maximum
> value implied by the off_t type.
> ---
>  gdb/testsuite/gdb.server/pread-offset-size.S  | 26 +++++++++++
>  .../gdb.server/pread-offset-size.exp          | 45 +++++++++++++++++++
>  gdbserver/hostio.cc                           | 18 +++++---
>  3 files changed, 84 insertions(+), 5 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.server/pread-offset-size.S
>  create mode 100644 gdb/testsuite/gdb.server/pread-offset-size.exp
> 
> diff --git a/gdb/testsuite/gdb.server/pread-offset-size.S b/gdb/testsuite/gdb.server/pread-offset-size.S
> new file mode 100644
> index 00000000000..f43e059653d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.server/pread-offset-size.S
> @@ -0,0 +1,26 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2025 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/>.  */
> +

Please add a short comment stating what this is trying to do.

> +	.text
> +	.globl	_start
> +_start:
> +	.skip 3742415472

Is this particular number significant?

> +   ret
> +	.globl	f
> +	.type	f, @function
> +f:
> +   ret
> diff --git a/gdb/testsuite/gdb.server/pread-offset-size.exp b/gdb/testsuite/gdb.server/pread-offset-size.exp
> new file mode 100644
> index 00000000000..5faf2162ef0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.server/pread-offset-size.exp
> @@ -0,0 +1,45 @@
> +# Copyright (C) 2025 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/>.
> +

This needs a short blurb here stating what the testcase is testing.

> +load_lib gdbserver-support.exp
> +
> +require allow_gdbserver_tests
> +
> +standard_testfile .S
> +
> +if { [prepare_for_testing ${testfile}.exp $testfile \
> +	$srcfile {debug additional_flags=-nostdlib} ] } {
> +    return -1
> +}
> +
> +gdb_exit
> +gdb_start

clean_restart

> +
> +gdb_test_no_output "set remote exec-file $binfile" \
> +"set remote exec-file"

Something's off with the indentation of the second line.  As in, it's not indented.  :-)

Indent it by four spaces, like:

gdb_test_no_output "set remote exec-file $binfile" \
    "set remote exec-file"

> +
> +# Make sure we're disconnected, in case we're testing with an
> +# extended-remote board, therefore already connected.
> +gdb_test "disconnect" ".*"
> +
> +set res [gdbserver_spawn ""]
> +set gdbserver_protocol [lindex $res 0]
> +set gdbserver_gdbport [lindex $res 1]
> +
> +gdb_test "target $gdbserver_protocol $gdbserver_gdbport" \
> +"Remote debugging using .*" \
> +"target $gdbserver_protocol $gdbserver_gdbport"

Ditto.  But also, you'll notice that other tests doing something similar avoid
putting the port number in gdb.sum, so that comparing test results is a bit
more stable.  So:

gdb_test "target $gdbserver_protocol $gdbserver_gdbport" \
    "Remote debugging using .*" \
    "target $gdbserver_protocol"

> +
> +gdb_test "break f" "Breakpoint 1.*"

Is this where the problem would manifest?  Some comments would help.

> diff --git a/gdbserver/hostio.cc b/gdbserver/hostio.cc
> index 17b6179d8ca..e88f8d79728 100644
> --- a/gdbserver/hostio.cc
> +++ b/gdbserver/hostio.cc
> @@ -89,12 +89,18 @@ require_filename (char **pp, char *filename)
>    return 0;
>  }
>  
> +template <typename T>
>  static int
> -require_int (char **pp, int *value)
> +require_int (char **pp, T *value)
>  {
> +  constexpr bool is_signed = std::is_signed<T>::value;
> +
>    char *p;
>    int count, firstdigit;
>  
> +  /* Max count of hexadecimal digits in off_t (1 hex digit is 4 bits).  */

"T", not "off_t".

> +  int max_count = sizeof(T) * CHAR_BIT / 4;

Space before parens.

> +
>    p = *pp;
>    *value = 0;
>    count = 0;
> @@ -111,9 +117,9 @@ require_int (char **pp, int *value)
>  	firstdigit = nib;
>  
>        /* Don't allow overflow.  */
> -      if (count >= 8 || (count == 7 && firstdigit >= 0x8))
> +      if (count >= max_count
> +	  || (is_signed && count == (max_count - 1) && firstdigit >= 0x8))
>  	return -1;
> -

Keep the empty line, so the comment above is clear to refer
to the whole block up to the empty line.

>        *value = *value * 16 + nib;
>        p++;
>        count++;
> @@ -343,7 +349,8 @@ handle_open (char *own_buf)
>  static void
>  handle_pread (char *own_buf, int *new_packet_len)
>  {
> -  int fd, ret, len, offset, bytes_sent;
> +  int fd, ret, len, bytes_sent;
> +  off_t offset;
>    char *p, *data;
>    static int max_reply_size = -1;
>  
> @@ -410,7 +417,8 @@ handle_pread (char *own_buf, int *new_packet_len)
>  static void
>  handle_pwrite (char *own_buf, int packet_len)
>  {
> -  int fd, ret, len, offset;
> +  int fd, ret, len;
> +  off_t offset;
>    char *p, *data;
>  
>    p = own_buf + strlen ("vFile:pwrite:");

[PATCH] gdbserver: Update require_int function to parse offset for pread packet

[Kirill Radkin]

I've addressed all review comments.
Also, additional information about issue was added to commit message.

---

Recently I have encountered an issue with gdbserver not being able to send debug information from large \
files (> 2Gb). I found that this issue is connected with parsing offsets (integer) from vFile::pread \
packets. There is already a patch (https://sourceware.org/bugzilla/show_bug.cgi?id=23198) that allows to \
parse integers up to 0x7fffffff (32-bit integer), but it doesn't fix the problem at all, because offsets \
can have a off_t type which can be larger than 32-bit integer (it depends on system).

This patch allows require_int() function to parse offset up to the maximum value implied by the off_t \
type.

Currently gdbserver uses require_int() function to parse the requested offset
(in vFile::pread packet and the like). This function allows integers up to
0x7fffffff (to fit in 32-bit int), however the offset (for pread system call)
has an off_t type which can be larger than 32-bit.

This patch allows require_int() function to parse offset up to the maximum
value implied by the off_t type.
---
 gdb/testsuite/gdb.server/pread-offset-size.S  | 29 +++++++++++
 .../gdb.server/pread-offset-size.exp          | 50 +++++++++++++++++++
 gdbserver/hostio.cc                           | 17 +++++--
 3 files changed, 92 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.server/pread-offset-size.S
 create mode 100644 gdb/testsuite/gdb.server/pread-offset-size.exp

diff --git a/gdb/testsuite/gdb.server/pread-offset-size.S b/gdb/testsuite/gdb.server/pread-offset-size.S
new file mode 100644
index 00000000000..63b2faa213d
--- /dev/null
+++ b/gdb/testsuite/gdb.server/pread-offset-size.S
@@ -0,0 +1,29 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 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/>.  */
+
+/* Here we are trying to create a large binary (> 2 GB),
+   3742415472 bytes is about 3,5 gigabytes.  */
+
+	.text
+	.globl	_start
+_start:
+	.skip 3742415472
+   ret
+	.globl	f
+	.type	f, @function
+f:
+   ret
diff --git a/gdb/testsuite/gdb.server/pread-offset-size.exp b/gdb/testsuite/gdb.server/pread-offset-size.exp
new file mode 100644
index 00000000000..e224e971ad5
--- /dev/null
+++ b/gdb/testsuite/gdb.server/pread-offset-size.exp
@@ -0,0 +1,50 @@
+# Copyright (C) 2025 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/>.
+#
+# Testing fix of a problem with gdbserver not being able to send
+# debug information from large files (> 2GB)
+
+load_lib gdbserver-support.exp
+
+require allow_gdbserver_tests
+
+standard_testfile .S
+
+if { [prepare_for_testing ${testfile}.exp $testfile \
+	$srcfile {debug additional_flags=-nostdlib} ] } {
+    return -1
+}
+
+clean_restart
+
+gdb_test_no_output "set remote exec-file $binfile" \
+    "set remote exec-file"
+
+# Make sure we're disconnected, in case we're testing with an
+# extended-remote board, therefore already connected.
+gdb_test "disconnect" ".*"
+
+set res [gdbserver_spawn ""]
+set gdbserver_protocol [lindex $res 0]
+set gdbserver_gdbport [lindex $res 1]
+
+gdb_test "target $gdbserver_protocol $gdbserver_gdbport" \
+    "Remote debugging using .*" \
+    "target $gdbserver_protocol"
+
+# If load of a large binary was successful,
+# we should be able to place a break on f
+
+gdb_test "break f" "Breakpoint 1.*"
diff --git a/gdbserver/hostio.cc b/gdbserver/hostio.cc
index 17b6179d8ca..4e77253750a 100644
--- a/gdbserver/hostio.cc
+++ b/gdbserver/hostio.cc
@@ -89,12 +89,18 @@ require_filename (char **pp, char *filename)
   return 0;
 }
 
+template <typename T>
 static int
-require_int (char **pp, int *value)
+require_int (char **pp, T *value)
 {
+  constexpr bool is_signed = std::is_signed<T>::value;
+
   char *p;
   int count, firstdigit;
 
+  /* Max count of hexadecimal digits in T (1 hex digit is 4 bits).  */
+  int max_count = sizeof (T) * CHAR_BIT / 4;
+
   p = *pp;
   *value = 0;
   count = 0;
@@ -111,7 +117,8 @@ require_int (char **pp, int *value)
 	firstdigit = nib;
 
       /* Don't allow overflow.  */
-      if (count >= 8 || (count == 7 && firstdigit >= 0x8))
+      if (count >= max_count
+	  || (is_signed && count == (max_count - 1) && firstdigit >= 0x8))
 	return -1;
 
       *value = *value * 16 + nib;
@@ -343,7 +350,8 @@ handle_open (char *own_buf)
 static void
 handle_pread (char *own_buf, int *new_packet_len)
 {
-  int fd, ret, len, offset, bytes_sent;
+  int fd, ret, len, bytes_sent;
+  off_t offset;
   char *p, *data;
   static int max_reply_size = -1;
 
@@ -410,7 +418,8 @@ handle_pread (char *own_buf, int *new_packet_len)
 static void
 handle_pwrite (char *own_buf, int packet_len)
 {
-  int fd, ret, len, offset;
+  int fd, ret, len;
+  off_t offset;
   char *p, *data;
 
   p = own_buf + strlen ("vFile:pwrite:");
-- 
2.43.0

Re: [PATCH] gdbserver: Update require_int function to parse offset for pread packet

[Pedro Alves]

Hi!

On 2025-06-03 14:48, Kirill Radkin wrote:
> I've addressed all review comments.
> Also, additional information about issue was added to commit message.

Thank you.  I've now pushed this, with some minor editorial tweaks.

Pedro Alves