Re: [PATCH] linux-user: Fix pipe() vs. pipe2() usage for ALPHA, MIPS, SH4 and SPARC

[Peter Maydell]

On Sun, 17 Jul 2022 at 17:10, Helge Deller <deller@gmx.de> wrote:
>
> In 2010, the commit b41a66edd0c added a thrird parameter "is_pipe2" to the

typo in commit hash (lost the first letter). Should be
fb41a66edd0c7bd6 ("alpha-linux-user: Fix pipe return mechanism."
I think ?

> internal do_pipe() function, but missed to actually use this parameter to
> decide if the pipe() or pipe2() syscall should be used.
> Instead it just continued to check the flags parameter and used pipe2()
> unconditionally if flags is non-zero.
>
> This change should make a difference for the ALPHA, MIPS, SH4 and SPARC
> targets if the emulated code calls pipe2() with a flags value of 0.
>
> Fixes: fb41a66edd0c ("alpha-linux-user: Fix pipe return mechanism.")
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 991b85e6b4..1e6e814871 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1600,7 +1600,7 @@ static abi_long do_pipe(CPUArchState *cpu_env, 
> abi_ulong pipedes,
>  {
>      int host_pipe[2];
>      abi_long ret;
> -    ret = flags ? do_pipe2(host_pipe, flags) : pipe(host_pipe);
> +    ret = is_pipe2 ? do_pipe2(host_pipe, flags) : pipe(host_pipe);

Why do we need to do this? If the flags argument is 0,
then pipe2() is the same as pipe(), so we can safely
emulate it with the host pipe() call. It is, or at
least was, worth doing that, so that guests that use
pipe2(fds, 0) can still run on hosts that don't implement
the pipe2 syscall.

There's probably a reasonable argument to be made that we don't
care any more about hosts so old they don't have pipe2 and that
we could just dump do_pipe2() and the CONFIG_PIPE2 check, and
have do_pipe() unconditionally call pipe2(). Would just need
somebody to check what kernel/glibc versions pipe2() came in.

The architecture specific bit of target behaviour that
we need the is_pipe2 flag for is purely for handling the
weird return code that only the pipe syscall has, and
for that we are correctly looking at the is_pipe2 argument.
(The bug that fb41a66edd0c7bd6 fixes is that we used to
incorrectly give the pipe syscall return argument behaviour
for pipe2-with-flags-zero.)

thanks
-- PMM