Re: [PATCH v2 4/4] linux-user: Add strace support for printing arguments of some clock and time functions

[Laurent Vivier]

Le 13/07/2020 à 11:50, Filip Bozuta a écrit :
> This patch implements strace argument printing functionality for following 
> syscalls:
> 
>     * clock_getres, clock_gettime, clock_settime - clock and time functions
> 
>         int clock_getres(clockid_t clockid, struct timespec *res)
>         int clock_gettime(clockid_t clockid, struct timespec *tp)
>         int clock_settime(clockid_t clockid, const struct timespec *tp)
>         man page: https://man7.org/linux/man-pages/man2/clock_getres.2.html
> 
>     * gettimeofday - get time
> 
>         int gettimeofday(struct timeval *tv, struct timezone *tz)
>         man page: https://man7.org/linux/man-pages/man2/gettimeofday.2.html
> 
>     * getitimer, setitimer - get or set value of an interval timer
> 
>         int getitimer(int which, struct itimerval *curr_value)
>         int setitimer(int which, const struct itimerval *new_value,
>                       struct itimerval *old_value)
>         man page: https://man7.org/linux/man-pages/man2/getitimer.2.html
> 
> Implementation notes:
> 
>     All of the syscalls have some structue types as argument types and thus
>     a separate printing function was stated in file "strace.list" for each
>     of them. All of these functions use existing functions for their
>     appropriate structure types ("print_timeval()" and "print_timezone()").
>     Functions "print_timespec()" and "print_itimerval()" were added in this
>     patch so that they can be used to print types "struct timespec" and
>     "struct itimerval" used by some of the syscalls. Function 
> "print_itimerval()"
>     uses the existing function "print_timeval()" to print fields of the
>     structure "struct itimerval" that are of type "struct timeval".
>     Function "print_itimer_type()" was added to print the type of the interval
>     typer which is the firt argument of "getitimer()" and "setitimer()".
>     Also, the existing function "print_timeval()" was changed a little so
>     that it prints the field names beside the values. Syscalls 
> "clock_getres()"
>     and "clocK_gettime()" have the same number and types of arguments and
>     thus their print functions "print_clock_getres" and "print_clock_gettime"
>     shate a common definition in file "strace.c".
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/strace.c    | 208 ++++++++++++++++++++++++++++++++++++++++-
>  linux-user/strace.list |  17 ++--
>  2 files changed, 217 insertions(+), 8 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 0d95cc6089..9bdee3b495 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -64,7 +64,10 @@ UNUSED static void print_string(abi_long, int);
>  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_timezone(abi_ulong, int);
> +UNUSED static void print_itimer_type(abi_ulong, int);
> +UNUSED static void print_itimerval(abi_ulong, int);
>  UNUSED static void print_number(abi_long, int);
>  UNUSED static void print_signal(abi_ulong, int);
>  UNUSED static void print_sockaddr(abi_ulong, abi_long, int);
> @@ -829,6 +832,89 @@ print_syscall_ret_adjtimex(void *cpu_env, const struct 
> syscallname *name,
>  }
>  #endif
>  
> +#if defined(TARGET_NR_clock_gettime) || defined(TARGET_NR_clock_getres)
> +static void
> +print_syscall_ret_clock_gettime(void *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)
> +{
> +    print_syscall_err(ret);
> +
> +    if (ret >= 0) {

The series needs to be rebased to use:

    if (!print_syscall_err(ret)) {

> +        qemu_log(TARGET_ABI_FMT_ld, ret);
> +        qemu_log(" (");
> +        print_timespec(arg1, 1);
> +        qemu_log(")");
> +    }
> +
> +    qemu_log("\n");
> +}
> +#define print_syscall_ret_clock_getres     print_syscall_ret_clock_gettime
> +#endif
> +
> +#ifdef TARGET_NR_gettimeofday
> +static void
> +print_syscall_ret_gettimeofday(void *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)
> +{
> +    print_syscall_err(ret);
> +
> +    if (ret >= 0) {

    if (!print_syscall_err(ret)) {

> +        qemu_log(TARGET_ABI_FMT_ld, ret);
> +        qemu_log(" (");
> +        print_timeval(arg0, 0);
> +        print_timezone(arg1, 1);
> +        qemu_log(")");
> +    }
> +
> +    qemu_log("\n");
> +}
> +#endif
> +
> +#ifdef TARGET_NR_getitimer
> +static void
> +print_syscall_ret_getitimer(void *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)
> +{
> +    print_syscall_err(ret);
> +
> +    if (ret >= 0) {

    if (!print_syscall_err(ret)) {

> +        qemu_log(TARGET_ABI_FMT_ld, ret);
> +        qemu_log(" (");
> +        print_itimerval(arg1, 1);
> +        qemu_log(")");
> +    }
> +
> +    qemu_log("\n");
> +}
> +#endif
> +
> +
> +#ifdef TARGET_NR_getitimer
> +static void
> +print_syscall_ret_setitimer(void *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)
> +{
> +    print_syscall_err(ret);
> +
> +    if (ret >= 0) {

    if (!print_syscall_err(ret)) {

> +        qemu_log(TARGET_ABI_FMT_ld, ret);
> +        qemu_log(" (old_value = ");
> +        print_itimerval(arg2, 1);
> +        qemu_log(")");
> +    }
> +
> +    qemu_log("\n");
> +}
> +#endif
> +
...

All the other changes seem ok.

Thanks,
Laurent