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.1 required=3.0 tests=AWL,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 96A651F4B4 for ; Mon, 19 Oct 2020 21:33:17 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 62AB4384A01D; Mon, 19 Oct 2020 21:33:16 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 62AB4384A01D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1603143196; bh=Ig/J8s3s1SmDypEQjLSOxjHmDmeyvKMATGqalKUmQG8=; h=References:In-Reply-To:Date:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=n1SzJ79SR/Je8xJxPiJzMV8ZTgBjF0cwFgU+jNRIXxD3dISWJ3VDMzf15Rkm3IuAw 68IPceUyz0xbBRwHGpgHWiCaDGYXH8OmZIJlBGrf09ZFjzOCjfcdAokEQKhGGb354K DjGhG5O0AAdzzfXNSV/H6WaZ+ejJJs09BfKuaWQ4= Received: from mail-oi1-x241.google.com (mail-oi1-x241.google.com [IPv6:2607:f8b0:4864:20::241]) by sourceware.org (Postfix) with ESMTPS id A76143850408 for ; Mon, 19 Oct 2020 21:33:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A76143850408 Received: by mail-oi1-x241.google.com with SMTP id 16so1654164oix.9 for ; Mon, 19 Oct 2020 14:33: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=Ig/J8s3s1SmDypEQjLSOxjHmDmeyvKMATGqalKUmQG8=; b=lnNrOUaChgtAVdBO42BWR54eUYzKVBr+NSSZmg37r1yyfaFx9SvUVO35uoylQ7zCB/ shN9qG78S5sxNkBDif/3oBcSMxQ+gaapnd0doYKDU4YlTZbvdUcTW7dvRbDpXioNqRWZ GrurN56rbDVny328seFH8o7/awXrMZ5OezY9d5cJBJMBq8qUBemHOQ/sk1nM82jEm6u1 kQE0TJ3b31x7jaSmv9fMEn9lWk4hHRGbjdHBRSCerqsA+IskrV3QtZKKwwLXZaSOJbf7 b/YTvGzfHz2bHGsD//t66En36RzgBkrAa0dsUJUuIxg7Ohy0xVnTt5Ld9kvdocC3n55Z 6vsg== X-Gm-Message-State: AOAM532mC/+642t7bQdm9hTXPfonHl+lmU5O6dGlVgBZSyaVk1FZfR3g nKpbwmON10GU15o5EYC3Rro1ooWeOz5H8fkAkHE= X-Google-Smtp-Source: ABdhPJxnzuqgL5kfu28+23ZlRGMFMA6qsSYDlKibitgzYM7nfOoNOprRkYtl6rO4+LjOJ4+Uo7ItHk+ajbAejqmRTOM= X-Received: by 2002:aca:4c7:: with SMTP id 190mr1047961oie.58.1603143191861; Mon, 19 Oct 2020 14:33:11 -0700 (PDT) MIME-Version: 1.0 References: <20201010121935.3263605-1-hjl.tools@gmail.com> <20201014174659.GL32292@arm.com> <20201015115728.GA64160@gmail.com> <20201019150846.GP32292@arm.com> In-Reply-To: <20201019150846.GP32292@arm.com> Date: Mon, 19 Oct 2020 14:32:35 -0700 Message-ID: Subject: Re: V6 [PATCH] sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305] To: Dave Martin Content-Type: text/plain; charset="UTF-8" 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: "H.J. Lu via Libc-alpha" Reply-To: "H.J. Lu" Cc: GNU C Library , Joseph Myers Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On Mon, Oct 19, 2020 at 8:08 AM Dave Martin wrote: > > On Thu, Oct 15, 2020 at 04:57:28AM -0700, H.J. Lu via Libc-alpha wrote: > > On Wed, Oct 14, 2020 at 06:47:01PM +0100, Dave Martin wrote: > > > > [AT_L1I_CACHEGEOMETRY - 2] = { "L1I_CACHEGEOMETRY: 0x", hex }, > > > > [AT_L1D_CACHESIZE - 2] = { "L1D_CACHESIZE: ", dec }, > > > > diff --git a/include/features.h b/include/features.h > > > > index f3e62d3362..38b528e027 100644 > > > > --- a/include/features.h > > > > +++ b/include/features.h > > > > @@ -55,6 +55,8 @@ > > > > _FORTIFY_SOURCE Add security hardening to many library functions. > > > > Set to 1 or 2; 2 performs stricter checks than 1. > > > > > > > > + _SC_SIGSTKSZ_SOURCE Select non-constant MINSIGSTKSZ and SIGSTKSZ. > > > > + > > > > > > Maybe "correct (but non compiletime constant)" > > > > > > > Fixed. > > > > > > diff --git a/sysdeps/unix/sysv/linux/bits/sigstksz.h b/sysdeps/unix/sysv/linux/bits/sigstksz.h > > > > new file mode 100644 > > > > index 0000000000..cd5b3cc895 > > > > --- /dev/null > > > > +++ b/sysdeps/unix/sysv/linux/bits/sigstksz.h > > > > @@ -0,0 +1,33 @@ > > > > +/* Definition of MINSIGSTKSZ and SIGSTKSZ. Linux version. > > > > + Copyright (C) 2020 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 > > > > + . */ > > > > + > > > > +#ifndef _SIGNAL_H > > > > +# error "Never include directly; use instead." > > > > +#endif > > > > + > > > > +#if __USE_SC_SIGSTKSZ > > > > +# include > > > > + > > > > +/* 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 > > > > +#endif > > > > > > To help raise awareness, is it worth adding deprecation warnings on > > > these? > > > > > > Could we still consider them deprecated even with _SC_SIGSTKSZ_SOURCE? > > > Ideally they should be (or even removed), since even if these values are > > > "correct", using them is still a potential portability problem when > > > building for other library stacks. > > > > > > I think the rule ought to be to use these only if _SC_SIGSTKSZ / > > > _SC_MINSIGSTKSZ aren't available, and with the caveat that the values > > > may be wrong -- similar to the situation with PAGESIZE. > > > > > > > > > It could be worth making this feature test macro more general and > > > harvesting any other broken legacy macros we're aware of (such as > > > PAGESIZE, but there are probably others). Probably out of scope for > > > this patch, though. > > > > I will investigate to deprecate MINSIGSTKSZ and SIGSTKSZ after my patch > > has been merged into master branch. > > > > > > diff --git a/sysdeps/unix/sysv/linux/sysconf-sigstksz.h b/sysdeps/unix/sysv/linux/sysconf-sigstksz.h > > > > new file mode 100644 > > > > index 0000000000..64d450b22c > > > > --- /dev/null > > > > +++ b/sysdeps/unix/sysv/linux/sysconf-sigstksz.h > > > > @@ -0,0 +1,38 @@ > > > > +/* sysconf_sigstksz (). Linux version. > > > > + Copyright (C) 2020 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 > > > > + . */ > > > > + > > > > +/* Return sysconf (_SC_SIGSTKSZ). */ > > > > + > > > > +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; > > > > +} > > > > diff --git a/sysdeps/unix/sysv/linux/x86/dl-minsigstacksize.h b/sysdeps/unix/sysv/linux/x86/dl-minsigstacksize.h > > > > new file mode 100644 > > > > index 0000000000..d2dc436572 > > > > --- /dev/null > > > > +++ b/sysdeps/unix/sysv/linux/x86/dl-minsigstacksize.h > > > > @@ -0,0 +1,77 @@ > > > > +/* Emulate AT_MINSIGSTKSZ. Linux/x86 version. > > > > + Copyright (C) 2020 Free Software Foundation, Inc. > > > > + > > > > + 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 > > > > + . */ > > > > + > > > > +#include > > > > + > > > > +/* Emulate AT_MINSIGSTKSZ with XSAVE. */ > > > > + > > > > +static inline void > > > > +dl_check_minsigstacksize (void) > > > > +{ > > > > + /* NB: Default to a constant MINSIGSTKSZ. */ > > > > + _Static_assert (__builtin_constant_p (MINSIGSTKSZ), > > > > + "MINSIGSTKSZ is constant"); > > > > + /* Return if AT_MINSIGSTKSZ is provide by kernel. */ > > > > + if (GLRO(dl_minsigstacksize) != MINSIGSTKSZ) > > > > + return; > > > > > > Couldn't the kernel actually yield MINSIGSTKSZ or a smaller value, say, > > > if running on hardware that doesn't have AVX-512? > > > > > It is OK for MINSIGSTKSZ > AT_MINSIGSTKSZ. For _SC_SIGSTKSZ_SOURCE, > > dynamic MINSIGSTKSZ is defined as sysconf (_SC_SIGSTKSZ) which is > > > > MAX (SIGSTKSZ, MAX (MINSIGSTKSZ, sysconf (_SC_MINSIGSTKSZ)) * 4) > > > > and dynamic MINSIGSTKSZ is always > MINSIGSTKSZ. > > > > > We might want a separate flag to indicate whether we obtained a value > > > from the auxv, rather relying on MINSIGSTKSZ having this magic meaning. > > > > AT_MINSIGSTKSZ is the only way for GLRO(dl_minsigstacksize) != MINSIGSTKSZ. > > Yes, but reading AT_MINSIGSTKSZ doesn't guarantee that > GLRO(dl_minsigstkszsize) != MINSIGSTKSZ, no? > > What if the value reported for AT_MINSIGSTKSZ is actually the same as > MINSIGSTKSZ? This could be the case on some arches in future even if > it's never true today. But the code here assumes that AT_MINSIGSTKSZ > wasn't available in this case, and reverts to a fallback guess. Since the fallback tracks what the kernel does, if AT_MINSIGSTKSZ is 2KB, the fallback will be 2KB or slightly larger. -- H.J.