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 [8.43.85.97]) (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 43D301F4B4 for ; Mon, 12 Oct 2020 13:21:55 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4290C38708E5; Mon, 12 Oct 2020 13:21:54 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4290C38708E5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1602508914; bh=QalWdF9/5Mh+A+K6SntDg+EL8fVfIRHZWAuNPnf9K6Q=; 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=DYCQlpEp1qqjw2Dlb6GCISKi9aZLhSS1sWgYDz/taS7tWutiP0F4A3r2z00i7KxZA +OG4hPt7h+x/XBCxPvwFUvwfITfk60ZLi0VJIg/zkWEgHwEP6Fn83kkxm0KiL9Lmdr tnTNux8g0pnLTHM4lz6md5zHKZydvs7AzUSic3WE= Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 26B9B3857C45 for ; Mon, 12 Oct 2020 13:21:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 26B9B3857C45 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 C1E66D6E; Mon, 12 Oct 2020 06:21:50 -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 375BD3F66B; Mon, 12 Oct 2020 06:21:50 -0700 (PDT) Date: Mon, 12 Oct 2020 14:21:47 +0100 To: "H.J. Lu" Subject: Re: V4 [PATCH] sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305] Message-ID: <20201012132146.GB32292@arm.com> References: <20201010121935.3263605-1-hjl.tools@gmail.com> <20201012075332.GB3819@arm.com> <20201012110359.GA32292@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: GNU C Library , libc-coord@lists.openwall.com Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On Mon, Oct 12, 2020 at 05:42:06AM -0700, H.J. Lu via Libc-alpha wrote: > On Mon, Oct 12, 2020 at 4:04 AM Dave Martin wrote: > > > > On Mon, Oct 12, 2020 at 08:53:32AM +0100, Szabolcs Nagy via Libc-alpha wrote: > > > The 10/10/2020 05:19, H.J. Lu via Libc-alpha wrote: > > > > Add _SC_MINSIGSTKSZ for the minimum signal stack size derived from > > > > AT_MINSIGSTKSZ, which 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, and _SC_SIGSTKSZ > > > > which is the suggested minimum number of bytes of stack space required > > > > for a signal stack. > > > > > > > > If AT_MINSIGSTKSZ isn't available, sysconf (_SC_MINSIGSTKSZ) returns > > > > MINSIGSTKSZ. On Linux/x86 with XSAVE, the signal frame used by kernel > > > > is composed of the following areas and laid out as: > > > > > > > > ------------------------------ > > > > | alignment padding | > > > > ------------------------------ > > > > | xsave buffer | > > > > ------------------------------ > > > > | fsave header (32-bit only) | > > > > ------------------------------ > > > > | siginfo + ucontext | > > > > ------------------------------ > > > > > > > > Compute AT_MINSIGSTKSZ value as size of xsave buffer + size of fsave > > > > header (32-bit only) + size of siginfo and ucontext + alignment padding. > > > > > > > > If _SC_SIGSTKSZ_SOURCE is defined, MINSIGSTKSZ and SIGSTKSZ are redefined > > > > as > > > > > > > > /* Default stack size for a signal handler: sysconf (SC_SIGSTKSZ). */ > > > > # undef SIGSTKSZ > > > > # define SIGSTKSZ sysconf (_SC_SIGSTKSZ) > > > > > > > > /* Minimum stack size for a signal handler: SIGSTKSZ. */ > > > > # undef MINSIGSTKSZ > > > > # define MINSIGSTKSZ SIGSTKSZ > > > > > > > > Compilation will fail if the source assumes constant MINSIGSTKSZ or > > > > SIGSTKSZ. > > > > > > > > The reason for not simply increasing the kernel's MINSIGSTKSZ #define > > > > (apart from the fact that it is rarely used, due to glibc's shadowing > > > > definitions) was that userspace binaries will have baked in the old > > > > value of the constant and may be making assumptions about it. > > > > > > > > For example, the type (char [MINSIGSTKSZ]) changes if this #define > > > > changes. This could be a problem if an newly built library tries to > > > > memcpy() or dump such an object defined by and old binary. > > > > Bounds-checking and the stack sizes passed to things like sigaltstack() > > > > and makecontext() could similarly go wrong. > > > > > > > > > this looks reasonable to me. > > > > > > i added libc-coord on cc as it seems to be > > > a useful generic api across targets. > > > > > > > > > /* Return MAX (MINSIGSTKSZ, sysconf (_SC_MINSIGSTKSZ)) * 4. */ > > > this can be excessive for sigstksz, > > > but reasonable on glibc given the > > > overhead of libc internal signals > > > and lazy binding. > > > > Interesting points. Can we put actual numbers on those? > > > > Code that tries to allocate correctly sized stacks would need this > > information. > > > > In the presence of things like IFUNC, I suppose there might be no hard > > limit on the amount of stack that might be required to resolve a > > symbol, but I hope most IFUNC functions are pretty minimal. > > > > > does this decrease the size on any > > > existing target? > > > > To avoid unpleasant surprises, I think we should explicitly clamp both > > parameters to be no less than the value of the legacy #define. Then > > the answer becomes "no" by construction. Allowing them to be smaller > > will likely save little memory, so it's probably not worth the risk it. > > I added sysconf-sigstksz.h: > > static long int > sysconf_sigstksz (void) > { > long int minsigstacksize = GLRO(dl_minsigstacksize); > assert (minsigstacksize != 0); > _Static_assert (__builtin_constant_p (MINSIGSTKSZ), > "MINSIGSTKSZ is constant"); > if (minsigstacksize < MINSIGSTKSZ) > minsigstacksize = MINSIGSTKSZ; > /* MAX (MINSIGSTKSZ, sysconf (_SC_MINSIGSTKSZ)) * 4. */ > long int sigstacksize = minsigstacksize * 4; > /* Return MAX (SIGSTKSZ, sigstacksize). */ > _Static_assert (__builtin_constant_p (SIGSTKSZ), > "SIGSTKSZ is constant"); > if (sigstacksize < SIGSTKSZ) > sigstacksize = SIGSTKSZ; > return sigstacksize; > } > > A target can override it. Something like that seems reasonable. > > (The same rule doesn't apply to AT_MINSIGSTKSZ though, since that's a > > new invention, and purely a property of the kernel. We yield the "true" > > kernel value there, which can be less then MINSIGSTKSZ.) > > > > > > On ia64, we should probably override the default "* 4" rule and keep the > > old value. There seems zero chance that this architecture will be > > extended with additional register state, and this avoids making the > > current huge value even larger. > > IA64 has > > /* Minimum stack size for a signal handler. */ > #define MINSIGSTKSZ 4096 > > /* System default stack size. */ > #define SIGSTKSZ 16384 Really? I see in sysdeps/unix/sysv/linux/ia64/bits/sigstack.h: Yes, this should be 131072 but the constant got defined incorrectly in the kernel and we have to live with it. Users should in any case use SIGSTKSZ as the size user-supplied buffers should have. */ #define MINSIGSTKSZ 131027 /* System default stack size. */ #define SIGSTKSZ 262144 (which matches the kernel definitions.) > On ia64, SIGSTKSZ is unchanged with and without > -D_SC_SIGSTKSZ_SOURCE. If needed, ia64 can override > bits/sigstksz.h to keep MINSIGSTKSZ and SIGSTKSZ as is. OK, I guess if the effect is just to leave the old values for ia64 then that should be fine. Cheers ---Dave