Re: [PATCH v3 02/12] linux-user: Add missing clock_gettime64() syscall strace

[Laurent Vivier <laurent@vivier.eu>]

Le 25/09/2022 à 17:27, Helge Deller a écrit :
> On 9/25/22 17:09, Laurent Vivier wrote:
>> Le 18/09/2022 à 21:45, Helge Deller a écrit :
>>> Allow linux-user to strace the clock_gettime64() syscall.
>>> This syscall is used a lot on 32-bit guest architectures which use newer
>>> glibc versions.
>>>
>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>> ---
>>>   linux-user/strace.c    | 53 ++++++++++++++++++++++++++++++++++++++++++
>>>   linux-user/strace.list |  4 ++++
>>>   2 files changed, 57 insertions(+)
>>>
>>> diff --git a/linux-user/strace.c b/linux-user/strace.c
>>> index a4eeef7ae1..816e679995 100644
>>> --- a/linux-user/strace.c
>>> +++ b/linux-user/strace.c
>>> @@ -82,6 +82,7 @@ UNUSED static void print_buf(abi_long addr, abi_long len, int last);
>>>   UNUSED static void print_raw_param(const char *, abi_long, int);
>>>   UNUSED static void print_timeval(abi_ulong, int);
>>>   UNUSED static void print_timespec(abi_ulong, int);
>>> +UNUSED static void print_timespec64(abi_ulong, int);
>>>   UNUSED static void print_timezone(abi_ulong, int);
>>>   UNUSED static void print_itimerval(abi_ulong, int);
>>>   UNUSED static void print_number(abi_long, int);
>>> @@ -795,6 +796,24 @@ print_syscall_ret_clock_gettime(CPUArchState *cpu_env, const struct syscallname
>>>   #define print_syscall_ret_clock_getres     print_syscall_ret_clock_gettime
>>>   #endif
>>>
>>> +#if defined(TARGET_NR_clock_gettime64)
>>> +static void
>>> +print_syscall_ret_clock_gettime64(CPUArchState *cpu_env, const struct syscallname *name,
>>> +                                abi_long ret, abi_long arg0, abi_long arg1,
>>> +                                abi_long arg2, abi_long arg3, abi_long arg4,
>>> +                                abi_long arg5)
>>> +{
>>> +    if (!print_syscall_err(ret)) {
>>> +        qemu_log(TARGET_ABI_FMT_ld, ret);
>>> +        qemu_log(" (");
>>> +        print_timespec64(arg1, 1);
>>> +        qemu_log(")");
>>> +    }
>>> +
>>> +    qemu_log("\n");
>>> +}
>>> +#endif
>>> +
>>>   #ifdef TARGET_NR_gettimeofday
>>>   static void
>>>   print_syscall_ret_gettimeofday(CPUArchState *cpu_env, const struct syscallname *name,
>>> @@ -1652,6 +1671,27 @@ print_timespec(abi_ulong ts_addr, int last)
>>>       }
>>>   }
>>>
>>> +static void
>>> +print_timespec64(abi_ulong ts_addr, int last)
>>> +{
>>> +    if (ts_addr) {
>>> +        struct target__kernel_timespec *ts;
>>> +
>>> +        ts = lock_user(VERIFY_READ, ts_addr, sizeof(*ts), 1);
>>> +        if (!ts) {
>>> +            print_pointer(ts_addr, last);
>>> +            return;
>>> +        }
>>> +        qemu_log("{tv_sec = %lld"
>>> +                 ",tv_nsec = %lld}%s",
>>> +                 (long long)tswap64(ts->tv_sec), (long long)tswap64(ts->tv_nsec),
>>> +                 get_comma(last));
>>> +        unlock_user(ts, ts_addr, 0);
>>> +    } else {
>>> +        qemu_log("NULL%s", get_comma(last));
>>> +    }
>>> +}
>>> +
>>>   static void
>>>   print_timezone(abi_ulong tz_addr, int last)
>>>   {
>>> @@ -2267,6 +2307,19 @@ print_clock_gettime(CPUArchState *cpu_env, const struct syscallname *name,
>>>   #define print_clock_getres     print_clock_gettime
>>>   #endif
>>>
>>> +#if defined(TARGET_NR_clock_gettime64)
>>> +static void
>>> +print_clock_gettime64(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_enums(clockids, arg0, 0);
>>> +    print_pointer(arg1, 1);
>>> +    print_syscall_epilogue(name);
>>> +}
>>> +#endif
>>
>> I think it could be simply:
>>
>> #define print_clock_gettime64 print_clock_gettime where print_clock_gettime() is defined.
> 
> Unfortunately not, because one uses print_timespec() while the other uses print_timespec64().
> 

The syscall_ret part cannot be shared, but the prefix function can, they are identical.

Thanks,
Laurent