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

[Ilya Leoshkevich]

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;
>      }