[1/2] tools/nolibc: add pipe() support

Message ID d01fc60c6a85cc4af87f6f88eb5017b83c181f4d.1690307717.git.tanyuan@tinylab.org
State New
Headers
Series tools/nolibc: add pipe() and its testcase |

Commit Message

Yuan Tan July 25, 2023, 6:01 p.m. UTC
  pipe is crucial for shell.

Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
---
 tools/include/nolibc/sys.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
  

Comments

Zhangjin Wu July 28, 2023, 3:50 p.m. UTC | #1
Hi, Yuan

> pipe is crucial for shell.
>

As the syscall manpage[1] shows, pipe() is just one of the exceptions how
may require two return registers in some platforms, e.g.:
arch/mips/kernel/syscall.c

    * For historic reasons the pipe(2) syscall on MIPS has an unusual calling
     * convention.  It returns results in registers $v0 / $v1 which means there
     * is no need for it to do verify the validity of a userspace pointer
     * argument.  Historically that used to be expensive in Linux.  These days
     * the performance advantage is negligible.
     */
    asmlinkage int sysm_pipe(void)
    {
            int fd[2];
            int error = do_pipe_flags(fd, 0);
            if (error)
                    return error;
            current_pt_regs()->regs[3] = fd[1];
            return fd[0];
    }

The other exceptions are getxpid(2), getxuid(2), and getxgid(2) on
Alpha.

Since we are able to use pipe2() for pipe() (introduced from early Linux
2.6.27, glibc 2.9) and use getpid+getppid for getxpid, getuid+geteuid
for getxuid and getgid+getegit for getxgid.

So, it is possible provide such pipe() as a wraper of pipe2() and
getxpid, getxuid and getxgid as wrappers of their corresponding syscall
pairs, then, no need to provide a second return value for all of the
existing architectures, for example:

    #ifndef pipe
    int pipe(int pipefd[2])
    {
        pipe2(pipefd, 0);
    }
    #endif

And for mips:

    // may be in tools/include/nolibc/types.h ?
    struct fd_pair {
           long fd[2];
    };

    // tools/include/nolibc/arch-mips.h
    struct fd_pair pipe(void)
    {
            struct fd_pair fds;
            int pipefds[2];
    
            pipe2(pipefds, 0);
    
            fds.fd[0] = pipefds[0];
            fds.fd[1] = pipefds[1];
    
            return fds;
    }

To use such method, the test case should be changed too, perhaps an
easier method is even only provide pipe2() for all and let users define
their own pipe() if really required, we also need to change the test
case.

Best regards,
Zhangjin
[1]: https://man7.org/linux/man-pages/man2/syscall.2.html

> Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
> ---
>  tools/include/nolibc/sys.h | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
> index 8bfe7db20b80..09841fc266fe 100644
> --- a/tools/include/nolibc/sys.h
> +++ b/tools/include/nolibc/sys.h
> @@ -752,6 +752,23 @@ int open(const char *path, int flags, ...)
>  }
>  
>  
> +/*
> + * int pipe(int pipefd[2]);
> + */
> +
> +static __attribute__((unused))
> +int sys_pipe(int pipefd[2])
> +{
> +	return my_syscall1(__NR_pipe, pipefd);
> +}
> +
> +static __attribute__((unused))
> +int pipe(int pipefd[2])
> +{
> +	return __sysret(sys_pipe(pipefd));
> +}
> +
> +
>  /*
>   * int prctl(int option, unsigned long arg2, unsigned long arg3,
>   *                       unsigned long arg4, unsigned long arg5);
> -- 
> 2.39.2
  
Willy Tarreau July 28, 2023, 7:17 p.m. UTC | #2
Hi Zhangjin, hi Yuan,

On Fri, Jul 28, 2023 at 11:50:31PM +0800, Zhangjin Wu wrote:
> Hi, Yuan
> 
> > pipe is crucial for shell.
> >
> 
> As the syscall manpage[1] shows, pipe() is just one of the exceptions how
> may require two return registers in some platforms, e.g.:
> arch/mips/kernel/syscall.c
> 
>     * For historic reasons the pipe(2) syscall on MIPS has an unusual calling
>      * convention.  It returns results in registers $v0 / $v1 which means there
>      * is no need for it to do verify the validity of a userspace pointer
>      * argument.  Historically that used to be expensive in Linux.  These days
>      * the performance advantage is negligible.
>      */
(...)

Ah indeed! I vaguely remembered that I had left that one aside for some
time but did not remember why. Now I remember that I couldn't handle the
MIPS implementation by then while it used to be my primary target.

> Since we are able to use pipe2() for pipe() (introduced from early Linux
> 2.6.27, glibc 2.9) and use getpid+getppid for getxpid, getuid+geteuid
> for getxuid and getgid+getegit for getxgid.
> 
> So, it is possible provide such pipe() as a wraper of pipe2() and

Indeed.

> getxpid, getxuid and getxgid as wrappers of their corresponding syscall
> pairs,

I doubt anyone needs these ones, I didn't know them and do not even
have their man page. Let's keep the focus on what developers really use.

> then, no need to provide a second return value for all of the
> existing architectures, for example:


> 
>     #ifndef pipe
>     int pipe(int pipefd[2])
>     {
>         pipe2(pipefd, 0);
>     }
>     #endif
> 
> And for mips:
> 
>     // may be in tools/include/nolibc/types.h ?
>     struct fd_pair {
>            long fd[2];
>     };
> 
>     // tools/include/nolibc/arch-mips.h
>     struct fd_pair pipe(void)
>     {
>             struct fd_pair fds;
>             int pipefds[2];
>     
>             pipe2(pipefds, 0);
>     
>             fds.fd[0] = pipefds[0];
>             fds.fd[1] = pipefds[1];
>     
>             return fds;
>     }

This one does not have the correct prototype for the function exposed
to the user, pipe really is "int pipe(int pipefd[2])". Maybe you were
thinking about sys_pipe() instead ? But since MIPS also has pipe2() now,
there's no reason to make an exception.

> To use such method, the test case should be changed too, perhaps an
> easier method is even only provide pipe2() for all and let users define
> their own pipe() if really required, we also need to change the test
> case.

No, we need to provide users with what they need to compile standard
code. If we rely on pipe2() to deliver pipe(), that's fine. We can even
do it per-arch if there are constraints but it seems to me that pipe2()
is OK.

Thanks,
Willy
  
Zhangjin Wu July 29, 2023, 8:37 a.m. UTC | #3
> Hi Zhangjin, hi Yuan,
> 
> On Fri, Jul 28, 2023 at 11:50:31PM +0800, Zhangjin Wu wrote:
> > Hi, Yuan
> > 
> > > pipe is crucial for shell.
> > >
> > 
> > As the syscall manpage[1] shows, pipe() is just one of the exceptions how
> > may require two return registers in some platforms, e.g.:
> > arch/mips/kernel/syscall.c
> > 
> >     * For historic reasons the pipe(2) syscall on MIPS has an unusual calling
> >      * convention.  It returns results in registers $v0 / $v1 which means there
> >      * is no need for it to do verify the validity of a userspace pointer
> >      * argument.  Historically that used to be expensive in Linux.  These days
> >      * the performance advantage is negligible.
> >      */
> (...)
> 
> Ah indeed! I vaguely remembered that I had left that one aside for some
> time but did not remember why. Now I remember that I couldn't handle the
> MIPS implementation by then while it used to be my primary target.
>

Seems pipe() is the **only** one some architectures (except Alpha)
return two values, and now we have pipe2(), so, it is ok for us to
simply use pipe2() instead ;-)

> > Since we are able to use pipe2() for pipe() (introduced from early Linux
> > 2.6.27, glibc 2.9) and use getpid+getppid for getxpid, getuid+geteuid
> > for getxuid and getgid+getegit for getxgid.
> > 
> > So, it is possible provide such pipe() as a wraper of pipe2() and
> 
> Indeed.
> 
> > getxpid, getxuid and getxgid as wrappers of their corresponding syscall
> > pairs,
> 
> I doubt anyone needs these ones, I didn't know them and do not even
> have their man page. Let's keep the focus on what developers really use.
>

Yeah.

> > then, no need to provide a second return value for all of the
> > existing architectures, for example:
> 
> 
> > 
> >     #ifndef pipe
> >     int pipe(int pipefd[2])
> >     {
> >         pipe2(pipefd, 0);
> >     }
> >     #endif
> > 
> > And for mips:
> > 
> >     // may be in tools/include/nolibc/types.h ?
> >     struct fd_pair {
> >            long fd[2];
> >     };
> > 
> >     // tools/include/nolibc/arch-mips.h
> >     struct fd_pair pipe(void)
> >     {
[...]
> >     
> >             pipe2(pipefds, 0);
[...]
> >     }
> 
> This one does not have the correct prototype for the function exposed
> to the user, pipe really is "int pipe(int pipefd[2])". Maybe you were
> thinking about sys_pipe() instead ? But since MIPS also has pipe2() now,
> there's no reason to make an exception.
>

Yes, pipe2() should be a better choice, but I have seen this sentence in
syscall manpage [1]:

       /* On Alpha, IA-64, MIPS, SuperH, and SPARC/SPARC64, pipe() has the
          following prototype; see NOTES */

       #include <unistd.h>

       struct fd_pair {
           long fd[2];
       };
       struct fd_pair pipe(void);

If it is about syscall, then we are ok to align all of the architectures
together to use "int pipe(int pipefd[2])", otherwise, it will be
required to define them in their own arch-<ARCH>.h, just like some
defined for arch-s390.h.

[1]: https://www.man7.org/linux/man-pages/man2/pipe.2.html

> > To use such method, the test case should be changed too, perhaps an
> > easier method is even only provide pipe2() for all and let users define
> > their own pipe() if really required, we also need to change the test
> > case.
> 
> No, we need to provide users with what they need to compile standard
> code. If we rely on pipe2() to deliver pipe(), that's fine. We can even
> do it per-arch if there are constraints but it seems to me that pipe2()
> is OK.
>

Ok.

Thanks,
Zhangjin

> Thanks,
> Willy
  
Willy Tarreau July 29, 2023, 10:04 a.m. UTC | #4
On Sat, Jul 29, 2023 at 04:37:00PM +0800, Zhangjin Wu wrote:
> > This one does not have the correct prototype for the function exposed
> > to the user, pipe really is "int pipe(int pipefd[2])". Maybe you were
> > thinking about sys_pipe() instead ? But since MIPS also has pipe2() now,
> > there's no reason to make an exception.
> >
> 
> Yes, pipe2() should be a better choice, but I have seen this sentence in
> syscall manpage [1]:
> 
>        /* On Alpha, IA-64, MIPS, SuperH, and SPARC/SPARC64, pipe() has the
>           following prototype; see NOTES */
> 
>        #include <unistd.h>
> 
>        struct fd_pair {
>            long fd[2];
>        };
>        struct fd_pair pipe(void);
> 
> If it is about syscall, then we are ok to align all of the architectures
> together to use "int pipe(int pipefd[2])"

Yes it's OK, that's how applications expect it to be used:

  https://pubs.opengroup.org/onlinepubs/9699919799/functions/pipe.html

For the archs you mention above, it's the libc that wraps the call,
exactly what we ought to do as well (using pipe2() since it will be
easier).

Willy
  

Patch

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index 8bfe7db20b80..09841fc266fe 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -752,6 +752,23 @@  int open(const char *path, int flags, ...)
 }
 
 
+/*
+ * int pipe(int pipefd[2]);
+ */
+
+static __attribute__((unused))
+int sys_pipe(int pipefd[2])
+{
+	return my_syscall1(__NR_pipe, pipefd);
+}
+
+static __attribute__((unused))
+int pipe(int pipefd[2])
+{
+	return __sysret(sys_pipe(pipefd));
+}
+
+
 /*
  * int prctl(int option, unsigned long arg2, unsigned long arg3,
  *                       unsigned long arg4, unsigned long arg5);