1. Patchew
  2. QEMU
  3. View series

[PATCH v2] linux-user: Fix strace output for s390x mmap()

Ilya Leoshkevich posted 1 patch 2 days, 14 hours ago 
linux-user/strace.c       | 2 +-
linux-user/syscall.c      | 5 +----
linux-user/syscall_defs.h | 7 +++++++
3 files changed, 9 insertions(+), 5 deletions(-)
[PATCH v2] linux-user: Fix strace output for s390x mmap()
Posted by Ilya Leoshkevich 2 days, 14 hours ago 
print_mmap() assumes that mmap() receives arguments via memory if
mmap2() is present. s390x (as opposed to s390) does not fit this
pattern: it does not have mmap2(), but mmap() still receives arguments
via memory.

Fix by sharing the detection logic between syscall.c and strace.c.

Cc: qemu-stable@nongnu.org
Fixes: d971040c2d16 ("linux-user: Fix strace output for old_mmap")
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---

v1: https://lore.kernel.org/qemu-devel/20241119211138.148806-1-iii@linux.ibm.com/
v1 -> v2: Share the detection logic between syscall.c and strace.c
          (Richard).

 linux-user/strace.c       | 2 +-
 linux-user/syscall.c      | 5 +----
 linux-user/syscall_defs.h | 7 +++++++
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index b70eadc19ef..9c55f39b095 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -3971,7 +3971,7 @@ print_mmap(CPUArchState *cpu_env, const struct syscallname *name,
 {
     return print_mmap_both(cpu_env, name, arg0, arg1, arg2, arg3,
                            arg4, arg5,
-#if defined(TARGET_NR_mmap2)
+#ifdef TARGET_ARCH_WANT_SYS_OLD_MMAP
                             true
 #else
                             false
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 0279f235768..1ce4c79784f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10588,10 +10588,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
         return ret;
 #ifdef TARGET_NR_mmap
     case TARGET_NR_mmap:
-#if (defined(TARGET_I386) && defined(TARGET_ABI32)) || \
-    (defined(TARGET_ARM) && defined(TARGET_ABI32)) || \
-    defined(TARGET_M68K) || defined(TARGET_MICROBLAZE) \
-    || defined(TARGET_S390X)
+#ifdef TARGET_ARCH_WANT_SYS_OLD_MMAP
         {
             abi_ulong *v;
             abi_ulong v1, v2, v3, v4, v5, v6;
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 0e08dfae3e4..faad9147c91 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2766,4 +2766,11 @@ struct target_open_how_ver0 {
 #define RESOLVE_NO_SYMLINKS     0x04
 #endif
 
+#if (defined(TARGET_I386) && defined(TARGET_ABI32)) || \
+    (defined(TARGET_ARM) && defined(TARGET_ABI32)) || \
+    defined(TARGET_M68K) || defined(TARGET_MICROBLAZE) || \
+    defined(TARGET_S390X)
+#define TARGET_ARCH_WANT_SYS_OLD_MMAP
+#endif
+
 #endif
-- 
2.47.0
Re: [PATCH v2] linux-user: Fix strace output for s390x mmap()
Posted by Richard Henderson 15 hours ago 
On 11/20/24 15:26, Ilya Leoshkevich wrote:
> print_mmap() assumes that mmap() receives arguments via memory if
> mmap2() is present. s390x (as opposed to s390) does not fit this
> pattern: it does not have mmap2(), but mmap() still receives arguments
> via memory.
> 
> Fix by sharing the detection logic between syscall.c and strace.c.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: d971040c2d16 ("linux-user: Fix strace output for old_mmap")
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> 
> v1: https://lore.kernel.org/qemu-devel/20241119211138.148806-1-iii@linux.ibm.com/
> v1 -> v2: Share the detection logic between syscall.c and strace.c
>            (Richard).

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

and queued.


r~
Re: [PATCH v2] linux-user: Fix strace output for s390x mmap()
Posted by Philippe Mathieu-Daudé 2 days, 3 hours ago 
On 20/11/24 22:26, Ilya Leoshkevich wrote:
> print_mmap() assumes that mmap() receives arguments via memory if
> mmap2() is present. s390x (as opposed to s390) does not fit this
> pattern: it does not have mmap2(), but mmap() still receives arguments
> via memory.
> 
> Fix by sharing the detection logic between syscall.c and strace.c.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: d971040c2d16 ("linux-user: Fix strace output for old_mmap")
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> 
> v1: https://lore.kernel.org/qemu-devel/20241119211138.148806-1-iii@linux.ibm.com/
> v1 -> v2: Share the detection logic between syscall.c and strace.c
>            (Richard).
> 
>   linux-user/strace.c       | 2 +-
>   linux-user/syscall.c      | 5 +----
>   linux-user/syscall_defs.h | 7 +++++++
>   3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index b70eadc19ef..9c55f39b095 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -3971,7 +3971,7 @@ print_mmap(CPUArchState *cpu_env, const struct syscallname *name,
>   {
>       return print_mmap_both(cpu_env, name, arg0, arg1, arg2, arg3,
>                              arg4, arg5,
> -#if defined(TARGET_NR_mmap2)
> +#ifdef TARGET_ARCH_WANT_SYS_OLD_MMAP

We still want to print for mmap2, so:

   #if defined(TARGET_NR_mmap2) || defined(TARGET_ARCH_WANT_SYS_OLD_MMAP)

>                               true
>   #else
>                               false
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 0279f235768..1ce4c79784f 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -10588,10 +10588,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
>           return ret;
>   #ifdef TARGET_NR_mmap
>       case TARGET_NR_mmap:
> -#if (defined(TARGET_I386) && defined(TARGET_ABI32)) || \
> -    (defined(TARGET_ARM) && defined(TARGET_ABI32)) || \
> -    defined(TARGET_M68K) || defined(TARGET_MICROBLAZE) \
> -    || defined(TARGET_S390X)
> +#ifdef TARGET_ARCH_WANT_SYS_OLD_MMAP
>           {
>               abi_ulong *v;
>               abi_ulong v1, v2, v3, v4, v5, v6;
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 0e08dfae3e4..faad9147c91 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -2766,4 +2766,11 @@ struct target_open_how_ver0 {
>   #define RESOLVE_NO_SYMLINKS     0x04
>   #endif
>   
> +#if (defined(TARGET_I386) && defined(TARGET_ABI32)) || \
> +    (defined(TARGET_ARM) && defined(TARGET_ABI32)) || \
> +    defined(TARGET_M68K) || defined(TARGET_MICROBLAZE) || \
> +    defined(TARGET_S390X)
> +#define TARGET_ARCH_WANT_SYS_OLD_MMAP
> +#endif
> +
>   #endif
Re: [PATCH v2] linux-user: Fix strace output for s390x mmap()
Posted by iii 2 days, 2 hours ago 
On 2024-11-21 10:00, Philippe Mathieu-Daudé wrote:
> On 20/11/24 22:26, Ilya Leoshkevich wrote:
>> print_mmap() assumes that mmap() receives arguments via memory if
>> mmap2() is present. s390x (as opposed to s390) does not fit this
>> pattern: it does not have mmap2(), but mmap() still receives arguments
>> via memory.
>> 
>> Fix by sharing the detection logic between syscall.c and strace.c.
>> 
>> Cc: qemu-stable@nongnu.org
>> Fixes: d971040c2d16 ("linux-user: Fix strace output for old_mmap")
>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>> ---
>> 
>> v1: 
>> https://lore.kernel.org/qemu-devel/20241119211138.148806-1-iii@linux.ibm.com/
>> v1 -> v2: Share the detection logic between syscall.c and strace.c
>>            (Richard).
>> 
>>   linux-user/strace.c       | 2 +-
>>   linux-user/syscall.c      | 5 +----
>>   linux-user/syscall_defs.h | 7 +++++++
>>   3 files changed, 9 insertions(+), 5 deletions(-)
>> 
>> diff --git a/linux-user/strace.c b/linux-user/strace.c
>> index b70eadc19ef..9c55f39b095 100644
>> --- a/linux-user/strace.c
>> +++ b/linux-user/strace.c
>> @@ -3971,7 +3971,7 @@ print_mmap(CPUArchState *cpu_env, const struct 
>> syscallname *name,
>>   {
>>       return print_mmap_both(cpu_env, name, arg0, arg1, arg2, arg3,
>>                              arg4, arg5,
>> -#if defined(TARGET_NR_mmap2)
>> +#ifdef TARGET_ARCH_WANT_SYS_OLD_MMAP
> 
> We still want to print for mmap2, so:
> 
>   #if defined(TARGET_NR_mmap2) || 
> defined(TARGET_ARCH_WANT_SYS_OLD_MMAP)
> 
>>                               true
>>   #else
>>                               false

mmap2() has its own flow from what I can see:

print_mmap2()
   print_mmap_both(..., is_old_mmap=false)

It should not call print_mmap(), which I'm changing here.

[...]
Re: [PATCH v2] linux-user: Fix strace output for s390x mmap()
Posted by Philippe Mathieu-Daudé 2 days, 2 hours ago 
On 21/11/24 10:26, iii wrote:
> On 2024-11-21 10:00, Philippe Mathieu-Daudé wrote:
>> On 20/11/24 22:26, Ilya Leoshkevich wrote:
>>> print_mmap() assumes that mmap() receives arguments via memory if
>>> mmap2() is present. s390x (as opposed to s390) does not fit this
>>> pattern: it does not have mmap2(), but mmap() still receives arguments
>>> via memory.
>>>
>>> Fix by sharing the detection logic between syscall.c and strace.c.
>>>
>>> Cc: qemu-stable@nongnu.org
>>> Fixes: d971040c2d16 ("linux-user: Fix strace output for old_mmap")
>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>>> ---
>>>
>>> v1: 
>>> https://lore.kernel.org/qemu-devel/20241119211138.148806-1-iii@linux.ibm.com/
>>> v1 -> v2: Share the detection logic between syscall.c and strace.c
>>>            (Richard).
>>>
>>>   linux-user/strace.c       | 2 +-
>>>   linux-user/syscall.c      | 5 +----
>>>   linux-user/syscall_defs.h | 7 +++++++
>>>   3 files changed, 9 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/linux-user/strace.c b/linux-user/strace.c
>>> index b70eadc19ef..9c55f39b095 100644
>>> --- a/linux-user/strace.c
>>> +++ b/linux-user/strace.c
>>> @@ -3971,7 +3971,7 @@ print_mmap(CPUArchState *cpu_env, const struct 
>>> syscallname *name,
>>>   {
>>>       return print_mmap_both(cpu_env, name, arg0, arg1, arg2, arg3,
>>>                              arg4, arg5,
>>> -#if defined(TARGET_NR_mmap2)
>>> +#ifdef TARGET_ARCH_WANT_SYS_OLD_MMAP
>>
>> We still want to print for mmap2, so:
>>
>>   #if defined(TARGET_NR_mmap2) || defined(TARGET_ARCH_WANT_SYS_OLD_MMAP)
>>
>>>                               true
>>>   #else
>>>                               false
> 
> mmap2() has its own flow from what I can see:
> 
> print_mmap2()
>    print_mmap_both(..., is_old_mmap=false)
> 
> It should not call print_mmap(), which I'm changing here.
> 
> [...]

If so, better to clean that in a previous patch.

Cc'ing Helge since I'm a bit confused by commit d971040c2d intent
("linux-user: Fix strace output for old_mmap").
Re: [PATCH v2] linux-user: Fix strace output for s390x mmap()
Posted by iii 2 days, 2 hours ago 
On 2024-11-21 11:07, Philippe Mathieu-Daudé wrote:
> On 21/11/24 10:26, iii wrote:
>> On 2024-11-21 10:00, Philippe Mathieu-Daudé wrote:
>>> On 20/11/24 22:26, Ilya Leoshkevich wrote:
>>>> print_mmap() assumes that mmap() receives arguments via memory if
>>>> mmap2() is present. s390x (as opposed to s390) does not fit this
>>>> pattern: it does not have mmap2(), but mmap() still receives 
>>>> arguments
>>>> via memory.
>>>> 
>>>> Fix by sharing the detection logic between syscall.c and strace.c.
>>>> 
>>>> Cc: qemu-stable@nongnu.org
>>>> Fixes: d971040c2d16 ("linux-user: Fix strace output for old_mmap")
>>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>>>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>>>> ---
>>>> 
>>>> v1: 
>>>> https://lore.kernel.org/qemu-devel/20241119211138.148806-1-iii@linux.ibm.com/
>>>> v1 -> v2: Share the detection logic between syscall.c and strace.c
>>>>            (Richard).
>>>> 
>>>>   linux-user/strace.c       | 2 +-
>>>>   linux-user/syscall.c      | 5 +----
>>>>   linux-user/syscall_defs.h | 7 +++++++
>>>>   3 files changed, 9 insertions(+), 5 deletions(-)
>>>> 
>>>> diff --git a/linux-user/strace.c b/linux-user/strace.c
>>>> index b70eadc19ef..9c55f39b095 100644
>>>> --- a/linux-user/strace.c
>>>> +++ b/linux-user/strace.c
>>>> @@ -3971,7 +3971,7 @@ print_mmap(CPUArchState *cpu_env, const struct 
>>>> syscallname *name,
>>>>   {
>>>>       return print_mmap_both(cpu_env, name, arg0, arg1, arg2, arg3,
>>>>                              arg4, arg5,
>>>> -#if defined(TARGET_NR_mmap2)
>>>> +#ifdef TARGET_ARCH_WANT_SYS_OLD_MMAP
>>> 
>>> We still want to print for mmap2, so:
>>> 
>>>   #if defined(TARGET_NR_mmap2) || 
>>> defined(TARGET_ARCH_WANT_SYS_OLD_MMAP)
>>> 
>>>>                               true
>>>>   #else
>>>>                               false
>> 
>> mmap2() has its own flow from what I can see:
>> 
>> print_mmap2()
>>    print_mmap_both(..., is_old_mmap=false)
>> 
>> It should not call print_mmap(), which I'm changing here.
>> 
>> [...]
> 
> If so, better to clean that in a previous patch.

Sorry, I'm confused. What previous patch are you referring to?

> Cc'ing Helge since I'm a bit confused by commit d971040c2d intent
> ("linux-user: Fix strace output for old_mmap").

My understanding of the Helge's idea is that "defined(TARGET_NR_mmap2)" 
is
true if, and only if, mmap() takes arguments via memory.

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.