sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305]

  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.
---
 NEWS                                          |  5 ++
 bits/confname.h                               |  8 +-
 bits/sigstksz.h                               | 21 +++++
 conform/data/unistd.h-data                    |  2 +
 elf/dl-support.c                              |  5 ++
 elf/dl-sysdep.c                               |  9 ++
 include/features.h                            |  8 ++
 manual/conf.texi                              | 21 +++++
 posix/sysconf.c                               |  3 +
 signal/Makefile                               |  5 +-
 signal/signal.h                               |  1 +
 signal/tst-minsigstksz-5.c                    | 84 +++++++++++++++++++
 support/Makefile                              |  3 +
 sysdeps/generic/ldsodefs.h                    |  3 +
 sysdeps/unix/sysv/linux/Makefile              |  8 ++
 sysdeps/unix/sysv/linux/bits/sigstksz.h       | 33 ++++++++
 sysdeps/unix/sysv/linux/sysconf.c             | 16 ++++
 .../unix/sysv/linux/x86/dl-minsigstacksize.h  | 77 +++++++++++++++++
 sysdeps/x86/cpu-features.c                    | 16 ++--
 sysdeps/x86/dl-minsigstacksize.h              | 23 +++++
 20 files changed, 343 insertions(+), 8 deletions(-)
 create mode 100644 bits/sigstksz.h
 create mode 100644 signal/tst-minsigstksz-5.c
 create mode 100644 sysdeps/unix/sysv/linux/bits/sigstksz.h
 create mode 100644 sysdeps/unix/sysv/linux/x86/dl-minsigstacksize.h
 create mode 100644 sysdeps/x86/dl-minsigstacksize.h
  

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.

does this decrease the size on any
existing target?


> ---
>  NEWS                                          |  5 ++
>  bits/confname.h                               |  8 +-
>  bits/sigstksz.h                               | 21 +++++
>  conform/data/unistd.h-data                    |  2 +
>  elf/dl-support.c                              |  5 ++
>  elf/dl-sysdep.c                               |  9 ++
>  include/features.h                            |  8 ++
>  manual/conf.texi                              | 21 +++++
>  posix/sysconf.c                               |  3 +
>  signal/Makefile                               |  5 +-
>  signal/signal.h                               |  1 +
>  signal/tst-minsigstksz-5.c                    | 84 +++++++++++++++++++
>  support/Makefile                              |  3 +
>  sysdeps/generic/ldsodefs.h                    |  3 +
>  sysdeps/unix/sysv/linux/Makefile              |  8 ++
>  sysdeps/unix/sysv/linux/bits/sigstksz.h       | 33 ++++++++
>  sysdeps/unix/sysv/linux/sysconf.c             | 16 ++++
>  .../unix/sysv/linux/x86/dl-minsigstacksize.h  | 77 +++++++++++++++++
>  sysdeps/x86/cpu-features.c                    | 16 ++--
>  sysdeps/x86/dl-minsigstacksize.h              | 23 +++++
>  20 files changed, 343 insertions(+), 8 deletions(-)
>  create mode 100644 bits/sigstksz.h
>  create mode 100644 signal/tst-minsigstksz-5.c
>  create mode 100644 sysdeps/unix/sysv/linux/bits/sigstksz.h
>  create mode 100644 sysdeps/unix/sysv/linux/x86/dl-minsigstacksize.h
>  create mode 100644 sysdeps/x86/dl-minsigstacksize.h
> 
> diff --git a/NEWS b/NEWS
> index e84c39aeb1..a7995f8c6a 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -9,6 +9,11 @@ Version 2.33
>  
>  Major new features:
>  
> +* Add _SC_MINSIGSTKSZ and _SC_SIGSTKSZ.  When _SC_SIGSTKSZ_SOURCE is
> +  defined, MINSIGSTKSZ and SIGSTKSZ are no longer constant on Linux.
> +  MINSIGSTKSZ is redefined to sysconf(_SC_MINSIGSTKSZ) and SIGSTKSZ
> +  is redefined to sysconf (_SC_SIGSTKSZ).
> +
>  * The dynamic linker accepts the --argv0 argument and provides opportunity
>    to change argv[0] string.
>  
> diff --git a/bits/confname.h b/bits/confname.h
> index 5dc8215093..451d3eb636 100644
> --- a/bits/confname.h
> +++ b/bits/confname.h
> @@ -525,8 +525,14 @@ enum
>  
>      _SC_THREAD_ROBUST_PRIO_INHERIT,
>  #define _SC_THREAD_ROBUST_PRIO_INHERIT	_SC_THREAD_ROBUST_PRIO_INHERIT
> -    _SC_THREAD_ROBUST_PRIO_PROTECT
> +    _SC_THREAD_ROBUST_PRIO_PROTECT,
>  #define _SC_THREAD_ROBUST_PRIO_PROTECT	_SC_THREAD_ROBUST_PRIO_PROTECT
> +
> +    _SC_MINSIGSTKSZ,
> +#define	_SC_MINSIGSTKSZ			_SC_MINSIGSTKSZ
> +
> +    _SC_SIGSTKSZ
> +#define	_SC_SIGSTKSZ			_SC_SIGSTKSZ
>    };
>  
>  /* Values for the NAME argument to `confstr'.  */
> diff --git a/bits/sigstksz.h b/bits/sigstksz.h
> new file mode 100644
> index 0000000000..5535d873b5
> --- /dev/null
> +++ b/bits/sigstksz.h
> @@ -0,0 +1,21 @@
> +/* Definition of MINSIGSTKSZ.  Generic 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
> diff --git a/conform/data/unistd.h-data b/conform/data/unistd.h-data
> index aa070528e8..70ab521a29 100644
> --- a/conform/data/unistd.h-data
> +++ b/conform/data/unistd.h-data
> @@ -242,6 +242,7 @@ constant _SC_MEMLOCK_RANGE
>  constant _SC_MEMORY_PROTECTION
>  constant _SC_MESSAGE_PASSING
>  #ifndef POSIX
> +constant _SC_MINSIGSTKSZ
>  constant _SC_MONOTONIC_CLOCK
>  #endif
>  constant _SC_MQ_OPEN_MAX
> @@ -279,6 +280,7 @@ constant _SC_SIGNALS
>  #endif
>  constant _SC_SIGQUEUE_MAX
>  #ifndef POSIX
> +constant _SC_SIGSTKSZ
>  constant _SC_SINGLE_PROCESS
>  constant _SC_SPIN_LOCKS
>  #endif
> diff --git a/elf/dl-support.c b/elf/dl-support.c
> index afbc94df54..18ac0bdd44 100644
> --- a/elf/dl-support.c
> +++ b/elf/dl-support.c
> @@ -136,6 +136,8 @@ void (*_dl_init_static_tls) (struct link_map *) = &_dl_nothread_init_static_tls;
>  
>  size_t _dl_pagesize = EXEC_PAGESIZE;
>  
> +size_t _dl_minsigstacksize = MINSIGSTKSZ;
> +
>  int _dl_inhibit_cache;
>  
>  unsigned int _dl_osversion;
> @@ -294,6 +296,9 @@ _dl_aux_init (ElfW(auxv_t) *av)
>        case AT_RANDOM:
>  	_dl_random = (void *) av->a_un.a_val;
>  	break;
> +      case AT_MINSIGSTKSZ:
> +	GLRO(dl_minsigstacksize) = av->a_un.a_val;
> +	break;
>        DL_PLATFORM_AUXV
>        }
>    if (seen == 0xf)
> diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
> index 854570821c..2b1bce8cf5 100644
> --- a/elf/dl-sysdep.c
> +++ b/elf/dl-sysdep.c
> @@ -117,6 +117,11 @@ _dl_sysdep_start (void **start_argptr,
>    user_entry = (ElfW(Addr)) ENTRY_POINT;
>    GLRO(dl_platform) = NULL; /* Default to nothing known about the platform.  */
>  
> +  /* NB: Default to a constant MINSIGSTKSZ.  */
> +  _Static_assert (__builtin_constant_p (MINSIGSTKSZ),
> +		  "MINSIGSTKSZ is constant");
> +  GLRO(dl_minsigstacksize) = MINSIGSTKSZ;
> +
>    for (av = GLRO(dl_auxv); av->a_type != AT_NULL; set_seen (av++))
>      switch (av->a_type)
>        {
> @@ -181,6 +186,9 @@ _dl_sysdep_start (void **start_argptr,
>        case AT_RANDOM:
>  	_dl_random = (void *) av->a_un.a_val;
>  	break;
> +      case AT_MINSIGSTKSZ:
> +	GLRO(dl_minsigstacksize) = av->a_un.a_val;
> +	break;
>        DL_PLATFORM_AUXV
>        }
>  
> @@ -308,6 +316,7 @@ _dl_show_auxv (void)
>  	  [AT_SYSINFO_EHDR - 2] =	{ "SYSINFO_EHDR:      0x", hex },
>  	  [AT_RANDOM - 2] =		{ "RANDOM:            0x", hex },
>  	  [AT_HWCAP2 - 2] =		{ "HWCAP2:            0x", hex },
> +	  [AT_MINSIGSTKSZ - 2] =	{ "MINSIGSTKSZ        ", dec },
>  	  [AT_L1I_CACHESIZE - 2] =	{ "L1I_CACHESIZE:     ", dec },
>  	  [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..fa5a94455a 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.
> +
>     _REENTRANT, _THREAD_SAFE
>  			Obsolete; equivalent to _POSIX_C_SOURCE=199506L.
>  
> @@ -96,6 +98,7 @@
>     __USE_ATFILE		Define *at interfaces and AT_* constants for them.
>     __USE_GNU		Define GNU extensions.
>     __USE_FORTIFY_LEVEL	Additional security measures used, according to level.
> +   __USE_SC_SIGSTKSZ	Define non-constant MINSIGSTKSZ and SIGSTKSZ.
>  
>     The macros `__GNU_LIBRARY__', `__GLIBC__', and `__GLIBC_MINOR__' are
>     defined by this file unconditionally.  `__GNU_LIBRARY__' is provided
> @@ -139,6 +142,7 @@
>  #undef	__USE_ATFILE
>  #undef	__USE_GNU
>  #undef	__USE_FORTIFY_LEVEL
> +#undef	__USE_SC_SIGSTKSZ
>  #undef	__KERNEL_STRICT_NAMES
>  #undef	__GLIBC_USE_ISOC2X
>  #undef	__GLIBC_USE_DEPRECATED_GETS
> @@ -407,6 +411,10 @@
>  # define __USE_FORTIFY_LEVEL 0
>  #endif
>  
> +#ifdef	_SC_SIGSTKSZ_SOURCE
> +# define __USE_SC_SIGSTKSZ	1
> +#endif
> +
>  /* The function 'gets' existed in C89, but is impossible to use
>     safely.  It has been removed from ISO C11 and ISO C++14.  Note: for
>     compatibility with various implementations of <cstdio>, this test
> diff --git a/manual/conf.texi b/manual/conf.texi
> index f959b00bb6..ba9847aaa4 100644
> --- a/manual/conf.texi
> +++ b/manual/conf.texi
> @@ -913,6 +913,27 @@ Inquire about the parameter corresponding to @code{NL_SETMAX}.
>  @item _SC_NL_TEXTMAX
>  @standards{X/Open, unistd.h}
>  Inquire about the parameter corresponding to @code{NL_TEXTMAX}.
> +
> +@item _SC_MINSIGSTKSZ
> +@standards{GNU, unistd.h}
> +Inquire about the minimum number of bytes of free stack space required
> +in order to guarantee successful, non-nested handling of a single signal
> +whose handler is an empty function.
> +
> +@item _SC_SIGSTKSZ
> +@standards{GNU, unistd.h}
> +Inquire about 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 @code{SIGSTKSZ} constant.  Programs requiring stronger
> +guarantees should avoid using it if at all possible.
>  @end vtable
>  
>  @node Examples of Sysconf
> diff --git a/posix/sysconf.c b/posix/sysconf.c
> index ba303384c1..ca7833e6c4 100644
> --- a/posix/sysconf.c
> +++ b/posix/sysconf.c
> @@ -266,6 +266,9 @@ __sysconf (int name)
>      case _SC_XOPEN_REALTIME:
>      case _SC_XOPEN_REALTIME_THREADS:
>  
> +    case _SC_MINSIGSTKSZ:
> +    case _SC_SIGSTKSZ:
> +
>        break;
>      }
>  
> diff --git a/signal/Makefile b/signal/Makefile
> index 2ec3ddd74f..641d30582d 100644
> --- a/signal/Makefile
> +++ b/signal/Makefile
> @@ -31,7 +31,8 @@ headers := signal.h sys/signal.h \
>  	   bits/types/sigevent_t.h bits/types/siginfo_t.h \
>  	   bits/types/sigset_t.h bits/types/sigval_t.h \
>  	   bits/types/stack_t.h bits/types/struct_sigstack.h \
> -	   bits/types/__sigval_t.h bits/signal_ext.h
> +	   bits/types/__sigval_t.h bits/signal_ext.h \
> +	   bits/sigstksz.h
>  
>  routines	:= signal raise killpg \
>  		   sigaction sigprocmask kill \
> @@ -48,7 +49,7 @@ routines	:= signal raise killpg \
>  tests		:= tst-signal tst-sigset tst-sigsimple tst-raise tst-sigset2 \
>  		   tst-sigwait-eintr tst-sigaction \
>  		   tst-minsigstksz-1 tst-minsigstksz-2 tst-minsigstksz-3 \
> -		   tst-minsigstksz-3a tst-minsigstksz-4 \
> +		   tst-minsigstksz-3a tst-minsigstksz-4 tst-minsigstksz-5 \
>  		   tst-sigisemptyset
>  
>  include ../Rules
> diff --git a/signal/signal.h b/signal/signal.h
> index effe3d698f..0311eb2a66 100644
> --- a/signal/signal.h
> +++ b/signal/signal.h
> @@ -312,6 +312,7 @@ extern int siginterrupt (int __sig, int __interrupt) __THROW
>    __attribute_deprecated_msg__ ("Use sigaction with SA_RESTART instead");
>  
>  # include <bits/sigstack.h>
> +# include <bits/sigstksz.h>
>  # include <bits/ss_flags.h>
>  
>  /* Alternate signal handler stack interface.
> diff --git a/signal/tst-minsigstksz-5.c b/signal/tst-minsigstksz-5.c
> new file mode 100644
> index 0000000000..8be380227c
> --- /dev/null
> +++ b/signal/tst-minsigstksz-5.c
> @@ -0,0 +1,84 @@
> +/* Test of signal delivery on an alternate stack with MINSIGSTKSZ size.
> +   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/>.  */
> +
> +#include <signal.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +
> +static volatile sig_atomic_t handler_run;
> +
> +static void
> +handler (int signo)
> +{
> +  /* Clear a bit of on-stack memory.  */
> +  volatile char buffer[256];
> +  for (size_t i = 0; i < sizeof (buffer); ++i)
> +    buffer[i] = 0;
> +  handler_run = 1;
> +}
> +
> +int
> +do_test (void)
> +{
> +  size_t stack_buffer_size = 64 * 1024 * 1024;
> +  void *stack_buffer = xmalloc (stack_buffer_size);
> +  void *stack_end = stack_buffer + stack_buffer_size;
> +  memset (stack_buffer, 0xCC, stack_buffer_size);
> +
> +  void *stack_bottom = stack_buffer + (stack_buffer_size + MINSIGSTKSZ) / 2;
> +  void *stack_top = stack_bottom + MINSIGSTKSZ;
> +  stack_t stack =
> +    {
> +      .ss_sp = stack_bottom,
> +      .ss_size = MINSIGSTKSZ,
> +    };
> +  if (sigaltstack (&stack, NULL) < 0)
> +    FAIL_RET ("sigaltstack: %m\n");
> +
> +  struct sigaction act =
> +    {
> +      .sa_handler = handler,
> +      .sa_flags = SA_ONSTACK,
> +    };
> +  if (sigaction (SIGUSR1, &act, NULL) < 0)
> +    FAIL_RET ("sigaction: %m\n");
> +
> +  if (kill (getpid (), SIGUSR1) < 0)
> +    FAIL_RET ("kill: %m\n");
> +
> +  if (handler_run != 1)
> +    FAIL_RET ("handler did not run\n");
> +
> +  for (void *p = stack_buffer; p < stack_bottom; ++p)
> +    if (*(unsigned char *) p != 0xCC)
> +      FAIL_RET ("changed byte %zd bytes below configured stack\n",
> +		stack_bottom - p);
> +  for (void *p = stack_top; p < stack_end; ++p)
> +    if (*(unsigned char *) p != 0xCC)
> +      FAIL_RET ("changed byte %zd bytes above configured stack\n",
> +		p - stack_top);
> +
> +  free (stack_buffer);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/support/Makefile b/support/Makefile
> index 93faafddf9..ae889d8627 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -203,6 +203,9 @@ CFLAGS-support_paths.c = \
>  # -fexcess-precision=standard.
>  CFLAGS-timespec.c += -fexcess-precision=standard
>  
> +# Get the real <bits/sigstksz.h>.
> +CFLAGS-xsigstack.c += -D_ISOMAC
> +
>  ifeq (,$(CXX))
>  LINKS_DSO_PROGRAM = links-dso-program-c
>  else
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index 382eeb9be0..5ce847aeac 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -528,6 +528,9 @@ struct rtld_global_ro
>    /* Cached value of `getpagesize ()'.  */
>    EXTERN size_t _dl_pagesize;
>  
> +  /* Cached value of `sysconf (_SC_MINSIGSTKSZ)'.  */
> +  EXTERN size_t _dl_minsigstacksize;
> +
>    /* Do we read from ld.so.cache?  */
>    EXTERN int _dl_inhibit_cache;
>  
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index d760debf40..a17ca28de8 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -190,6 +190,9 @@ sysdep_routines += ntp_gettime ntp_gettimex
>  endif
>  
>  ifeq ($(subdir),signal)
> +# Compile tst-minsigstksz-5.c with _SC_SIGSTKSZ_SOURCE.
> +CFLAGS-tst-minsigstksz-5.c += -D_SC_SIGSTKSZ_SOURCE
> +
>  tests-special += $(objpfx)tst-signal-numbers.out
>  # Depending on signal.o* is a hack.  What we actually want is a dependency
>  # on signal.h and everything it includes.  That's impractical to write
> @@ -228,6 +231,11 @@ ifeq ($(subdir),sunrpc)
>  sysdep_headers += nfs/nfs.h
>  endif
>  
> +ifeq ($(subdir),support)
> +# Compile xsigstack.c with _SC_SIGSTKSZ_SOURCE.
> +CFLAGS-xsigstack.c += -D_SC_SIGSTKSZ_SOURCE
> +endif
> +
>  ifeq ($(subdir),termios)
>  sysdep_headers += termio.h
>  endif
> diff --git a/sysdeps/unix/sysv/linux/bits/sigstksz.h b/sysdeps/unix/sysv/linux/bits/sigstksz.h
> new file mode 100644
> index 0000000000..8e91bfad64
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/bits/sigstksz.h
> @@ -0,0 +1,33 @@
> +/* Definition of MINSIGSTKSZ.  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
> +
> +#ifdef __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
> diff --git a/sysdeps/unix/sysv/linux/sysconf.c b/sysdeps/unix/sysv/linux/sysconf.c
> index 7958a74164..0abc8d80bb 100644
> --- a/sysdeps/unix/sysv/linux/sysconf.c
> +++ b/sysdeps/unix/sysv/linux/sysconf.c
> @@ -75,6 +75,22 @@ __sysconf (int name)
>        }
>        break;
>  
> +    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);
> +	assert (minsigstacksize != 0);
> +	_Static_assert (__builtin_constant_p (MINSIGSTKSZ),
> +			"MINSIGSTKSZ is constant");
> +	if (minsigstacksize < MINSIGSTKSZ)
> +	  minsigstacksize = MINSIGSTKSZ;
> +	return minsigstacksize * 4;
> +      }
> +
>      default:
>        break;
>      }
> 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;
> +
> +  /* Emulate AT_MINSIGSTKSZ.  In Linux kernel, the signal frame data
> +     with XSAVE is composed of the following areas and laid out as:
> +     ------------------------------
> +     | alignment padding          |
> +     ------------------------------
> +     | xsave buffer               |
> +     ------------------------------
> +     | fsave header (32-bit only) |
> +     ------------------------------
> +     | siginfo + ucontext         |
> +     ------------------------------
> + */
> +
> +  unsigned int sigframe_size;
> +
> +#ifdef __x86_64__
> +  /* NB: sizeof(struct rt_sigframe) + 8-byte return address in Linux
> +     kernel.  */
> +  sigframe_size = 440 + 8;
> +#else
> +  /* NB: sizeof(struct sigframe_ia32) + sizeof(struct fregs_state)) +
> +     4-byte return address + 3 * 4-byte arguments in Linux kernel.  */
> +  sigframe_size = 736 + 112 + 4 + 3 * 4;
> +#endif
> +
> +  /* Add 15 bytes to align the stack to 16 bytes.  */
> +  sigframe_size += 15;
> +
> +  /* Make the space before xsave buffer multiple of 16 bytes.  */
> +  sigframe_size = ALIGN_UP (sigframe_size, 16);
> +
> +  /* Add (64 - 16)-byte padding to align xsave buffer at 64 bytes.  */
> +  sigframe_size += 64 - 16;
> +
> +  unsigned int eax, ebx, ecx, edx;
> +  __cpuid_count (0xd, 0, eax, ebx, ecx, edx);
> +
> +  /* Add the size of xsave buffer.  */
> +  sigframe_size += ebx;
> +
> +  /* Add the size of FP_XSTATE_MAGIC2.  */
> +#define FP_XSTATE_MAGIC2 0x46505845U
> +  sigframe_size += sizeof (FP_XSTATE_MAGIC2);
> +
> +  GLRO(dl_minsigstacksize) = sigframe_size;
> +}
> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> index 286ff96771..c80ab05cbd 100644
> --- a/sysdeps/x86/cpu-features.c
> +++ b/sysdeps/x86/cpu-features.c
> @@ -20,6 +20,7 @@
>  #include <cpu-features.h>
>  #include <dl-hwcap.h>
>  #include <libc-pointer-arith.h>
> +#include <dl-minsigstacksize.h>
>  
>  #if HAVE_TUNABLES
>  # define TUNABLE_NAMESPACE cpu
> @@ -350,11 +351,16 @@ get_common_indices (struct cpu_features *cpu_features,
>      }
>  
>    if (cpu_features->basic.max_cpuid >= 0xd)
> -    __cpuid_count (0xd, 1,
> -		   cpu_features->features[COMMON_CPUID_INDEX_D_ECX_1].cpuid.eax,
> -		   cpu_features->features[COMMON_CPUID_INDEX_D_ECX_1].cpuid.ebx,
> -		   cpu_features->features[COMMON_CPUID_INDEX_D_ECX_1].cpuid.ecx,
> -		   cpu_features->features[COMMON_CPUID_INDEX_D_ECX_1].cpuid.edx);
> +    {
> +      /* NB: OSXSAVE has been set in COMMON_CPUID_INDEX_1 above.  */
> +      if (CPU_FEATURES_CPU_P (cpu_features, OSXSAVE))
> +	dl_check_minsigstacksize ();
> +      __cpuid_count (0xd, 1,
> +		     cpu_features->features[COMMON_CPUID_INDEX_D_ECX_1].cpuid.eax,
> +		     cpu_features->features[COMMON_CPUID_INDEX_D_ECX_1].cpuid.ebx,
> +		     cpu_features->features[COMMON_CPUID_INDEX_D_ECX_1].cpuid.ecx,
> +		     cpu_features->features[COMMON_CPUID_INDEX_D_ECX_1].cpuid.edx);
> +    }
>  
>    if (cpu_features->basic.max_cpuid >= 0x19)
>      __cpuid_count (0x19, 0,
> diff --git a/sysdeps/x86/dl-minsigstacksize.h b/sysdeps/x86/dl-minsigstacksize.h
> new file mode 100644
> index 0000000000..da2e40ffd4
> --- /dev/null
> +++ b/sysdeps/x86/dl-minsigstacksize.h
> @@ -0,0 +1,23 @@
> +/* Emulate AT_MINSIGSTKSZ.  Generic 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/>.  */
> +
> +/* Emulate AT_MINSIGSTKSZ with XSAVE. */
> +
> +static inline void
> +dl_check_minsigstacksize (void)
> +{
> +}
> -- 
> 2.26.2
> 

--
  

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.

(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.

[...]

Cheers
---Dave
  

On Mon, Oct 12, 2020 at 4:04 AM Dave Martin <Dave.Martin@arm.com> 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.

> (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

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.
  

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 <Dave.Martin@arm.com> 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
  

The 10/12/2020 12:04, 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.

that's not easy in general (and the user would
also need to know the stack use of libc calls).

i think libc internal sigframe and lazy binding
are both roughly MINSIGSTKSZ (because they are
dominated by the register state that has to be
saved), i actually don't know if libc internal
signals go to the user registered sigaltstack
or not (if not then that's not an issue here,
but still an issue for PTHREAD_STACK_MIN).

(if libc internal signals could somehow always
go to a libc internal sigaltstack that would be
best probably, but there is no api for that)


> > 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.
> 
> (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.)

note that there is a posix conformance requirement
on sigaltstack:

The sigaltstack() function shall fail if:

[ENOMEM]
    The size of the alternate stack area is less than MINSIGSTKSZ.


if there are multiple different views on what
MINSIGSTKSZ is then it's not clear what this
requirement is about. i think glibc does not
do any checks and leaves it to the kernel, but
that may not be right if there is different
MINSIGSTKSZ in userspace. and if libc tries to
enforce a larger MINSIGSTKSZ than linux then
using AT_MINSIGSTKSZ may unexpectedly fail.

it is probably best not to enforce the failure
in userspace for now, but that does not seem
conforming.
  

On Mon, Oct 12, 2020 at 03:12:38PM +0100, Szabolcs Nagy via Libc-alpha wrote:
> The 10/12/2020 12:04, 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.
> 
> that's not easy in general (and the user would
> also need to know the stack use of libc calls).
> 
> i think libc internal sigframe and lazy binding
> are both roughly MINSIGSTKSZ (because they are
> dominated by the register state that has to be
> saved), i actually don't know if libc internal
> signals go to the user registered sigaltstack
> or not (if not then that's not an issue here,
> but still an issue for PTHREAD_STACK_MIN).

That seems a reasonable argument.

I think there could be some flexibility here in practice, since there
are often a lot of non-live registers when entring the lazy binding code.

For SVE, we know for example that all the SVE regs (apart from the low
bits of V8-V15 IIRC) are non-live, unless the function call conforms to
the SVE ABI -- the latter can be known at compile-time, so in theory
there could be separate lazy binding stubs for the two cases so that
much of the SVE register save/restore can be skipped when not needed.


For signal frames though, we don't know what code will run once we get
into the handler, so we have to plan for the worst case.


> (if libc internal signals could somehow always
> go to a libc internal sigaltstack that would be
> best probably, but there is no api for that)

Hmmm, true.

> 
> 
> > > 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.
> > 
> > (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.)
> 
> note that there is a posix conformance requirement
> on sigaltstack:
> 
> The sigaltstack() function shall fail if:
> 
> [ENOMEM]
>     The size of the alternate stack area is less than MINSIGSTKSZ.
> 
> 
> if there are multiple different views on what
> MINSIGSTKSZ is then it's not clear what this
> requirement is about. i think glibc does not
> do any checks and leaves it to the kernel, but
> that may not be right if there is different
> MINSIGSTKSZ in userspace. and if libc tries to
> enforce a larger MINSIGSTKSZ than linux then
> using AT_MINSIGSTKSZ may unexpectedly fail.
> 
> it is probably best not to enforce the failure
> in userspace for now, but that does not seem
> conforming.

I'd say the "obvious" intention here is to avoid people blindly
registering stacks that are too small.  But unless the handler really
does nothing, MINSIGSTKSZ is likely to be insufficient anyway.  This
rule gives LTP something to do, but I'm less than convinced it's
useful (or even harmless).

So I'd hope that we could assert that MINSIGSTKSZ is not always the
correct value here.  A successful sigaltstack() doesn't mean the
stack is big enough, and an ENOMEM error from sigaltstack() has never
meant that the stack is too small (since MINSIGSTKSZ is rounded up on
most arches).

For arm64 we have an explicit opt-in for monster signal frames, so
we could apply a larger threshold in that case.  In the interest of
compatibility we don't do this at present so, yes, you can register a
signal stack that is definitely too small today.

x86 has no explicit opt-in, so it might be tricker to raise the
threshold there.

Cheers
---Dave
  

On Mon, Oct 12, 2020 at 03:37:19PM +0100, Dave Martin wrote:
> For arm64 we have an explicit opt-in for monster signal frames, so
> we could apply a larger threshold in that case.  In the interest of
> compatibility we don't do this at present so, yes, you can register a
> signal stack that is definitely too small today.
> 
> x86 has no explicit opt-in, so it might be tricker to raise the
> threshold there.

Is there any chance we could get the kernel folks to fix this and make
it explicit opt-in on x86 (and all archs)?

Rich
  

On Sat, 10 Oct 2020, H.J. Lu via Libc-alpha wrote:

> +* Add _SC_MINSIGSTKSZ and _SC_SIGSTKSZ.  When _SC_SIGSTKSZ_SOURCE is
> +  defined, MINSIGSTKSZ and SIGSTKSZ are no longer constant on Linux.
> +  MINSIGSTKSZ is redefined to sysconf(_SC_MINSIGSTKSZ) and SIGSTKSZ
> +  is redefined to sysconf (_SC_SIGSTKSZ).

I don't think the _SC_SIGSTKSZ_SOURCE option is a good idea.  But in any 
case, all feature test macros need to be documented in creature.texi, and 
the new _SC_* constants would need documenting in conf.texi, with 
appropriate documentation that they are GNU extensions.

> +#ifdef __USE_SC_SIGSTKSZ

And the internal macros for new feature test macros ought to be tested via 
__GLIBC_USE (thus, always defined to either 0 or 1) rather than #ifdef.
  

On Sat, 10 Oct 2020, H.J. Lu via Libc-alpha wrote:

> diff --git a/conform/data/unistd.h-data b/conform/data/unistd.h-data
> index aa070528e8..70ab521a29 100644
> --- a/conform/data/unistd.h-data
> +++ b/conform/data/unistd.h-data
> @@ -242,6 +242,7 @@ constant _SC_MEMLOCK_RANGE
>  constant _SC_MEMORY_PROTECTION
>  constant _SC_MESSAGE_PASSING
>  #ifndef POSIX
> +constant _SC_MINSIGSTKSZ
>  constant _SC_MONOTONIC_CLOCK
>  #endif
>  constant _SC_MQ_OPEN_MAX
> @@ -279,6 +280,7 @@ constant _SC_SIGNALS
>  #endif
>  constant _SC_SIGQUEUE_MAX
>  #ifndef POSIX
> +constant _SC_SIGSTKSZ
>  constant _SC_SINGLE_PROCESS
>  constant _SC_SPIN_LOCKS
>  #endif

conform/ data is for declarations that are actually part of some standard 
tested there.  No GNU extensions should ever be added there.  If some 
extension is added to the next revision of POSIX, then, when we add 
support for that revision of POSIX to conform/, the additions all need to 
be appropriately conditional so the relevant "constant" lines only appear 
in the preprocessed output when POSIX202X or XOPEN202X (whatever the value 
of X is) are defined - not for any of the existing supported standards 
there.