V4 [PATCH] sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305]

[H.J. Lu]

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.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-sysconf-Add-_SC_MINSIGSTKSZ-_SC_SIGSTKSZ-BZ-20305.patch
Type: application/x-patch
Size: 26130 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20201012/bb634b68/attachment-0001.bin>