From: Dave Martin <Dave.Martin@arm.com>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: GNU C Library <libc-alpha@sourceware.org>,
Joseph Myers <joseph@codesourcery.com>
Subject: Re: V6 [PATCH] sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305]
Date: Mon, 19 Oct 2020 16:08:47 +0100 [thread overview]
Message-ID: <20201019150846.GP32292@arm.com> (raw)
In-Reply-To: <20201015115728.GA64160@gmail.com>
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
> > > + <https://www.gnu.org/licenses/>. */
> > > +
> > > +#ifndef _SIGNAL_H
> > > +# error "Never include <bits/sigstksz.h> directly; use <signal.h> instead."
> > > +#endif
> > > +
> > > +#if __USE_SC_SIGSTKSZ
> > > +# include <unistd.h>
> > > +
> > > +/* 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
> > > + <https://www.gnu.org/licenses/>. */
> > > +
> > > +/* 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
> > > + <https://www.gnu.org/licenses/>. */
> > > +
> > > +#include <unistd.h>
> > > +
> > > +/* 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.
[...]
Cheers
---Dave
next prev parent reply other threads:[~2020-10-19 15:08 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-10 12:19 H.J. Lu
2020-10-12 7:53 ` Szabolcs Nagy
2020-10-12 11:04 ` Dave Martin
2020-10-12 12:42 ` V4 " H.J. Lu
2020-10-12 13:21 ` Dave Martin
2020-10-12 14:12 ` Szabolcs Nagy
2020-10-12 14:37 ` Dave Martin
2020-10-12 15:36 ` [libc-coord] " Rich Felker
2020-10-12 22:03 ` Joseph Myers
2020-10-13 20:32 ` V5 " H.J. Lu
2020-10-14 17:47 ` Dave Martin
2020-10-14 18:07 ` Florian Weimer
2020-10-19 16:30 ` Dave Martin
2020-10-15 11:57 ` V6 " H.J. Lu
2020-10-19 15:08 ` Dave Martin [this message]
2020-10-19 21:32 ` H.J. Lu
2020-10-20 9:19 ` Dave Martin
2020-10-20 14:59 ` H.J. Lu
2020-10-20 15:22 ` Dave Martin
2020-10-20 18:19 ` V7 " H.J. Lu
2020-11-03 3:06 ` PING: " H.J. Lu
2020-11-04 16:50 ` Dave Martin
2020-11-04 17:48 ` H.J. Lu
2020-11-18 14:13 ` H.J. Lu
2020-11-18 14:25 ` Zack Weinberg
2020-11-18 14:40 ` H.J. Lu
2020-11-18 15:12 ` Zack Weinberg
2020-11-18 15:17 ` H.J. Lu
2020-11-18 15:20 ` Florian Weimer
2020-11-18 17:04 ` Dave Martin
2020-11-18 17:35 ` Florian Weimer
2020-11-18 17:48 ` H.J. Lu
2020-11-18 18:09 ` Dave Martin
2020-11-19 14:59 ` Szabolcs Nagy
2020-11-19 15:10 ` H.J. Lu
2020-11-19 15:39 ` Zack Weinberg
2020-11-19 15:51 ` Florian Weimer
2020-11-19 16:16 ` Rich Felker
2020-11-19 16:52 ` Dave Martin
2020-11-19 16:37 ` Dave Martin
2020-11-19 17:29 ` Rich Felker
2020-11-19 17:33 ` Szabolcs Nagy
2020-11-19 19:39 ` Dave Martin
2020-11-20 14:08 ` H.J. Lu
2020-11-20 14:11 ` Florian Weimer
2020-11-20 23:13 ` V8 " H.J. Lu
2021-01-20 14:16 ` Carlos O'Donell
2021-01-20 15:05 ` V9 " H.J. Lu
2021-01-22 19:41 ` V10 " H.J. Lu
2021-01-25 13:31 ` Carlos O'Donell
2021-01-25 13:57 ` H.J. Lu
2021-01-25 13:59 ` Carlos O'Donell
2021-01-25 13:58 ` Carlos O'Donell
2021-01-25 14:16 ` Florian Weimer
2021-02-02 13:08 ` Carlos O'Donell
2021-01-25 14:34 ` Carlos O'Donell
2021-01-20 15:06 ` V8 " Florian Weimer
2021-01-20 15:30 ` Carlos O'Donell
2021-01-20 15:33 ` H.J. Lu
2021-01-20 15:59 ` Carlos O'Donell
2021-01-20 16:04 ` H.J. Lu
2021-01-20 15:33 ` Florian Weimer
2020-10-15 12:26 ` [PATCH] Deprecate SIGSTKSZ/MINSIGSTKSZ with _SC_SIGSTKSZ_SOURCE H.J. Lu
2020-10-15 19:59 ` Joseph Myers
2020-10-15 21:22 ` V2 " H.J. Lu
2020-10-16 0:57 ` Joseph Myers
2021-07-09 18:53 ` Carlos O'Donell
2021-07-09 19:34 ` H.J. Lu
2020-10-12 22:07 ` [PATCH] sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305] Joseph Myers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201019150846.GP32292@arm.com \
--to=dave.martin@arm.com \
--cc=hjl.tools@gmail.com \
--cc=joseph@codesourcery.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).