From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x31.google.com (mail-oa1-x31.google.com [IPv6:2001:4860:4864:20::31]) by sourceware.org (Postfix) with ESMTPS id 880683858C60 for ; Thu, 3 Nov 2022 20:56:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 880683858C60 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-oa1-x31.google.com with SMTP id 586e51a60fabf-13bd19c3b68so3590983fac.7 for ; Thu, 03 Nov 2022 13:56:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=hc3kCFMcSmXg6jvedIicpFeFHChu8MuP6gF0WrraQ0o=; b=nPnBlNAIIzMQ/oRW2doi3JUehMwF6a7ro/kl0x7ggtvdSMRYhTQ5kEXzpeZrGvjEnX Aqk3UVpPDQsKWH7XUMsMbot6PHFGUubQue2cu5nG4gIM1fhpWmycROFtR7d7uGoYH1yE TrSSZa+n2rnfbrCA/8awtZ0RwD6/RLW6dOXSbqFIbp+fKKMxMjyKY2HkCO645iamU1vQ 8EumxuvG6yGl5DYINcpVhnuZXdZKc9Ewkv+iADW/wCrLMTR/XV0xyIvFWCS562ArsZQD HmJ0h0BBOzZRRrKrRm9E5a9eRLiE1XIHutTL0KFwyz8+i+Wy8Le/dSWogjGQyGtwc4sM ipLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=hc3kCFMcSmXg6jvedIicpFeFHChu8MuP6gF0WrraQ0o=; b=G3y9Sq2Z34oCTdlFrFJ7ACVeA6ssf8k2+E8dXmMDSzoc/zebpzuh+fGaVghSCig4SN QvXXk52HooYJCOmIljk6UEZxVD4ytb0opVDjb2Y1F79L2a0i/hOPyElrR5YNKvjWmcSZ 01lBBFV4QFwbJzJNioeAQndmSf+eABXAc/LNl/GRKKbXFYH9SHLr4LqrYtJgpLKahT7l z5iaiysBovk/ug2P1BADjD9GYn4Ngm79ZQMw8hR382GWkMFF8rsLJvVBixJ7KFs9n1BX ZlF2janjzKUHQ6iS5xRKClMAw2CfXSuMfnfzqo2x9+mPRqkFvBRLao7JKCEJYUMOVfqF MQyQ== X-Gm-Message-State: ACrzQf3M8UUlD1BEv9QiIy6MgZIxkGMX+byoUEDpMPmyAWCJc2tXGceY cEfLw53V7NkmfLTRXvpz/dfG5GOPLs56GYPwHVQ= X-Google-Smtp-Source: AMsMyM5oXeR8uDN9yYx6IRxRywsiF2tEzXNhjU5W6jByBIuSPVIXBCwrtN21VcKgzBMrZcYbFx2e2nVYiBDu8YylXnU= X-Received: by 2002:a05:6870:f5a4:b0:136:3e0d:acdd with SMTP id eh36-20020a056870f5a400b001363e0dacddmr20579948oab.298.1667508992957; Thu, 03 Nov 2022 13:56:32 -0700 (PDT) MIME-Version: 1.0 References: <20220930192613.3491147-1-adhemerval.zanella@linaro.org> <20220930192613.3491147-5-adhemerval.zanella@linaro.org> <1d4ce210-2b28-b061-9780-f643eaa80a27@linaro.org> <8a3dc5d9-b731-4c45-7252-8157ba0be6c2@linaro.org> In-Reply-To: From: "H.J. Lu" Date: Thu, 3 Nov 2022 13:55:56 -0700 Message-ID: Subject: Re: [PATCH v2 4/9] aarch64: Add the clone3 wrapper To: Adhemerval Zanella Netto Cc: Szabolcs Nagy , libc-alpha@sourceware.org, Christian Brauner Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3016.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, Nov 3, 2022 at 9:55 AM Adhemerval Zanella Netto wrote: > > > > On 03/11/22 13:52, Szabolcs Nagy wrote: > > The 11/03/2022 13:39, Adhemerval Zanella Netto wrote: > >> > >> > >> On 03/11/22 13:31, Szabolcs Nagy wrote: > >>> The 11/03/2022 13:22, Adhemerval Zanella Netto wrote: > >>>> > >>>> > >>>> On 03/11/22 11:01, Szabolcs Nagy wrote: > >>>>> The 11/03/2022 10:15, Adhemerval Zanella Netto wrote: > >>>>>> On 02/11/22 09:12, Szabolcs Nagy wrote: > >>>>>>> The 09/30/2022 16:26, Adhemerval Zanella via Libc-alpha wrote: > >>>>>>>> It follow the internal signature: > >>>>>>>> > >>>>>>>> extern int clone3 (struct clone_args *__cl_args, size_t __size, > >>>>>>>> int (*__func) (void *__arg), void *__arg); > >>>>>>>> > >>>>>>>> And x86_64 semantics to return EINVAL if either cl_args or func > >>>>>>>> is NULL. The stack is 16-byte aligned prior executing func. > >>>>>>> > >>>>>>> "x86_64 semantics" sounds wrong: maybe this should be documented? > >>>>>>> i'd expect 0 cl_args/func to be UB like in most posix apis. > >>>>>> > >>>>>> Right, I think it is worth to document the function semantic > >>>>>> properly at least on its internal header (include/clone_internal.h). > >>>>>> H.J also added a new clone3.h headers, which is not currently installed > >>>>>> that I am inclined to just remove it from now. We might reinstate > >>>>>> if/when we decide to provide the clone3 as an ABI. > >>>>>> > >>>>>> And returning EINVAL for 0 cl_args/func aligns with our exported clone > >>>>>> interface, where EINVAL is also returned for 0 function argument. > >>>>> > >>>>> ok. > >>>>> > >>>>>>> > >>>>>>> and aligning sp in the child fails if signals are allowed there > >>>>>>> (pthreads does not allow signals now, direct callers might). > >>>>>>> i dont know if that's a concert (or if unaligned stack is > >>>>>>> something we should fix up in clone3). > >>>>>> > >>>>>> It was overlooked on initial x86_64 clone3 implementation as well. I > >>>>>> think it better to just return EINVAL for unaligned stacks and avoid > >>>>>> to change the stack pointer in the created thread. > >>>>> > >>>>> long time ago linux did that on aarch64, but it was removed: > >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e6d9a52543338603e25e71e0e4942f05dae0dd8a > >>>>> > >>>>> i think in clone3 the kernel should have aligned (it knows > >>>>> the bounds now), doing it in the userspace wrapper is weird > >>>>> (should we adjust the stack size?). and not doing it at all > >>>>> makes clone3 hard to use portably (user has to know target > >>>>> specific pcs requirements). > >>>>> > >>>>> not sure what's the best way forward. > >>>> > >>>> I think the stack size won't matter much here, at least not from > >>>> kernel point of view (the resulting stack size will most likely > >>>> be page aligned anyway). But I think this kernel commit makes a good > >>>> point that silently adjusting the stack in userland is not the > >>>> correct approach, I think H.J has done to make it consistent with > >>>> glibc clone implementation which does it. > >>>> > >>>> IMHO the best approach would to just remove the stack alignment, > >>>> since it incurs the signal handling issue. > >>> > >>> current generic clone callers dont align the stack and > >>> e.g. unaligned pthread custom stack should work. > >>> > >>> so we have to do arch specific stack alignment somewhere, > >>> maybe in pthread_create? > >> > >> I am thinking on __clone_internal, where if an unaligned stack is > >> used it creates a new clone_args struct with adjust arguments. It > >> can adjust the struct in place (not sure which is better). > > > > if the api is not exposed, then i think the arg can be modified > > in place. (if clone3 api is exposed to users then we should not > > modify user structs unless the clone3 api contract explicitly > > allows this.) > > > > either aligning in pthread_create or __clone_internal works for me, > > the target specific clone3 syscall should not in case that gets > > exposed to users. > > > > The arg modification would be done only internally by __clone_internal, > if we ever export __clone3 it will not mess with stack alignment (my > idea is to remove it from x86_64 as well). Is there a bug for the signal handling issue? -- H.J.