From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.2 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 741B21F4B4 for ; Wed, 7 Oct 2020 15:46:13 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9AD783857800; Wed, 7 Oct 2020 15:46:11 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9AD783857800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1602085571; bh=fCcmzLFbHtgLoB6EthgAECrLHPVuDbWfICMvAET8F0c=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=xVz5t4AWB/7iE+lVQ4OUYMs145fUFxZ8CqSjL18+mgQdTjef8yDhHikNxpk5VXzfW UUuAx8TWpYQr2NTfzoE43gaE51i2OmtC5xvrmwMuyGyZxv3dOcRGuphOeejylbvG1n sVI8iaXTMFb8wWpA3Wm6nTCGJZyCcJqVFe8RpWEg= Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 8711A3857C5B for ; Wed, 7 Oct 2020 15:46:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8711A3857C5B Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C43F5106F; Wed, 7 Oct 2020 08:46:04 -0700 (PDT) Received: from arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C53A73F66B; Wed, 7 Oct 2020 08:46:02 -0700 (PDT) Date: Wed, 7 Oct 2020 16:45:59 +0100 To: "H.J. Lu" Subject: Re: [RFC PATCH 0/4] x86: Improve Minimum Alternate Stack Size Message-ID: <20201007154559.GI6642@arm.com> References: <20201005134534.GT6642@arm.com> <20201006092532.GU6642@arm.com> <20201006152553.GY6642@arm.com> <20201006165520.GA6642@arm.com> <20201007104720.GH6642@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) 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: , From: Dave Martin via Libc-alpha Reply-To: Dave Martin Cc: linux-arch , Len Brown , Tony Luck , GNU C Library , "Ravi V. Shankar" , "Chang S. Bae" , the arch/x86 maintainers , LKML , Dave Hansen , Andy Lutomirski , Linux API , Thomas Gleixner , Borislav Petkov , Ingo Molnar Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On Wed, Oct 07, 2020 at 06:30:03AM -0700, H.J. Lu wrote: > On Wed, Oct 7, 2020 at 3:47 AM Dave Martin wrote: > > > > On Tue, Oct 06, 2020 at 10:44:14AM -0700, H.J. Lu wrote: [...] > > > I updated my glibc patch to add both _SC_MINSIGSTKSZ and _SC_SIGSTKSZ. > > > _SC_MINSIGSTKSZ is the minimum signal stack size from AT_MINSIGSTKSZ, > > > which is the signal frame size used by kernel, and _SC_SIGSTKSZ is the value > > > of sysconf (_SC_MINSIGSTKSZ) + 6KB for user application. > > > > Can I suggest sysconf (_SC_MINSIGSTKSZ) * 4 instead? > > Done. OK. I was prepared to have to argue my case a bit more, but if you think this is OK then I will stop arguing ;) > > If the arch has more or bigger registers to save in the signal frame, > > the chances are that they're going to get saved in some userspace stack > > frames too. So I suspect that the user signal handler stack usage may > > scale up to some extent rather than being a constant. > > > > > > To help people migrate without unpleasant surprises, I also figured it > > would be a good idea to make sure that sysconf (_SC_MINSIGSTKSZ) >= > > legacy MINSIGSTKSZ, and sysconf (_SC_SIGSTKSZ) >= legacy SIGSTKSZ. > > This should makes it safer to use sysconf (_SC_MINSIGSTKSZ) as a > > drop-in replacement for MINSIGSTKSZ, etc. > > > > (To explain: AT_MINSIGSTKSZ may actually be < MINSIGSTKSZ on AArch64. > > My idea was that sysconf () should hide this surprise, but people who > > really want to know the kernel value would tolerate some > > nonportability and read AT_MINSIGSTKSZ directly.) > > > > > > So then: > > > > kernel_minsigstksz = getauxval(AT_MINSIGSTKSZ); > > minsigstksz = LEGACY_MINSIGSTKSZ; > > if (kernel_minsigstksz > minsigstksz) > > minsistksz = kernel_minsigstksz; > > > > sigstksz = LEGACY_SIGSTKSZ; > > if (minsigstksz * 4 > sigstksz) > > sigstksz = minsigstksz * 4; > > I updated users/hjl/AT_MINSIGSTKSZ branch with > > +@item _SC_MINSIGSTKSZ > +@standards{GNU, unistd.h} Can we specify these more agressively now? > +Inquire about the signal stack size used by the kernel. I think we've already concluded that this should included all mandatory overheads, including those imposed by the compiler and glibc? e.g.: --8<-- The returned value is the minimum number of bytes of free stack space required in order to gurantee successful, non-nested handling of a single signal whose handler is an empty function. -->8-- > + > +@item _SC_SIGSTKSZ > +@standards{GNU, unistd.h} > +Inquire about the default signal stack size for a signal handler. Similarly: --8<-- The returned value is the suggested minimum number of bytes of stack space required for a signal stack. This is not guaranteed to be enough for any specific purpose other than the invocation of a single, non-nested, empty handler, but nonetheless should be enough for basic scenarios involving simple signal handlers and very low levels of signal nesting (say, 2 or 3 levels at the very most). This value is provided for developer convenience and to ease migration from the legacy SIGSTKSZ constant. Programs requiring stronger guarantees should avoid using it if at all possible. -->8-- If these descriptions are too wordy, we might want to move some of it out to signal.texi, though. > > case _SC_MINSIGSTKSZ: > assert (GLRO(dl_minsigstacksize) != 0); > return GLRO(dl_minsigstacksize); > > case _SC_SIGSTKSZ: > { > /* Return MAX (MINSIGSTKSZ, sysconf (_SC_MINSIGSTKSZ)) * 4. */ > long int minsigstacksize = GLRO(dl_minsigstacksize); > _Static_assert (__builtin_constant_p (MINSIGSTKSZ), > "MINSIGSTKSZ is constant"); > if (minsigstacksize < MINSIGSTKSZ) > minsigstacksize = MINSIGSTKSZ; > return minsigstacksize * 4; > } > > > > > (Or something like that, unless the architecture provides its own > > definitions. ia64's MINSIGSTKSZ is enormous, so it probably doesn't > > want this.) > > > > > > Also: should all these values be rounded up to a multiple of the > > architecture's preferred stack alignment? > > Kernel should provide a properly aligned AT_MINSIGSTKSZ. OK. Can you comment on Chang S. Bae's series? I wasn't sure that the proposal produces an aligned value for AT_MINSIGSTKSZ on x86, but maybe I just worked it out wrong. > > Should the preferred stack alignment also be exposed through sysconf? > > Portable code otherwise has no way to know this, though if the > > preferred alignment is <= the minimum malloc()/alloca() alignment then > > this is generally not an issue.) > > Good question. But it is orthogonal to the signal stack size issue. Ack. There are various other brokennesses around this area, such as the fact that the register data may now run off the end of the mcontext_t object that is supposed to contain it. Ideally we should fork ucontext_t or mcontext_t into two types, one for the ucontext API and one for the signal API (which are anyway highly incompatible with each other). If this stuff is addressed, I guess we could bundle it under a more general feature test macro. But it's probably best to nail down this series in something like its current form first. I'll follow up on libc-alpha with a wishlist, but that will be a separate conversation... [...] Cheers ---Dave