[PATCH] linux-user: Improve strace output of personality() and sysinfo()

Helge Deller posted 1 patch 3 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20221223100107.17303-1-deller@gmx.de
Maintainers: Laurent Vivier <laurent@vivier.eu>
There is a newer version of this series
linux-user/strace.list | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] linux-user: Improve strace output of personality() and sysinfo()
Posted by Helge Deller 3 years, 1 month ago
Make the strace look nicer for those two syscalls.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 linux-user/strace.list | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/strace.list b/linux-user/strace.list
index f9254725a1..909298099e 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1043,7 +1043,7 @@
 { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_personality
-{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
+{ TARGET_NR_personality, "personality" , "%s(%p)", NULL, print_syscall_ret_addr },
 #endif
 #ifdef TARGET_NR_pipe
 { TARGET_NR_pipe, "pipe" , NULL, NULL, NULL },
@@ -1502,7 +1502,7 @@
 { TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_sysinfo
-{ TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL },
+{ TARGET_NR_sysinfo, "sysinfo" , "%s(%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_sys_kexec_load
 { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL },
--
2.38.1
Re: [PATCH] linux-user: Improve strace output of personality() and sysinfo()
Posted by Philippe Mathieu-Daudé 3 years, 1 month ago
On 23/12/22 11:01, Helge Deller wrote:
> Make the strace look nicer for those two syscalls.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> ---
>   linux-user/strace.list | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index f9254725a1..909298099e 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -1043,7 +1043,7 @@
>   { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_personality
> -{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
> +{ TARGET_NR_personality, "personality" , "%s(%p)", NULL, print_syscall_ret_addr },

Shouldn't this be:

    { TARGET_NR_personality, "personality" , "%s(%u)", NULL, NULL },

?

>   #endif
>   #ifdef TARGET_NR_pipe
>   { TARGET_NR_pipe, "pipe" , NULL, NULL, NULL },
> @@ -1502,7 +1502,7 @@
>   { TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_sysinfo
> -{ TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL },
> +{ TARGET_NR_sysinfo, "sysinfo" , "%s(%p)", NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_sys_kexec_load
>   { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL },
> --
> 2.38.1
> 
>
Re: [PATCH] linux-user: Improve strace output of personality() and sysinfo()
Posted by Helge Deller 3 years, 1 month ago
On 12/23/22 11:50, Philippe Mathieu-Daudé wrote:
> On 23/12/22 11:01, Helge Deller wrote:
>> Make the strace look nicer for those two syscalls.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> ---
>>   linux-user/strace.list | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/linux-user/strace.list b/linux-user/strace.list
>> index f9254725a1..909298099e 100644
>> --- a/linux-user/strace.list
>> +++ b/linux-user/strace.list
>> @@ -1043,7 +1043,7 @@
>>   { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
>>   #endif
>>   #ifdef TARGET_NR_personality
>> -{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
>> +{ TARGET_NR_personality, "personality" , "%s(%p)", NULL, print_syscall_ret_addr },
>
> Shouldn't this be:
>
>     { TARGET_NR_personality, "personality" , "%s(%u)", NULL, NULL },

Basically yes, but...
it's a bitmap, so printing it as hex value (similiar to a pointer)
is easier to read/analyze.

Helge

>
> ?
>
>>   #endif
>>   #ifdef TARGET_NR_pipe
>>   { TARGET_NR_pipe, "pipe" , NULL, NULL, NULL },
>> @@ -1502,7 +1502,7 @@
>>   { TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL },
>>   #endif
>>   #ifdef TARGET_NR_sysinfo
>> -{ TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL },
>> +{ TARGET_NR_sysinfo, "sysinfo" , "%s(%p)", NULL, NULL },
>>   #endif
>>   #ifdef TARGET_NR_sys_kexec_load
>>   { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL },
>> --
>> 2.38.1
>>
>>
>
Re: [PATCH] linux-user: Improve strace output of personality() and sysinfo()
Posted by Philippe Mathieu-Daudé 3 years, 1 month ago
On 23/12/22 11:53, Helge Deller wrote:
> On 12/23/22 11:50, Philippe Mathieu-Daudé wrote:
>> On 23/12/22 11:01, Helge Deller wrote:
>>> Make the strace look nicer for those two syscalls.
>>>
>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>> ---
>>>   linux-user/strace.list | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/linux-user/strace.list b/linux-user/strace.list
>>> index f9254725a1..909298099e 100644
>>> --- a/linux-user/strace.list
>>> +++ b/linux-user/strace.list
>>> @@ -1043,7 +1043,7 @@
>>>   { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
>>>   #endif
>>>   #ifdef TARGET_NR_personality
>>> -{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
>>> +{ TARGET_NR_personality, "personality" , "%s(%p)", NULL, 
>>> print_syscall_ret_addr },
>>
>> Shouldn't this be:
>>
>>     { TARGET_NR_personality, "personality" , "%s(%u)", NULL, NULL },
> 
> Basically yes, but...
> it's a bitmap, so printing it as hex value (similiar to a pointer)
> is easier to read/analyze.

Oh, good point. Then "%s(0x"TARGET_ABI_FMT_lx")" is self-explicit.

Also for clarity:

#define print_syscall_ret_persona print_syscall_ret_addr

So what do you think of:

{ TARGET_NR_personality, "personality" , "%s(0x"TARGET_ABI_FMT_lx")",
    NULL, print_syscall_ret_persona },

?

Re: [PATCH] linux-user: Improve strace output of personality() and sysinfo()
Posted by Helge Deller 3 years, 1 month ago
On 12/23/22 12:01, Philippe Mathieu-Daudé wrote:
> On 23/12/22 11:53, Helge Deller wrote:
>> On 12/23/22 11:50, Philippe Mathieu-Daudé wrote:
>>> On 23/12/22 11:01, Helge Deller wrote:
>>>> Make the strace look nicer for those two syscalls.
>>>>
>>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>> ---
>>>>   linux-user/strace.list | 4 ++--
>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/linux-user/strace.list b/linux-user/strace.list
>>>> index f9254725a1..909298099e 100644
>>>> --- a/linux-user/strace.list
>>>> +++ b/linux-user/strace.list
>>>> @@ -1043,7 +1043,7 @@
>>>>   { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
>>>>   #endif
>>>>   #ifdef TARGET_NR_personality
>>>> -{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
>>>> +{ TARGET_NR_personality, "personality" , "%s(%p)", NULL, print_syscall_ret_addr },
>>>
>>> Shouldn't this be:
>>>
>>>     { TARGET_NR_personality, "personality" , "%s(%u)", NULL, NULL },
>>
>> Basically yes, but...
>> it's a bitmap, so printing it as hex value (similiar to a pointer)
>> is easier to read/analyze.
>
> Oh, good point. Then "%s(0x"TARGET_ABI_FMT_lx")" is self-explicit.

Hmm ... I don't see that as any benefit for the user and the output is the same.

> Also for clarity:
>
> #define print_syscall_ret_persona print_syscall_ret_addr
>
> So what do you think of:
>
> { TARGET_NR_personality, "personality" , "%s(0x"TARGET_ABI_FMT_lx")",
>     NULL, print_syscall_ret_persona },

This change does make sense if someone fully implements the
strace of personality() including showing the flags as string.
Until then it's IMHO just one more function which uses space
and gains nothing.

Helge
Re: [PATCH] linux-user: Improve strace output of personality() and sysinfo()
Posted by Laurent Vivier 3 years ago
Le 23/12/2022 à 13:27, Helge Deller a écrit :
> On 12/23/22 12:01, Philippe Mathieu-Daudé wrote:
>> On 23/12/22 11:53, Helge Deller wrote:
>>> On 12/23/22 11:50, Philippe Mathieu-Daudé wrote:
>>>> On 23/12/22 11:01, Helge Deller wrote:
>>>>> Make the strace look nicer for those two syscalls.
>>>>>
>>>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>>> ---
>>>>>   linux-user/strace.list | 4 ++--
>>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/linux-user/strace.list b/linux-user/strace.list
>>>>> index f9254725a1..909298099e 100644
>>>>> --- a/linux-user/strace.list
>>>>> +++ b/linux-user/strace.list
>>>>> @@ -1043,7 +1043,7 @@
>>>>>   { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
>>>>>   #endif
>>>>>   #ifdef TARGET_NR_personality
>>>>> -{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
>>>>> +{ TARGET_NR_personality, "personality" , "%s(%p)", NULL, print_syscall_ret_addr },
>>>>
>>>> Shouldn't this be:
>>>>
>>>>     { TARGET_NR_personality, "personality" , "%s(%u)", NULL, NULL },
>>>
>>> Basically yes, but...
>>> it's a bitmap, so printing it as hex value (similiar to a pointer)
>>> is easier to read/analyze.
>>
>> Oh, good point. Then "%s(0x"TARGET_ABI_FMT_lx")" is self-explicit.
> 
> Hmm ... I don't see that as any benefit for the user and the output is the same.
> 

I agree with Philippe for this part, it's not a pointer, don't use %p.

Thanks,
Laurent




[PATCH v2] linux-user: Improve strace output of personality() and sysinfo()
Posted by Helge Deller 3 years ago
Make the strace look nicer for those two syscalls.

Signed-off-by: Helge Deller <deller@gmx.de>

--
v2: use TARGET_ABI_FMT_lx instead of %p in personality output
    as suggested by Philippe Mathieu-Daudé and Laurent Vivier


diff --git a/linux-user/strace.list b/linux-user/strace.list
index f9254725a1..703c0f1608 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1043,7 +1043,8 @@
 { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_personality
-{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
+{ TARGET_NR_personality, "personality" , "%s(0x"TARGET_ABI_FMT_lx")", NULL,
+  print_syscall_ret_addr },
 #endif
 #ifdef TARGET_NR_pipe
 { TARGET_NR_pipe, "pipe" , NULL, NULL, NULL },
@@ -1502,7 +1503,7 @@
 { TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_sysinfo
-{ TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL },
+{ TARGET_NR_sysinfo, "sysinfo" , "%s(%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_sys_kexec_load
 { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL },
Re: [PATCH v2] linux-user: Improve strace output of personality() and sysinfo()
Posted by Laurent Vivier 3 years ago
Le 27/01/2023 à 21:18, Helge Deller a écrit :
> Make the strace look nicer for those two syscalls.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> --
> v2: use TARGET_ABI_FMT_lx instead of %p in personality output
>      as suggested by Philippe Mathieu-Daudé and Laurent Vivier
> 
> 
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index f9254725a1..703c0f1608 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -1043,7 +1043,8 @@
>   { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_personality
> -{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
> +{ TARGET_NR_personality, "personality" , "%s(0x"TARGET_ABI_FMT_lx")", NULL,
> +  print_syscall_ret_addr },
>   #endif
>   #ifdef TARGET_NR_pipe
>   { TARGET_NR_pipe, "pipe" , NULL, NULL, NULL },
> @@ -1502,7 +1503,7 @@
>   { TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_sysinfo
> -{ TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL },
> +{ TARGET_NR_sysinfo, "sysinfo" , "%s(%p)", NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_sys_kexec_load
>   { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL },
> 

Applied to my linux-user-for-8.0 branch.

Thanks,
Laurent


Re: [PATCH v2] linux-user: Improve strace output of personality() and sysinfo()
Posted by Laurent Vivier 3 years ago
Le 27/01/2023 à 21:18, Helge Deller a écrit :
> Make the strace look nicer for those two syscalls.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> --
> v2: use TARGET_ABI_FMT_lx instead of %p in personality output
>      as suggested by Philippe Mathieu-Daudé and Laurent Vivier
> 
> 
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index f9254725a1..703c0f1608 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -1043,7 +1043,8 @@
>   { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_personality
> -{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
> +{ TARGET_NR_personality, "personality" , "%s(0x"TARGET_ABI_FMT_lx")", NULL,
> +  print_syscall_ret_addr },
>   #endif
>   #ifdef TARGET_NR_pipe
>   { TARGET_NR_pipe, "pipe" , NULL, NULL, NULL },
> @@ -1502,7 +1503,7 @@
>   { TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_sysinfo
> -{ TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL },
> +{ TARGET_NR_sysinfo, "sysinfo" , "%s(%p)", NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_sys_kexec_load
>   { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL },
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>


Re: [PATCH v2] linux-user: Improve strace output of personality() and sysinfo()
Posted by Richard Henderson 3 years ago
On 1/27/23 10:18, Helge Deller wrote:
> Make the strace look nicer for those two syscalls.
> 
> Signed-off-by: Helge Deller<deller@gmx.de>

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


r~