From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x333.google.com (mail-ot1-x333.google.com [IPv6:2607:f8b0:4864:20::333]) by sourceware.org (Postfix) with ESMTPS id A588A3857C6F for ; Sat, 22 May 2021 01:53:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A588A3857C6F Received: by mail-ot1-x333.google.com with SMTP id v19-20020a0568301413b0290304f00e3d88so19739399otp.4 for ; Fri, 21 May 2021 18:53:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rPDWxpCR3/MTy/2XyF9gcPdv+65h6Ma057Ccbt5/anc=; b=cvCJi2MMPokG/076W1fFiD2vcRUCFMnAiTcSVA7E6glLrCwxty3t5PU60vlnujJvvE hCWR8gVx2wbTeE/R4x+SIggoHFd1mUumbCIZEu67SNHoHkGf1037IpUCyoEeWZOt7qNT i2iEBGrOkdgtrhPU2yIAnD1f510YRb0vmAr8LW6wbIxYSJcUvRmpdLYnzyIGFx1tutTe peE7qEdZ4hp2Mb2faHeqmJRbdKaOnEqzjywN1eP+onx1lGhJD+8nKilZB/9/lZsb2w9i zTcnUvjijVrGqmeejrLrKvn7IiGoJkQhxijXRFY3hA/LdMvWTZDa07HDh+Xnc83eVr9V GQGA== X-Gm-Message-State: AOAM530sA9d9Z2xN8wlfi4lzexSgR6vUyMD5a/7kQpck/aC7cgsGxLsI FkCwachd6XX87wyUH4qT5BnvIntA2AGGwTw2eEc= X-Google-Smtp-Source: ABdhPJzx/FlPly6E/qLcgRZmycms/Y7M4P3Yl44CUh+khzIh2hN0nmchOsAJfPW4V8IjCng9i6sqiLbj1hRZQAYE/aY= X-Received: by 2002:a9d:66d4:: with SMTP id t20mr10974370otm.125.1621648392110; Fri, 21 May 2021 18:53:12 -0700 (PDT) MIME-Version: 1.0 References: <20210515123442.1432385-1-hjl.tools@gmail.com> <20210515123442.1432385-5-hjl.tools@gmail.com> In-Reply-To: From: "H.J. Lu" Date: Fri, 21 May 2021 18:52:36 -0700 Message-ID: Subject: Re: [PATCH v5 4/5] x86-64: Add the clone3 wrapper To: Noah Goldstein Cc: GNU C Library , Florian Weimer Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3033.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 22 May 2021 01:53:15 -0000 On Thu, May 20, 2021 at 11:39 AM Noah Goldstein wrote: > > On Thu, May 20, 2021 at 2:35 PM Noah Goldstein wrote: > > > > On Sat, May 15, 2021 at 9:23 AM H.J. Lu via Libc-alpha > > wrote: > > > > > > extern int clone3 (struct clone_args *__cl_args, > > > int (*__func) (void *__arg), void *__arg); > > > --- > > > sysdeps/unix/sysv/linux/x86_64/clone3.S | 92 +++++++++++++++++++++++++ > > > sysdeps/unix/sysv/linux/x86_64/sysdep.h | 2 + > > > 2 files changed, 94 insertions(+) > > > create mode 100644 sysdeps/unix/sysv/linux/x86_64/clone3.S > > > > > > diff --git a/sysdeps/unix/sysv/linux/x86_64/clone3.S b/sysdeps/unix/sysv/linux/x86_64/clone3.S > > > new file mode 100644 > > > index 0000000000..f7d4036a6a > > > --- /dev/null > > > +++ b/sysdeps/unix/sysv/linux/x86_64/clone3.S > > > @@ -0,0 +1,92 @@ > > > +/* The clone3 syscall wrapper. Linux/x86-64 version. > > > + Copyright (C) 2021 Free Software Foundation, Inc. > > > + This file is part of the GNU C Library. > > > + > > > + The GNU C Library is free software; you can redistribute it and/or > > > + modify it under the terms of the GNU Lesser General Public > > > + License as published by the Free Software Foundation; either > > > + version 2.1 of the License, or (at your option) any later version. > > > + > > > + The GNU C Library is distributed in the hope that it will be useful, > > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > > + Lesser General Public License for more details. > > > + > > > + You should have received a copy of the GNU Lesser General Public > > > + License along with the GNU C Library; if not, see > > > + . */ > > > + > > > +/* clone3() is even more special than fork() as it mucks with stacks > > > + and invokes a function in the right context after its all over. */ > > > + > > > +#include > > > +#include > > > + > > > +/* The userland implementation is: > > > + int clone3 (struct clone_args *cl_args, int (*func)(void *arg), > > > + void *arg); > > > + the kernel entry is: > > > + int clone3 (struct clone_args *cl_args, size_t size); > > > + > > > + The parameters are passed in registers from userland: > > > + rdi: cl_args > > > + rsi: func > > > + rdx: arg > > > + > > > + The kernel expects: > > > + rax: system call number > > > + rdi: cl_args > > > + rsi: size */ > > > + > > > + .text > > > +ENTRY (__clone3) > > > + /* Sanity check arguments. */ > > > + movq $-EINVAL, %rax > > > > Can this be movl? Yes. Fixed. > > > + testq %rdi, %rdi /* No NULL cl_args pointer. */ > > > + jz SYSCALL_ERROR_LABEL > > > + testq %rsi, %rsi /* No NULL function pointer. */ > > > + jz SYSCALL_ERROR_LABEL > > > + > > > + /* Save the function pointer in R8 which is preserved by the > > > + syscall. */ > > > + movq %rsi, %r8 > > > + > > > + /* Put sizeof (struct clone_args) in ESI. */ > > > + movl $CLONE_ARGS_SIZE , %esi > > > + > > > + /* Do the system call. */ > > > + movl $SYS_ify(clone3), %eax > > > + > > > + /* End FDE now, because in the child the unwind info will be > > > + wrong. */ > > > + cfi_endproc > > > + syscall > > > + > > > + test %RAX_LP, %RAX_LP > > > + jl SYSCALL_ERROR_LABEL > > > + jz L(thread_start) > > > + > > > > Is expectation to go to L(thread_start)? If so > > think jnz L(ret) and fallthrough is probably > > better. > > Or better take the error check branch off > the critical path with jnz L(error_or_ret) then jl > in L(error_or_ret) I don't think the clone wrapper is on the critical path. Since the same code is executed by both child and parent. I check the error return first. > > > > > + ret > > > + > > > +L(thread_start): > > > + cfi_startproc > > > + /* Clearing frame pointer is insufficient, use CFI. */ > > > + cfi_undefined (rip) > > > + /* Clear the frame pointer. The ABI suggests this be done, to mark > > > + the outermost frame obviously. */ > > > + xorl %ebp, %ebp > > > + > > > + /* Set up arguments for the function call. */ > > > + movq %rdx, %rdi /* Argument. */ > > > + call *%r8 /* Call function. */ > > > + /* Call exit with return value from function call. */ > > > + movq %rax, %rdi > > > + movl $SYS_ify(exit), %eax > > > + syscall > > > + cfi_endproc > > > + > > > + cfi_startproc > > > +PSEUDO_END (__clone3) > > > + > > > +libc_hidden_def (__clone3) > > > +weak_alias (__clone3, clone3) > > > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > > index dbad2c788a..f26ffc68ae 100644 > > > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > > @@ -377,6 +377,8 @@ > > > # define HAVE_GETCPU_VSYSCALL "__vdso_getcpu" > > > # define HAVE_CLOCK_GETRES64_VSYSCALL "__vdso_clock_getres" > > > > > > +# define HAVE_CLONE3_WAPPER 1 > > > + > > > # define SINGLE_THREAD_BY_GLOBAL 1 > > > > > > #endif /* __ASSEMBLER__ */ > > > -- > > > 2.31.1 > > > Thanks. -- H.J.