Re: [Qemu-devel] [PATCH v3 06/19] linux-user: Split out do_syscall1

[Philippe Mathieu-Daudé]

On 06/11/2018 09:51 PM, Richard Henderson wrote:
> There was supposed to be a single point of return for do_syscall
> so that tracing works properly.  However, there are a few bugs
> in that area.  It is significantly simpler to simply split out
> an inner function to enforce this.
> 
> Reviewed-by: Laurent Vivier <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> ---
>  linux-user/syscall.c | 77 +++++++++++++++++++++++++++-----------------
>  1 file changed, 48 insertions(+), 29 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index c212149245..ec3bc1cbe5 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7947,13 +7947,15 @@ static int host_to_target_cpu_mask(const unsigned 
> long *host_mask,
>      return 0;
>  }
>  
> -/* do_syscall() should always have a single exit point at the end so
> -   that actions, such as logging of syscall results, can be performed.
> -   All errnos that do_syscall() returns must be -TARGET_<errcode>. */
> -abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> -                    abi_long arg2, abi_long arg3, abi_long arg4,
> -                    abi_long arg5, abi_long arg6, abi_long arg7,
> -                    abi_long arg8)
> +/* This is an internal helper for do_syscall so that it is easier
> + * to have a single return point, so that actions, such as logging
> + * of syscall results, can be performed.
> + * All errnos that do_syscall() returns must be -TARGET_<errcode>.
> + */
> +static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
> +                            abi_long arg2, abi_long arg3, abi_long arg4,
> +                            abi_long arg5, abi_long arg6, abi_long arg7,
> +                            abi_long arg8)
>  {
>      CPUState *cpu = ENV_GET_CPU(cpu_env);
>      abi_long ret;
> @@ -7961,25 +7963,6 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>      struct statfs stfs;
>      void *p;
>  
> -#if defined(DEBUG_ERESTARTSYS)
> -    /* Debug-only code for exercising the syscall-restart code paths
> -     * in the per-architecture cpu main loops: restart every syscall
> -     * the guest makes once before letting it through.
> -     */
> -    {
> -        static int flag;
> -
> -        flag = !flag;
> -        if (flag) {
> -            return -TARGET_ERESTARTSYS;
> -        }
> -    }
> -#endif
> -
> -    trace_guest_user_syscall(cpu, num, arg1, arg2, arg3, arg4, arg5, arg6, 
> arg7, arg8);
> -    if(do_strace)
> -        print_syscall(num, arg1, arg2, arg3, arg4, arg5, arg6);
> -
>      switch(num) {
>      case TARGET_NR_exit:
>          /* In old applications this may be used to implement _exit(2).
> @@ -12765,11 +12748,47 @@ abi_long do_syscall(void *cpu_env, int num, 
> abi_long arg1,
>          break;
>      }
>  fail:
> -    if(do_strace)
> -        print_syscall_ret(num, ret);
> -    trace_guest_user_syscall_ret(cpu, num, ret);
>      return ret;
>  efault:
>      ret = -TARGET_EFAULT;
>      goto fail;
>  }
> +
> +abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> +                    abi_long arg2, abi_long arg3, abi_long arg4,
> +                    abi_long arg5, abi_long arg6, abi_long arg7,
> +                    abi_long arg8)
> +{
> +    CPUState *cpu = ENV_GET_CPU(cpu_env);
> +    abi_long ret;
> +
> +#ifdef DEBUG_ERESTARTSYS
> +    /* Debug-only code for exercising the syscall-restart code paths
> +     * in the per-architecture cpu main loops: restart every syscall
> +     * the guest makes once before letting it through.
> +     */
> +    {
> +        static bool flag;
> +        flag = !flag;
> +        if (flag) {
> +            return -TARGET_ERESTARTSYS;
> +        }
> +    }
> +#endif
> +
> +    trace_guest_user_syscall(cpu, num, arg1, arg2, arg3, arg4,
> +                             arg5, arg6, arg7, arg8);
> +
> +    if (unlikely(do_strace)) {
> +        print_syscall(num, arg1, arg2, arg3, arg4, arg5, arg6);
> +        ret = do_syscall1(cpu_env, num, arg1, arg2, arg3, arg4,
> +                          arg5, arg6, arg7, arg8);
> +        print_syscall_ret(num, ret);
> +    } else {
> +        ret = do_syscall1(cpu_env, num, arg1, arg2, arg3, arg4,
> +                          arg5, arg6, arg7, arg8);
> +    }
> +
> +    trace_guest_user_syscall_ret(cpu, num, ret);
> +    return ret;
> +}
>