1. Patchew
  2. QEMU
  3. View series

[PATCH] gdbstub/user-target: fix gdbserver int format (%d -> %x)

Dominik 'Disconnect3d' Czarnota posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20241227170413.72438-1-dominik.b.czarnota@gmail.com
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
There is a newer version of this series
gdbstub/user-target.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
[PATCH] gdbstub/user-target: fix gdbserver int format (%d -> %x)
Posted by Dominik 'Disconnect3d' Czarnota 1 year, 1 month ago 
From: disconnect3d <dominik.b.czarnota@gmail.com>

This commit fixes an incorrect format string for formatting integers
provided to GDB when debugging a target run in QEMU user mode.

The correct format is hexadecimal for both success and errno values,
some of which can be seen here [0].

[0] https://github.com/bminor/binutils-gdb/blob/e65a355022d0dc6b5707310876a72b5693ec0aa5/gdbserver/hostio.cc#L196-L213
---
 gdbstub/user-target.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdbstub/user-target.c b/gdbstub/user-target.c
index 22bf4008c0..4bfcf78aaa 100644
--- a/gdbstub/user-target.c
+++ b/gdbstub/user-target.c
@@ -317,9 +317,9 @@ void gdb_handle_v_file_open(GArray *params, void *user_ctx)
     int fd = open(filename, flags, mode);
 #endif
     if (fd < 0) {
-        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
+        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
     } else {
-        g_string_printf(gdbserver_state.str_buf, "F%d", fd);
+        g_string_printf(gdbserver_state.str_buf, "F%x", fd);
     }
     gdb_put_strbuf();
 }
@@ -329,7 +329,7 @@ void gdb_handle_v_file_close(GArray *params, void *user_ctx)
     int fd = gdb_get_cmd_param(params, 0)->val_ul;
 
     if (close(fd) == -1) {
-        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
+        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
         gdb_put_strbuf();
         return;
     }
@@ -352,7 +352,7 @@ void gdb_handle_v_file_pread(GArray *params, void *user_ctx)
 
     ssize_t n = pread(fd, buf, bufsiz, offset);
     if (n < 0) {
-        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
+        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
         gdb_put_strbuf();
         return;
     }
@@ -375,7 +375,7 @@ void gdb_handle_v_file_readlink(GArray *params, void *user_ctx)
     ssize_t n = readlink(filename, buf, BUFSIZ);
 #endif
     if (n < 0) {
-        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
+        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
         gdb_put_strbuf();
         return;
     }
-- 
2.30.2
Re: [PATCH] gdbstub/user-target: fix gdbserver int format (%d -> %x)
Posted by Michael Tokarev 1 year ago 
27.12.2024 20:04, Dominik 'Disconnect3d' Czarnota wrote:
> From: disconnect3d <dominik.b.czarnota@gmail.com>
> 
> This commit fixes an incorrect format string for formatting integers
> provided to GDB when debugging a target run in QEMU user mode.
> 
> The correct format is hexadecimal for both success and errno values,
> some of which can be seen here [0].
> 
> [0] https://github.com/bminor/binutils-gdb/blob/e65a355022d0dc6b5707310876a72b5693ec0aa5/gdbserver/hostio.cc#L196-L213

Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

Unfortunately we can't apply this one without Signed-off-by: tag
from you, and maybe without fixing the first From: line.

Thanks,

/mjt
Re: [PATCH] gdbstub/user-target: fix gdbserver int format (%d -> %x)
Posted by Ilya Leoshkevich 1 year, 1 month ago 
On Fri, 2024-12-27 at 18:04 +0100, Dominik 'Disconnect3d' Czarnota
wrote:
> From: disconnect3d <dominik.b.czarnota@gmail.com>
> 
> This commit fixes an incorrect format string for formatting integers
> provided to GDB when debugging a target run in QEMU user mode.
> 
> The correct format is hexadecimal for both success and errno values,
> some of which can be seen here [0].
> 

Nice catch, I haven't tested this with a lot of fds. After adding
3</dev/null 4</dev/null [...] 9</dev/null to an emulated program,
I get:

(gdb) info proc mappings
process 37804
warning: unable to open /proc file '/proc/37804/maps'

And with your fix:

(gdb) info proc mappings
process 37816
Mapped address spaces:

          Start Addr           End Addr       Size     Offset  Perms 
objfile
       0x2aa00000000      0x2aa0123d000  0x123d000        0x0  r-xp  
/home/iii/myrepos/wasmtime/target/s390x-unknown-linux-
gnu/debug/deps/filetests-fab17457420757a9
[...]

I would recommend the following tags:

Fixes: e282010b2e1e ("gdbstub: Add support for info proc mappings")
Cc: qemu-stable@nongnu.org

In any case:

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>

> [0]
> https://github.com/bminor/binutils-gdb/blob/e65a355022d0dc6b5707310876a72b5693ec0aa5/gdbserver/hostio.cc#L196-L213
> ---
>  gdbstub/user-target.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/gdbstub/user-target.c b/gdbstub/user-target.c
> index 22bf4008c0..4bfcf78aaa 100644
> --- a/gdbstub/user-target.c
> +++ b/gdbstub/user-target.c
> @@ -317,9 +317,9 @@ void gdb_handle_v_file_open(GArray *params, void
> *user_ctx)
>      int fd = open(filename, flags, mode);
>  #endif
>      if (fd < 0) {
> -        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
> +        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
>      } else {
> -        g_string_printf(gdbserver_state.str_buf, "F%d", fd);
> +        g_string_printf(gdbserver_state.str_buf, "F%x", fd);
>      }
>      gdb_put_strbuf();
>  }
> @@ -329,7 +329,7 @@ void gdb_handle_v_file_close(GArray *params, void
> *user_ctx)
>      int fd = gdb_get_cmd_param(params, 0)->val_ul;
>  
>      if (close(fd) == -1) {
> -        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
> +        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
>          gdb_put_strbuf();
>          return;
>      }
> @@ -352,7 +352,7 @@ void gdb_handle_v_file_pread(GArray *params, void
> *user_ctx)
>  
>      ssize_t n = pread(fd, buf, bufsiz, offset);
>      if (n < 0) {
> -        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
> +        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
>          gdb_put_strbuf();
>          return;
>      }
> @@ -375,7 +375,7 @@ void gdb_handle_v_file_readlink(GArray *params,
> void *user_ctx)
>      ssize_t n = readlink(filename, buf, BUFSIZ);
>  #endif
>      if (n < 0) {
> -        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
> +        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
>          gdb_put_strbuf();
>          return;
>      }
Re: [PATCH] gdbstub/user-target: fix gdbserver int format (%d -> %x)
Posted by Alex Bennée 1 year, 1 month ago 
Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com> writes:

> From: disconnect3d <dominik.b.czarnota@gmail.com>

You might want to fix your author attribution here.

>
> This commit fixes an incorrect format string for formatting integers
> provided to GDB when debugging a target run in QEMU user mode.
>
> The correct format is hexadecimal for both success and errno values,
> some of which can be seen here [0].
>
> [0]
> https://github.com/bminor/binutils-gdb/blob/e65a355022d0dc6b5707310876a72b5693ec0aa5/gdbserver/hostio.cc#L196-L213

The spec isn't totally clear on this but it seems the match
remote_hostio_parse_result() as well:

  /* Check for ",errno".  */
  if (*p == ',')
    {
      errno = 0;
      *remote_errno = (fileio_error) strtol (p + 1, &p2, 16);
      if (errno != 0 || p + 1 == p2)
	return -1;
      p = p2;
    }

So: Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


You are also missing a s-o-b: https://qemu.readthedocs.io/en/master/devel/submitting-a-patch.html#id29


> ---
>  gdbstub/user-target.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/gdbstub/user-target.c b/gdbstub/user-target.c
> index 22bf4008c0..4bfcf78aaa 100644
> --- a/gdbstub/user-target.c
> +++ b/gdbstub/user-target.c
> @@ -317,9 +317,9 @@ void gdb_handle_v_file_open(GArray *params, void *user_ctx)
>      int fd = open(filename, flags, mode);
>  #endif
>      if (fd < 0) {
> -        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
> +        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
>      } else {
> -        g_string_printf(gdbserver_state.str_buf, "F%d", fd);
> +        g_string_printf(gdbserver_state.str_buf, "F%x", fd);
>      }
>      gdb_put_strbuf();
>  }
> @@ -329,7 +329,7 @@ void gdb_handle_v_file_close(GArray *params, void *user_ctx)
>      int fd = gdb_get_cmd_param(params, 0)->val_ul;
>  
>      if (close(fd) == -1) {
> -        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
> +        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
>          gdb_put_strbuf();
>          return;
>      }
> @@ -352,7 +352,7 @@ void gdb_handle_v_file_pread(GArray *params, void *user_ctx)
>  
>      ssize_t n = pread(fd, buf, bufsiz, offset);
>      if (n < 0) {
> -        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
> +        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
>          gdb_put_strbuf();
>          return;
>      }
> @@ -375,7 +375,7 @@ void gdb_handle_v_file_readlink(GArray *params, void *user_ctx)
>      ssize_t n = readlink(filename, buf, BUFSIZ);
>  #endif
>      if (n < 0) {
> -        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
> +        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
>          gdb_put_strbuf();
>          return;
>      }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.