target/riscv: A collection of bug fixes

[PATCH 2/5] linux-user/strace: Add a wrapper for read and write

Posted by alistair23@gmail.com 4 days, 14 hours ago

From: Alistair Francis <alistair.francis@wdc.com>

The stack pointer is being truncated as 32-bits for qemu-riscv64, so
let's add a print_read_write helper that ensures all of the bits are
printed.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3238
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 linux-user/strace.c    | 14 ++++++++++++++
 linux-user/strace.list |  4 ++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 2cbaf94c89..023173857b 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -4309,6 +4309,20 @@ print_tgkill(CPUArchState *cpu_env, const struct syscallname *name,
 }
 #endif
 
+#if defined(TARGET_NR_read) || defined(TARGET_NR_write)
+static void
+print_read_write(CPUArchState *cpu_env, const struct syscallname *name,
+        abi_long arg0, abi_long arg1, abi_long arg2,
+        abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_raw_param("%d", arg0, 0);
+    print_pointer(arg1, 0);
+    print_raw_param("%d", arg2, 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
 #if defined(TARGET_NR_pread64) || defined(TARGET_NR_pwrite64)
 static void
 print_pread64(CPUArchState *cpu_env, const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index 35f001fecd..239b9e18b4 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1114,7 +1114,7 @@
 { TARGET_NR_quotactl, "quotactl" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_read
-{ TARGET_NR_read, "read" , "%s(%d,%#x,%d)", NULL, NULL },
+{ TARGET_NR_read, "read" , NULL, print_read_write, NULL },
 #endif
 #ifdef TARGET_NR_readahead
 { TARGET_NR_readahead, "readahead" , NULL, NULL, NULL },
@@ -1674,7 +1674,7 @@
                      print_syscall_ret_waitpid },
 #endif
 #ifdef TARGET_NR_write
-{ TARGET_NR_write, "write" , "%s(%d,%#x,%d)", NULL, NULL },
+{ TARGET_NR_write, "write" , NULL, print_read_write, NULL },
 #endif
 #ifdef TARGET_NR_writev
 { TARGET_NR_writev, "writev" , "%s(%d,%p,%#x)", NULL, NULL },
-- 
2.53.0

Re: [PATCH 2/5] linux-user/strace: Add a wrapper for read and write

Posted by Richard Henderson 3 days, 16 hours ago

On 4/7/26 14:36, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> The stack pointer is being truncated as 32-bits for qemu-riscv64, so
> let's add a print_read_write helper that ensures all of the bits are
> printed.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3238
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   linux-user/strace.c    | 14 ++++++++++++++
>   linux-user/strace.list |  4 ++--
>   2 files changed, 16 insertions(+), 2 deletions(-)

This is indicative of incorrect fowarding of values to system fprintf.
We really should be handling the format ourselves so abi_long etc are handled properly.

In particular,

> +print_read_write(CPUArchState *cpu_env, const struct syscallname *name,
> +        abi_long arg0, abi_long arg1, abi_long arg2,
> +        abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_raw_param("%d", arg0, 0);
> +    print_pointer(arg1, 0);
> +    print_raw_param("%d", arg2, 1);
> +    print_syscall_epilogue(name);
> +}
> +#endif

This is more work than we should require for such simple syscalls.

>   #ifdef TARGET_NR_read
> -{ TARGET_NR_read, "read" , "%s(%d,%#x,%d)", NULL, NULL },
> +{ TARGET_NR_read, "read" , NULL, print_read_write, NULL },

Ideally, I think %p is the correct format to use for a syscall pointer argument. 
Interpreted, of course, in the context of abi_ptr.


r~

Re: [PATCH 2/5] linux-user/strace: Add a wrapper for read and write

Posted by Chao Liu 4 days, 1 hour ago

On Tue, Apr 07, 2026 at 02:36:11PM +1000, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> The stack pointer is being truncated as 32-bits for qemu-riscv64, so
> let's add a print_read_write helper that ensures all of the bits are
> printed.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3238
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  linux-user/strace.c    | 14 ++++++++++++++
>  linux-user/strace.list |  4 ++--
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 2cbaf94c89..023173857b 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -4309,6 +4309,20 @@ print_tgkill(CPUArchState *cpu_env, const struct syscallname *name,
>  }
>  #endif
>  
> +#if defined(TARGET_NR_read) || defined(TARGET_NR_write)
> +static void
> +print_read_write(CPUArchState *cpu_env, const struct syscallname *name,
> +        abi_long arg0, abi_long arg1, abi_long arg2,
> +        abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_raw_param("%d", arg0, 0);
> +    print_pointer(arg1, 0);
> +    print_raw_param("%d", arg2, 1);
If TARGET_NR_read and TARGET_NR_write support 64-bit targets,
abi_long is int64_t, so %d will be truncated. We can use
TARGET_ABI_FMT_ld to replace it.

Otherwise LGTM.

Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>

Thanks,
Chao
> +    print_syscall_epilogue(name);
> +}
> +#endif
> +
>  #if defined(TARGET_NR_pread64) || defined(TARGET_NR_pwrite64)
>  static void
>  print_pread64(CPUArchState *cpu_env, const struct syscallname *name,
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index 35f001fecd..239b9e18b4 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -1114,7 +1114,7 @@
>  { TARGET_NR_quotactl, "quotactl" , NULL, NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_read
> -{ TARGET_NR_read, "read" , "%s(%d,%#x,%d)", NULL, NULL },
> +{ TARGET_NR_read, "read" , NULL, print_read_write, NULL },
>  #endif
>  #ifdef TARGET_NR_readahead
>  { TARGET_NR_readahead, "readahead" , NULL, NULL, NULL },
> @@ -1674,7 +1674,7 @@
>                       print_syscall_ret_waitpid },
>  #endif
>  #ifdef TARGET_NR_write
> -{ TARGET_NR_write, "write" , "%s(%d,%#x,%d)", NULL, NULL },
> +{ TARGET_NR_write, "write" , NULL, print_read_write, NULL },
>  #endif
>  #ifdef TARGET_NR_writev
>  { TARGET_NR_writev, "writev" , "%s(%d,%p,%#x)", NULL, NULL },
> -- 
> 2.53.0
>

Re: [PATCH 2/5] linux-user/strace: Add a wrapper for read and write

Posted by Philippe Mathieu-Daudé 3 days, 10 hours ago

On 7/4/26 18:55, Chao Liu wrote:
> On Tue, Apr 07, 2026 at 02:36:11PM +1000, alistair23@gmail.com wrote:
>> From: Alistair Francis <alistair.francis@wdc.com>
>>
>> The stack pointer is being truncated as 32-bits for qemu-riscv64, so
>> let's add a print_read_write helper that ensures all of the bits are
>> printed.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3238
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>   linux-user/strace.c    | 14 ++++++++++++++
>>   linux-user/strace.list |  4 ++--
>>   2 files changed, 16 insertions(+), 2 deletions(-)


>> +#if defined(TARGET_NR_read) || defined(TARGET_NR_write)
>> +static void
>> +print_read_write(CPUArchState *cpu_env, const struct syscallname *name,
>> +        abi_long arg0, abi_long arg1, abi_long arg2,
>> +        abi_long arg3, abi_long arg4, abi_long arg5)
>> +{
>> +    print_syscall_prologue(name);
>> +    print_raw_param("%d", arg0, 0);
>> +    print_pointer(arg1, 0);
>> +    print_raw_param("%d", arg2, 1);
> If TARGET_NR_read and TARGET_NR_write support 64-bit targets,
> abi_long is int64_t, so %d will be truncated. We can use
> TARGET_ABI_FMT_ld to replace it.

With the 2 "%d" replacements to TARGET_ABI_FMT_ld:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> 
> Otherwise LGTM.
> 
> Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
> 
> Thanks,
> Chao
>> +    print_syscall_epilogue(name);
>> +}
>> +#endif

Re: [PATCH 2/5] linux-user/strace: Add a wrapper for read and write

Posted by Daniel Henrique Barboza 4 days, 8 hours ago


On 4/7/2026 1:36 AM, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> The stack pointer is being truncated as 32-bits for qemu-riscv64, so
> let's add a print_read_write helper that ensures all of the bits are
> printed.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3238
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---


Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>


>   linux-user/strace.c    | 14 ++++++++++++++
>   linux-user/strace.list |  4 ++--
>   2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 2cbaf94c89..023173857b 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -4309,6 +4309,20 @@ print_tgkill(CPUArchState *cpu_env, const struct syscallname *name,
>   }
>   #endif
>   
> +#if defined(TARGET_NR_read) || defined(TARGET_NR_write)
> +static void
> +print_read_write(CPUArchState *cpu_env, const struct syscallname *name,
> +        abi_long arg0, abi_long arg1, abi_long arg2,
> +        abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_raw_param("%d", arg0, 0);
> +    print_pointer(arg1, 0);
> +    print_raw_param("%d", arg2, 1);
> +    print_syscall_epilogue(name);
> +}
> +#endif
> +
>   #if defined(TARGET_NR_pread64) || defined(TARGET_NR_pwrite64)
>   static void
>   print_pread64(CPUArchState *cpu_env, const struct syscallname *name,
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index 35f001fecd..239b9e18b4 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -1114,7 +1114,7 @@
>   { TARGET_NR_quotactl, "quotactl" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_read
> -{ TARGET_NR_read, "read" , "%s(%d,%#x,%d)", NULL, NULL },
> +{ TARGET_NR_read, "read" , NULL, print_read_write, NULL },
>   #endif
>   #ifdef TARGET_NR_readahead
>   { TARGET_NR_readahead, "readahead" , NULL, NULL, NULL },
> @@ -1674,7 +1674,7 @@
>                        print_syscall_ret_waitpid },
>   #endif
>   #ifdef TARGET_NR_write
> -{ TARGET_NR_write, "write" , "%s(%d,%#x,%d)", NULL, NULL },
> +{ TARGET_NR_write, "write" , NULL, print_read_write, NULL },
>   #endif
>   #ifdef TARGET_NR_writev
>   { TARGET_NR_writev, "writev" , "%s(%d,%p,%#x)", NULL, NULL },