From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x22e.google.com (mail-oi1-x22e.google.com [IPv6:2607:f8b0:4864:20::22e]) by sourceware.org (Postfix) with ESMTPS id A70933858C2D for ; Thu, 3 Nov 2022 21:59:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A70933858C2D 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-oi1-x22e.google.com with SMTP id r83so3484215oih.2 for ; Thu, 03 Nov 2022 14:59:05 -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=uPzGXCczBhTnsQpqMmDacK6aZ3zj++Gd4XAOBzbvgDQ=; b=R0+qEcRmhpaMFBK5UiIlmzAqXg+SSIlMeqOmM3hkAerSWHhUQZtCdEguaiQuSp2K70 NfuHtVG5Zv4MEmCizQZ1nWKTgVRsSfc2fgsjZtu6coR9/jyGxVGng4XEj5lAR+3Ztrb/ rDWrBPWxtcWdj2sCjbINGeNkICCVYgEWAqqUaX5pQ8g6z+SsHUd8M1S+G6r71wtJLc9X IuFEz/F3z5NRQCiJgS7MsMaLe/c2T6lgGxLqrQcDYYt8b8UdTKdjVi0OZE+ijH2s/i4Z 8uwQQmoRsKJsFvzw31tnjxFhXnx42pkvqzUpbm4JNJ0wv3yGM+JJYRW3C8KbLTFHqroq yfMg== 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=uPzGXCczBhTnsQpqMmDacK6aZ3zj++Gd4XAOBzbvgDQ=; b=V9a0DwJrsxZgQ/CELm4gIC05pukas66ThQcZ8NdtKKv3hHp/L4x/4993LoeHcwUDRS NgS7tJDu9WhoNvkij4VGxiMTG8NMPtGRLiSKwYSRMJYIeL6tEDDQoWvI43GrbueLQz/m OVOShu7eJWnfmZUir88A+OSo+TTKSXLy9vIg7XzqqmGX66aeyilb4ACDuyJwWo2POK4G C7ui/rmUVNClNzNm4lAqyh1VLjVQlKSQhM+7UF3FfbN8tHUsvm5uUBWTkZQXzwCoh2vN zRFPd2FMgP8ME+fBYPRO2A/G3NT4KUOnX6TfAUYHVU3igfNADqOX6BFuTVebm1CiURLd IG6Q== X-Gm-Message-State: ACrzQf3bKOkuJYzwWj8eyDB97B02+yV52Gl+z1n6MbnQ5FrHoCb6g6Tn CyY+0qfI0P/sHaee9lGxMhD3QR++NYJbvQ6w2lQ= X-Google-Smtp-Source: AMsMyM6c47tsuhwKI81L5XCPx77lefYjeVd68ecp8Vy/hTGl9wpi1STrqRHUWMA7eF+qFetFV4e1+B0X1uhQXqtm4S4= X-Received: by 2002:aca:674a:0:b0:35a:856:4b85 with SMTP id b10-20020aca674a000000b0035a08564b85mr14577686oiy.298.1667512744889; Thu, 03 Nov 2022 14:59:04 -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> <1a5cc9ec-de78-cb4d-3bd3-7f37dc666f73@linaro.org> In-Reply-To: <1a5cc9ec-de78-cb4d-3bd3-7f37dc666f73@linaro.org> From: "H.J. Lu" Date: Thu, 3 Nov 2022 14:58:28 -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=-3017.8 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 2:22 PM Adhemerval Zanella Netto wrote: > > > > On 03/11/22 13:55, 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). > > All the internal usage of __clone_internal are done with all signal masked, > so aligning the stack is currently safe. However, I still think moving out > the stack alignment of __clone3 is still a net gain: it remove an > implementation detail (block/unblock signals) and simplifies the arch-specific > code. > > However it makes a possible libc wrap clunky, the caller will need to know > the ABI stack alignment prior to the call since kernel does not automatically > align the stack. For the internal clone3, we can drop stack alignment adjustment. The internal users are responsible for correct stack alignment. If there is a public clone3 wrapper, it should adjust stack alignment. -- H.J.