Re: [PATCH 5/5] linux-user: Add strace support for printing arguments of fallocate()

[Laurent Vivier <laurent@vivier.eu>]

Le 02/06/2020 à 13:53, Filip Bozuta a écrit :
> From: Filip Bozuta <Filip.Bozuta@syrmia.com>
> 
> This patch implements strace argument printing functionality for following syscall:
> 
>     *fallocate - manipulate file space
> 
>         int fallocate(int fd, int mode, off_t offset, off_t len)
>         man page: https://www.man7.org/linux/man-pages/man2/fallocate.2.html
> 
> Implementation notes:
> 
>     This syscall's second argument "mode" is composed of predefined values
>     which represent flags that determine the type of operation that is
>     to be performed on the file space. For that reason, a printing
>     function "print_fallocate" was stated in file "strace.list". This printing
>     function uses an already existing function "print_flags()" to print flags of
>     the "mode" argument. These flags are stated inside an array "falloc_flags"
>     that contains values of type "struct flags". These values are instantiated
>     using an existing macro "FLAG_GENERIC()". Most of these flags are defined
>     after kernel version 3.0 which is why they are enwrapped in an #ifdef
>     directive.
>     The syscall's third ant fourth argument are of type "off_t" which can
>     cause variations between 32/64-bit architectures. To handle this variation,
>     function "target_offset64()" was copied from file "strace.c" and used in
>     "print_fallocate" to print "off_t" arguments for 32-bit architectures.
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/strace.c    | 56 ++++++++++++++++++++++++++++++++++++++++++
>  linux-user/strace.list |  2 +-
>  2 files changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 542bfdeb91..3998a00bb4 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -34,6 +34,22 @@ struct syscallname {
>  #define UNUSED
>  #endif
>  
> +#if TARGET_ABI_BITS == 32
> +static inline uint64_t target_offset64(uint32_t word0, uint32_t word1)
> +{
> +#ifdef TARGET_WORDS_BIGENDIAN
> +    return ((uint64_t)word0 << 32) | word1;
> +#else
> +    return ((uint64_t)word1 << 32) | word0;
> +#endif
> +}
> +#else /* TARGET_ABI_BITS == 32 */
> +static inline uint64_t target_offset64(uint64_t word0, uint64_t word1)
> +{
> +    return word0;
> +}
> +#endif /* TARGET_ABI_BITS != 32 */
> +

Rather than copying that from syscall.c perhaps you can move the
definition from linux-user/syscall.c to linux-user/qemu.h?

>  /*
>   * Structure used to translate flag values into strings.  This is
>   * similar that is in the actual strace tool.
> @@ -1097,6 +1113,26 @@ UNUSED static struct flags statx_mask[] = {
>      FLAG_END,
>  };
>  
> +UNUSED static struct flags falloc_flags[] = {
> +    FLAG_GENERIC(FALLOC_FL_KEEP_SIZE),
> +    FLAG_GENERIC(FALLOC_FL_PUNCH_HOLE),
> +#ifdef FALLOC_FL_NO_HIDE_STALE
> +    FLAG_GENERIC(FALLOC_FL_NO_HIDE_STALE),
> +#endif
> +#ifdef FALLOC_FL_COLLAPSE_RANGE
> +    FLAG_GENERIC(FALLOC_FL_COLLAPSE_RANGE),
> +#endif
> +#ifdef FALLOC_FL_ZERO_RANGE
> +    FLAG_GENERIC(FALLOC_FL_ZERO_RANGE),
> +#endif
> +#ifdef FALLOC_FL_INSERT_RANGE
> +    FLAG_GENERIC(FALLOC_FL_INSERT_RANGE),
> +#endif
> +#ifdef FALLOC_FL_UNSHARE_RANGE
> +    FLAG_GENERIC(FALLOC_FL_UNSHARE_RANGE),
> +#endif
> +};
> +
>  /*
>   * print_xxx utility functions.  These are used to print syscall
>   * parameters in certain format.  All of these have parameter
> @@ -1514,6 +1550,26 @@ print_faccessat(const struct syscallname *name,
>  }
>  #endif
>  
> +#ifdef TARGET_NR_fallocate
> +static void
> +print_fallocate(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_flags(falloc_flags, arg1, 0);
> +#if TARGET_ABI_BITS == 32
> +    print_raw_param("%ld", target_offset64(arg2, arg3), 0);
> +    print_raw_param("%ld", target_offset64(arg4, arg5), 1);

target_offset64 returns uint64_t so use PRIu64 rather than "%ld"

> +#else
> +    print_raw_param("%ld", arg2, 0);
> +    print_raw_param("%ld", arg3, 1);

TARGET_ABI_FMT_ld rather than "%ld"

> +#endif
> +    print_syscall_epilogue(name);
> +}
> +#endif
> +
>  #ifdef TARGET_NR_fchmodat
>  static void
>  print_fchmodat(const struct syscallname *name,
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index b72f757d3f..d7458ce884 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -182,7 +182,7 @@
>  { TARGET_NR_fadvise64_64, "fadvise64_64" , NULL, NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_fallocate
> -{ TARGET_NR_fallocate, "fallocate" , NULL, NULL, NULL },
> +{ TARGET_NR_fallocate, "fallocate" , NULL, print_fallocate, NULL },
>  #endif
>  #ifdef TARGET_NR_fanotify_init
>  { TARGET_NR_fanotify_init, "fanotify_init" , NULL, NULL, NULL },
> 

Thanks,
Laurent