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 <laurent@vivier.eu>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> 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;
> +}
>