[PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Zack Weinberg]

We don’t have a full conclusion about the way forward for alternate
signal stacks yet, but the notion that the very short list of things
that ISO C says you can do in an async signal handler should all work
in MINSIGSTKSZ space seemed to be well-received.  As a first step,
therefore, I propose to add tests to make sure those things do work.

To facilitate this, there is a new set of test support routines for
setting up alternate signal stacks; see support/xsignal.h for the API.

All of these tests pass on x86-64.  build-many-glibcs won’t tell us
anything additional; the tests need to be run.

         * support/xsignal.h (xalloc_sigstack, xfree_sigstack)
         (xget_sigstack_location): New test support functions.
         * support/xsigstack.c: New file, implementing them.
         * support/tst-xsigstack.c: New test for them.
         * support/Makefile: Update.

         * signal/tst-minsigstksz-1.c
         * signal/tst-minsigstksz-2.c
         * signal/tst-minsigstksz-3.c
         * signal/tst-minsigstksz-3a.c
         * signal/tst-minsigstksz-4.c: New tests.
         * signal/Makefile: Run them.
---
 signal/Makefile             |   2 +
 signal/tst-minsigstksz-1.c  | 131 ++++++++++++++++++++++++++++++++++++
 signal/tst-minsigstksz-2.c  |  66 ++++++++++++++++++
 signal/tst-minsigstksz-3.c  |  64 ++++++++++++++++++
 signal/tst-minsigstksz-3a.c |  69 +++++++++++++++++++
 signal/tst-minsigstksz-4.c  |  65 ++++++++++++++++++
 support/Makefile            |   2 +
 support/tst-xsigstack.c     |  64 ++++++++++++++++++
 support/xsignal.h           |  18 +++++
 support/xsigstack.c         |  95 ++++++++++++++++++++++++++
 10 files changed, 576 insertions(+)
 create mode 100644 signal/tst-minsigstksz-1.c
 create mode 100644 signal/tst-minsigstksz-2.c
 create mode 100644 signal/tst-minsigstksz-3.c
 create mode 100644 signal/tst-minsigstksz-3a.c
 create mode 100644 signal/tst-minsigstksz-4.c
 create mode 100644 support/tst-xsigstack.c
 create mode 100644 support/xsigstack.c

diff --git a/signal/Makefile b/signal/Makefile
index aa63434f47..d5a10d590d 100644
--- a/signal/Makefile
+++ b/signal/Makefile
@@ -47,6 +47,8 @@ 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 \
 
 include ../Rules
 
diff --git a/signal/tst-minsigstksz-1.c b/signal/tst-minsigstksz-1.c
new file mode 100644
index 0000000000..00344d5fbf
--- /dev/null
+++ b/signal/tst-minsigstksz-1.c
@@ -0,0 +1,131 @@
+/* Tests of signal delivery on an alternate stack (nonlethal).
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/check.h>
+
+/* C2011 7.4.1.1p5 specifies that only the following operations are
+   guaranteed to be well-defined inside an asynchronous signal handler:
+     * any operation on a lock-free atomic object
+     * assigning a value to an object declared as volatile sig_atomic_t
+     * calling abort, _Exit, quick_exit, or signal
+       * signal may only be called with its first argument equal to the
+         number of the signal that caused the handler to be called
+
+   We use this list as a guideline for the set of operations that ought
+   also to be safe in a _synchronous_ signal delivered on an alternate
+   signal stack with only MINSIGSTKSZ bytes of space.
+
+   This test program tests all of the above operations that do not,
+   one way or another, cause the program to be terminated.  */
+
+/* We do not try to test atomic operations exhaustively, only a simple
+   atomic counter increment.  This is only safe if atomic_[u]int is
+   unconditionally lock-free.  */
+#ifdef __STDC_NO_ATOMICS__
+# define TEST_ATOMIC_OPS 0
+#else
+# include <stdatomic.h>
+# if ATOMIC_INT_LOCK_FREE != 2
+#  define TEST_ATOMIC_OPS 0
+# else
+#  define TEST_ATOMIC_OPS 1
+# endif
+#endif
+
+static volatile sig_atomic_t signal_flag = 0;
+static volatile sig_atomic_t signal_err = 0;
+static void
+handler_set_flag (int unused)
+{
+  signal_flag = 1;
+}
+
+static void
+handler_set_flag_once (int sig)
+{
+  signal_flag = 1;
+  if (signal (sig, SIG_IGN) == SIG_ERR)
+    /* It is not safe to call FAIL_EXIT1 here.  Set another flag instead.  */
+    signal_err = 1;
+}
+
+#if TEST_ATOMIC_OPS
+static atomic_uint signal_count = 0;
+static void
+handler_count_up_1 (int unused)
+{
+  atomic_fetch_add (&signal_count, 1);
+}
+#endif
+
+int
+do_test (void)
+{
+  void *sstk = xalloc_sigstack (0);
+  struct sigaction sa;
+
+  /* Test 1: setting a volatile sig_atomic_t flag.  */
+  sa.sa_handler = handler_set_flag;
+  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
+  sigfillset (&sa.sa_mask);
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_EXIT1 ("sigaction (SIGUSR1, handler_set_flag): %m\n");
+
+  TEST_VERIFY_EXIT (signal_flag == 0);
+  raise (SIGUSR1);
+  TEST_VERIFY_EXIT (signal_flag == 1);
+  signal_flag = 0;
+  raise (SIGUSR1);
+  TEST_VERIFY_EXIT (signal_flag == 1);
+  signal_flag = 0;
+
+  /* Test 1: setting a volatile sig_atomic_t flag and then ignoring
+     further delivery of the signal. */
+  sa.sa_handler = handler_set_flag_once;
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_EXIT1 ("sigaction (SIGUSR1, handler_set_flag_once): %m\n");
+
+  raise (SIGUSR1);
+  TEST_VERIFY_EXIT (signal_flag == 1);
+  /* Note: if signal_err is 1, a system call failed, but we can't
+     report the error code because errno is indeterminate.  */
+  TEST_VERIFY_EXIT (signal_err == 0);
+
+  signal_flag = 0;
+  raise (SIGUSR1);
+  TEST_VERIFY_EXIT (signal_flag == 0);
+  TEST_VERIFY_EXIT (signal_err == 0);
+
+#if TEST_ATOMIC_OPS
+  sa.sa_handler = handler_count_up_1;
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_EXIT1 ("sigaction (SIGUSR1, handler_count_up_1): %m\n");
+
+  raise (SIGUSR1);
+  TEST_VERIFY_EXIT (atomic_load (&signal_count) == 1);
+  raise (SIGUSR1);
+  TEST_VERIFY_EXIT (atomic_load (&signal_count) == 2);
+#endif
+
+  xfree_sigstack (sstk);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/signal/tst-minsigstksz-2.c b/signal/tst-minsigstksz-2.c
new file mode 100644
index 0000000000..3368dde6b8
--- /dev/null
+++ b/signal/tst-minsigstksz-2.c
@@ -0,0 +1,66 @@
+/* Tests of signal delivery on an alternate stack (abort).
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/check.h>
+#include <stdlib.h>
+
+/* C2011 7.4.1.1p5 specifies that only the following operations are
+   guaranteed to be well-defined inside an asynchronous signal handler:
+     * any operation on a lock-free atomic object
+     * assigning a value to an object declared as volatile sig_atomic_t
+     * calling abort, _Exit, quick_exit, or signal
+       * signal may only be called with its first argument equal to the
+         number of the signal that caused the handler to be called
+
+   We use this list as a guideline for the set of operations that ought
+   also to be safe in a _synchronous_ signal delivered on an alternate
+   signal stack with only MINSIGSTKSZ bytes of space.
+
+   This test program tests calls to abort.  Note that it does _not_
+   install a handler for SIGABRT, because that signal would also be
+   delivered on the alternate stack and MINSIGSTKSZ does not provide
+   enough space for delivery of nested signals.  */
+
+static void
+handler (int unused)
+{
+  abort ();
+}
+
+int
+do_test (void)
+{
+  void *sstk = xalloc_sigstack (0);
+  struct sigaction sa;
+
+  sa.sa_handler = handler;
+  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
+  sigfillset (&sa.sa_mask);
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
+
+  raise (SIGUSR1);
+
+  xfree_sigstack (sstk);
+  FAIL_RET ("test process was not terminated by abort in signal handler");
+}
+
+#define EXPECTED_SIGNAL SIGABRT
+#include <support/test-driver.c>
diff --git a/signal/tst-minsigstksz-3.c b/signal/tst-minsigstksz-3.c
new file mode 100644
index 0000000000..a8d9a6369c
--- /dev/null
+++ b/signal/tst-minsigstksz-3.c
@@ -0,0 +1,64 @@
+/* Tests of signal delivery on an alternate stack (_Exit).
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/check.h>
+#include <stdlib.h>
+
+/* C2011 7.4.1.1p5 specifies that only the following operations are
+   guaranteed to be well-defined inside an asynchronous signal handler:
+     * any operation on a lock-free atomic object
+     * assigning a value to an object declared as volatile sig_atomic_t
+     * calling abort, _Exit, quick_exit, or signal
+       * signal may only be called with its first argument equal to the
+         number of the signal that caused the handler to be called
+
+   We use this list as a guideline for the set of operations that ought
+   also to be safe in a _synchronous_ signal delivered on an alternate
+   signal stack with only MINSIGSTKSZ bytes of space.
+
+   This test program tests calls to _Exit.  */
+
+#define EXPECTED_STATUS 3
+
+static void
+handler (int unused)
+{
+  _Exit (EXPECTED_STATUS);
+}
+
+int
+do_test (void)
+{
+  void *sstk = xalloc_sigstack (0);
+  struct sigaction sa;
+
+  sa.sa_handler = handler;
+  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
+  sigfillset (&sa.sa_mask);
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
+
+  raise (SIGUSR1);
+
+  xfree_sigstack (sstk);
+  FAIL_RET ("test process was not terminated by _Exit in signal handler");
+}
+
+#include <support/test-driver.c>
diff --git a/signal/tst-minsigstksz-3a.c b/signal/tst-minsigstksz-3a.c
new file mode 100644
index 0000000000..b58b8d01ba
--- /dev/null
+++ b/signal/tst-minsigstksz-3a.c
@@ -0,0 +1,69 @@
+/* Tests of signal delivery on an alternate stack (_exit).
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/check.h>
+#include <unistd.h>
+
+/* C2011 7.4.1.1p5 specifies that only the following operations are
+   guaranteed to be well-defined inside an asynchronous signal handler:
+     * any operation on a lock-free atomic object
+     * assigning a value to an object declared as volatile sig_atomic_t
+     * calling abort, _Exit, quick_exit, or signal
+       * signal may only be called with its first argument equal to the
+         number of the signal that caused the handler to be called
+
+   We use this list as a guideline for the set of operations that ought
+   also to be safe in a _synchronous_ signal delivered on an alternate
+   signal stack with only MINSIGSTKSZ bytes of space.
+
+   This test program tests calls to _exit, which is the same function
+   as _Exit, but specified by POSIX rather than ISO C.  For reasons
+   unknown to the author of this program, the C committee did not
+   think it could standardize _exit under that name; regardless, in a
+   POSIX-conformant environment, they should be completely
+   interchangeable.  */
+
+#define EXPECTED_STATUS 3
+
+static void
+handler (int unused)
+{
+  _exit (EXPECTED_STATUS);
+}
+
+int
+do_test (void)
+{
+  void *sstk = xalloc_sigstack (0);
+  struct sigaction sa;
+
+  sa.sa_handler = handler;
+  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
+  sigfillset (&sa.sa_mask);
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
+
+  raise (SIGUSR1);
+
+  xfree_sigstack (sstk);
+  FAIL_RET ("test process was not terminated by _exit in signal handler");
+}
+
+#include <support/test-driver.c>
diff --git a/signal/tst-minsigstksz-4.c b/signal/tst-minsigstksz-4.c
new file mode 100644
index 0000000000..0dc63b4dd4
--- /dev/null
+++ b/signal/tst-minsigstksz-4.c
@@ -0,0 +1,65 @@
+/* Tests of signal delivery on an alternate stack (quick_exit).
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/check.h>
+#include <stdlib.h>
+
+/* C2011 7.4.1.1p5 specifies that only the following operations are
+   guaranteed to be well-defined inside an asynchronous signal handler:
+     * any operation on a lock-free atomic object
+     * assigning a value to an object declared as volatile sig_atomic_t
+     * calling abort, _Exit, quick_exit, or signal
+       * signal may only be called with its first argument equal to the
+         number of the signal that caused the handler to be called
+
+   We use this list as a guideline for the set of operations that ought
+   also to be safe in a _synchronous_ signal delivered on an alternate
+   signal stack with only MINSIGSTKSZ bytes of space.
+
+   This test program tests calls to quick_exit.  Note that this is only
+   safe when there are no at_quick_exit callbacks.  */
+
+#define EXPECTED_STATUS 3
+
+static void
+handler (int unused)
+{
+  quick_exit (EXPECTED_STATUS);
+}
+
+int
+do_test (void)
+{
+  void *sstk = xalloc_sigstack (0);
+  struct sigaction sa;
+
+  sa.sa_handler = handler;
+  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
+  sigfillset (&sa.sa_mask);
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
+
+  raise (SIGUSR1);
+
+  xfree_sigstack (sstk);
+  FAIL_RET ("test process was not terminated by quick_exit in signal handler");
+}
+
+#include <support/test-driver.c>
diff --git a/support/Makefile b/support/Makefile
index 93a5143016..7fb2de8cee 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -145,6 +145,7 @@ libsupport-routines = \
   xsetsockopt \
   xsigaction \
   xsignal \
+  xsigstack \
   xsocket \
   xstrdup \
   xstrndup \
@@ -205,6 +206,7 @@ tests = \
   tst-test_compare_blob \
   tst-test_compare_string \
   tst-xreadlink \
+  tst-xsigstack \
 
 ifeq ($(run-built-tests),yes)
 tests-special = \
diff --git a/support/tst-xsigstack.c b/support/tst-xsigstack.c
new file mode 100644
index 0000000000..42859c79e9
--- /dev/null
+++ b/support/tst-xsigstack.c
@@ -0,0 +1,64 @@
+/* Test of sigaltstack wrappers.
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/xunistd.h>
+#include <support/check.h>
+
+#include <stdint.h>
+#include <stdio.h>
+
+static volatile uintptr_t handler_stackaddr;
+
+static void
+handler (int unused)
+{
+  int var;
+  handler_stackaddr = (uintptr_t) &var;
+}
+
+int
+do_test (void)
+{
+  void *sstk = xalloc_sigstack (0);
+
+  unsigned char *sp;
+  size_t size;
+  xget_sigstack_location (sstk, &sp, &size);
+  printf ("signal stack installed: sp=%p size=%zu\n", sp, size);
+
+  struct sigaction sa;
+  sa.sa_handler = handler;
+  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
+  sigfillset (&sa.sa_mask);
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
+
+  raise (SIGUSR1);
+
+  uintptr_t haddr = handler_stackaddr;
+  printf ("address of handler local variable: %p\n", (void *)haddr);
+  TEST_VERIFY ((uintptr_t)sp < haddr);
+  TEST_VERIFY (haddr < (uintptr_t)sp + size);
+
+  xfree_sigstack (sstk);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/support/xsignal.h b/support/xsignal.h
index 9ab8d1bfdd..3cf2a6e434 100644
--- a/support/xsignal.h
+++ b/support/xsignal.h
@@ -37,6 +37,24 @@ void xsigaction (int sig, const struct sigaction *newact,
 
 void xpthread_sigmask (int how, const sigset_t *set, sigset_t *oldset);
 
+/* Allocate and activate an alternate signal stack.  This stack will
+   have at least MINSIGSTKSZ + size bytes of space, rounded up to a
+   whole number of pages.  There will be large (at least 1 MiB)
+   inaccessible guard bands on either side of it.  The return value is
+   a cookie that can be passed to xfree_sigstack to deactivate and
+   deallocate the stack again.
+   It is _not_ necessary to call sigaltstack() after calling this function.
+   Terminates the process on error.  */
+void *xalloc_sigstack (size_t size);
+
+/* Deactivate and deallocate a signal stack created by xalloc_sigstack.  */
+void xfree_sigstack (void *stack);
+
+/* Extract the actual address and size of the alternate signal stack from
+   the cookie returned by xalloc_sigstack.  */
+void xget_sigstack_location (const void *stack, unsigned char **addrp,
+                             size_t *sizep);
+
 __END_DECLS
 
 #endif /* SUPPORT_SIGNAL_H */
diff --git a/support/xsigstack.c b/support/xsigstack.c
new file mode 100644
index 0000000000..74b459e2e6
--- /dev/null
+++ b/support/xsigstack.c
@@ -0,0 +1,95 @@
+/* sigaltstack wrappers.
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/xunistd.h>
+#include <support/check.h>
+
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/param.h> /* roundup, MAX */
+
+/* The "cookie" returned by xalloc_sigstack points to one of these
+   structures.  */
+struct sigstack_desc
+{
+  void *alloc_base;  /* Base address of the complete allocation.  */
+  size_t alloc_size; /* Size of the complete allocation.  */
+  stack_t alt_stack; /* The address and size of the stack itself.  */
+  stack_t old_stack; /* The previous signal stack.  */
+};
+
+void *
+xalloc_sigstack (size_t size)
+{
+  size_t pagesize = sysconf (_SC_PAGESIZE);
+  if (pagesize == -1)
+    FAIL_EXIT1 ("sysconf (_SC_PAGESIZE): %m\n");
+
+  size_t stacksize = roundup (size + MINSIGSTKSZ, pagesize);
+  size_t guardsize = MAX (stacksize, roundup (1024 * 1024, pagesize));
+
+  struct sigstack_desc *desc = xmalloc (sizeof (struct sigstack_desc));
+  desc->alloc_size = guardsize + stacksize + guardsize;
+  /* Use MAP_NORESERVE so that RAM will not be wasted on the guard
+     bands; touch all the pages of the actual stack before returning,
+     so we know they are allocated.  */
+  desc->alloc_base = xmmap (0,
+                            desc->alloc_size,
+                            PROT_READ|PROT_WRITE,
+                            MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE|MAP_STACK,
+                            -1);
+
+  xmprotect (desc->alloc_base, guardsize, PROT_NONE);
+  xmprotect (desc->alloc_base + guardsize + stacksize, guardsize, PROT_NONE);
+  memset (desc->alloc_base + guardsize, 0xA5, stacksize);
+
+  desc->alt_stack.ss_sp    = desc->alloc_base + guardsize;
+  desc->alt_stack.ss_flags = 0;
+  desc->alt_stack.ss_size  = stacksize;
+
+  if (sigaltstack (&desc->alt_stack, &desc->old_stack))
+    FAIL_EXIT1 ("sigaltstack (new stack: sp=%p, size=%zu, flags=%u): %m\n",
+                desc->alt_stack.ss_sp, desc->alt_stack.ss_size,
+                desc->alt_stack.ss_flags);
+
+  return desc;
+}
+
+void
+xfree_sigstack (void *stack)
+{
+  struct sigstack_desc *desc = stack;
+
+  if (sigaltstack (&desc->old_stack, 0))
+    FAIL_EXIT1 ("sigaltstack (restore old stack: sp=%p, size=%zu, flags=%u): "
+                "%m\n", desc->old_stack.ss_sp, desc->old_stack.ss_size,
+                desc->old_stack.ss_flags);
+  xmunmap (desc->alloc_base, desc->alloc_size);
+  free (desc);
+}
+
+void
+xget_sigstack_location (const void *stack, unsigned char **addrp, size_t *sizep)
+{
+  const struct sigstack_desc *desc = stack;
+  *addrp = desc->alt_stack.ss_sp;
+  *sizep = desc->alt_stack.ss_size;
+}
-- 
2.20.1

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Carlos O'Donell]

On 1/15/19 3:05 PM, Zack Weinberg wrote:
> We don’t have a full conclusion about the way forward for alternate
> signal stacks yet, but the notion that the very short list of things
> that ISO C says you can do in an async signal handler should all work
> in MINSIGSTKSZ space seemed to be well-received.  As a first step,
> therefore, I propose to add tests to make sure those things do work.
> 
> To facilitate this, there is a new set of test support routines for
> setting up alternate signal stacks; see support/xsignal.h for the API.
> 
> All of these tests pass on x86-64.  build-many-glibcs won’t tell us
> anything additional; the tests need to be run.

Thank you for doing this.

OK for master with comment addition in implementation.

You need Siddhesh to approve the extra tests.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

>          * support/xsignal.h (xalloc_sigstack, xfree_sigstack)
>          (xget_sigstack_location): New test support functions.
>          * support/xsigstack.c: New file, implementing them.
>          * support/tst-xsigstack.c: New test for them.
>          * support/Makefile: Update.
> 
>          * signal/tst-minsigstksz-1.c
>          * signal/tst-minsigstksz-2.c
>          * signal/tst-minsigstksz-3.c
>          * signal/tst-minsigstksz-3a.c
>          * signal/tst-minsigstksz-4.c: New tests.
>          * signal/Makefile: Run them.
> ---
>  signal/Makefile             |   2 +
>  signal/tst-minsigstksz-1.c  | 131 ++++++++++++++++++++++++++++++++++++
>  signal/tst-minsigstksz-2.c  |  66 ++++++++++++++++++
>  signal/tst-minsigstksz-3.c  |  64 ++++++++++++++++++
>  signal/tst-minsigstksz-3a.c |  69 +++++++++++++++++++
>  signal/tst-minsigstksz-4.c  |  65 ++++++++++++++++++
>  support/Makefile            |   2 +
>  support/tst-xsigstack.c     |  64 ++++++++++++++++++
>  support/xsignal.h           |  18 +++++
>  support/xsigstack.c         |  95 ++++++++++++++++++++++++++
>  10 files changed, 576 insertions(+)
>  create mode 100644 signal/tst-minsigstksz-1.c
>  create mode 100644 signal/tst-minsigstksz-2.c
>  create mode 100644 signal/tst-minsigstksz-3.c
>  create mode 100644 signal/tst-minsigstksz-3a.c
>  create mode 100644 signal/tst-minsigstksz-4.c
>  create mode 100644 support/tst-xsigstack.c
>  create mode 100644 support/xsigstack.c
> 
> diff --git a/signal/Makefile b/signal/Makefile
> index aa63434f47..d5a10d590d 100644
> --- a/signal/Makefile
> +++ b/signal/Makefile
> @@ -47,6 +47,8 @@ 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 \

OK.

>  
>  include ../Rules
>  
> diff --git a/signal/tst-minsigstksz-1.c b/signal/tst-minsigstksz-1.c
> new file mode 100644
> index 0000000000..00344d5fbf
> --- /dev/null
> +++ b/signal/tst-minsigstksz-1.c
> @@ -0,0 +1,131 @@
> +/* Tests of signal delivery on an alternate stack (nonlethal).
> +   Copyright (C) 2019 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/check.h>
> +
> +/* C2011 7.4.1.1p5 specifies that only the following operations are
> +   guaranteed to be well-defined inside an asynchronous signal handler:
> +     * any operation on a lock-free atomic object
> +     * assigning a value to an object declared as volatile sig_atomic_t
> +     * calling abort, _Exit, quick_exit, or signal
> +       * signal may only be called with its first argument equal to the
> +         number of the signal that caused the handler to be called
> +
> +   We use this list as a guideline for the set of operations that ought
> +   also to be safe in a _synchronous_ signal delivered on an alternate
> +   signal stack with only MINSIGSTKSZ bytes of space.
> +
> +   This test program tests all of the above operations that do not,
> +   one way or another, cause the program to be terminated.  */

OK. Agreed.

> +
> +/* We do not try to test atomic operations exhaustively, only a simple
> +   atomic counter increment.  This is only safe if atomic_[u]int is
> +   unconditionally lock-free.  */
> +#ifdef __STDC_NO_ATOMICS__
> +# define TEST_ATOMIC_OPS 0
> +#else
> +# include <stdatomic.h>
> +# if ATOMIC_INT_LOCK_FREE != 2
> +#  define TEST_ATOMIC_OPS 0
> +# else
> +#  define TEST_ATOMIC_OPS 1
> +# endif
> +#endif
> +
> +static volatile sig_atomic_t signal_flag = 0;
> +static volatile sig_atomic_t signal_err = 0;
> +static void
> +handler_set_flag (int unused)
> +{
> +  signal_flag = 1;
> +}

OK.

> +
> +static void
> +handler_set_flag_once (int sig)
> +{
> +  signal_flag = 1;
> +  if (signal (sig, SIG_IGN) == SIG_ERR)
> +    /* It is not safe to call FAIL_EXIT1 here.  Set another flag instead.  */
> +    signal_err = 1;
> +}

OK.

> +
> +#if TEST_ATOMIC_OPS
> +static atomic_uint signal_count = 0;
> +static void
> +handler_count_up_1 (int unused)
> +{
> +  atomic_fetch_add (&signal_count, 1);

OK.

> +}
> +#endif
> +
> +int
> +do_test (void)
> +{
> +  void *sstk = xalloc_sigstack (0);

OK.

> +  struct sigaction sa;
> +
> +  /* Test 1: setting a volatile sig_atomic_t flag.  */
> +  sa.sa_handler = handler_set_flag;
> +  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
> +  sigfillset (&sa.sa_mask);
> +  if (sigaction (SIGUSR1, &sa, 0))
> +    FAIL_EXIT1 ("sigaction (SIGUSR1, handler_set_flag): %m\n");
> +
> +  TEST_VERIFY_EXIT (signal_flag == 0);
> +  raise (SIGUSR1);
> +  TEST_VERIFY_EXIT (signal_flag == 1);
> +  signal_flag = 0;
> +  raise (SIGUSR1);
> +  TEST_VERIFY_EXIT (signal_flag == 1);
> +  signal_flag = 0;

OK. Minimal test.

> +
> +  /* Test 1: setting a volatile sig_atomic_t flag and then ignoring
> +     further delivery of the signal. */
> +  sa.sa_handler = handler_set_flag_once;
> +  if (sigaction (SIGUSR1, &sa, 0))
> +    FAIL_EXIT1 ("sigaction (SIGUSR1, handler_set_flag_once): %m\n");
> +
> +  raise (SIGUSR1);
> +  TEST_VERIFY_EXIT (signal_flag == 1);
> +  /* Note: if signal_err is 1, a system call failed, but we can't
> +     report the error code because errno is indeterminate.  */
> +  TEST_VERIFY_EXIT (signal_err == 0);
> +
> +  signal_flag = 0;
> +  raise (SIGUSR1);
> +  TEST_VERIFY_EXIT (signal_flag == 0);
> +  TEST_VERIFY_EXIT (signal_err == 0);
> +
> +#if TEST_ATOMIC_OPS
> +  sa.sa_handler = handler_count_up_1;
> +  if (sigaction (SIGUSR1, &sa, 0))
> +    FAIL_EXIT1 ("sigaction (SIGUSR1, handler_count_up_1): %m\n");
> +
> +  raise (SIGUSR1);
> +  TEST_VERIFY_EXIT (atomic_load (&signal_count) == 1);
> +  raise (SIGUSR1);
> +  TEST_VERIFY_EXIT (atomic_load (&signal_count) == 2);

OK.

> +#endif
> +
> +  xfree_sigstack (sstk);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/signal/tst-minsigstksz-2.c b/signal/tst-minsigstksz-2.c
> new file mode 100644
> index 0000000000..3368dde6b8
> --- /dev/null
> +++ b/signal/tst-minsigstksz-2.c
> @@ -0,0 +1,66 @@
> +/* Tests of signal delivery on an alternate stack (abort).

OK.

> +   Copyright (C) 2019 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/check.h>
> +#include <stdlib.h>
> +
> +/* C2011 7.4.1.1p5 specifies that only the following operations are
> +   guaranteed to be well-defined inside an asynchronous signal handler:
> +     * any operation on a lock-free atomic object
> +     * assigning a value to an object declared as volatile sig_atomic_t
> +     * calling abort, _Exit, quick_exit, or signal
> +       * signal may only be called with its first argument equal to the
> +         number of the signal that caused the handler to be called
> +
> +   We use this list as a guideline for the set of operations that ought
> +   also to be safe in a _synchronous_ signal delivered on an alternate
> +   signal stack with only MINSIGSTKSZ bytes of space.
> +
> +   This test program tests calls to abort.  Note that it does _not_
> +   install a handler for SIGABRT, because that signal would also be
> +   delivered on the alternate stack and MINSIGSTKSZ does not provide
> +   enough space for delivery of nested signals.  */

OK. I'm not sure if it's explicitly spelled out that the default disposition
for SIGABRT is to core, but I doubt that will change today. This test should
work as intended and if the default is not to core or terminate then you'll
notice the test failing.

> +
> +static void
> +handler (int unused)
> +{
> +  abort ();
> +}
> +
> +int
> +do_test (void)
> +{
> +  void *sstk = xalloc_sigstack (0);
> +  struct sigaction sa;
> +
> +  sa.sa_handler = handler;
> +  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
> +  sigfillset (&sa.sa_mask);
> +  if (sigaction (SIGUSR1, &sa, 0))
> +    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
> +
> +  raise (SIGUSR1);

OK.

> +
> +  xfree_sigstack (sstk);
> +  FAIL_RET ("test process was not terminated by abort in signal handler");
> +}
> +
> +#define EXPECTED_SIGNAL SIGABRT
> +#include <support/test-driver.c>
> diff --git a/signal/tst-minsigstksz-3.c b/signal/tst-minsigstksz-3.c
> new file mode 100644
> index 0000000000..a8d9a6369c
> --- /dev/null
> +++ b/signal/tst-minsigstksz-3.c
> @@ -0,0 +1,64 @@
> +/* Tests of signal delivery on an alternate stack (_Exit).

OK.

> +   Copyright (C) 2019 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/check.h>
> +#include <stdlib.h>
> +
> +/* C2011 7.4.1.1p5 specifies that only the following operations are
> +   guaranteed to be well-defined inside an asynchronous signal handler:
> +     * any operation on a lock-free atomic object
> +     * assigning a value to an object declared as volatile sig_atomic_t
> +     * calling abort, _Exit, quick_exit, or signal
> +       * signal may only be called with its first argument equal to the
> +         number of the signal that caused the handler to be called
> +
> +   We use this list as a guideline for the set of operations that ought
> +   also to be safe in a _synchronous_ signal delivered on an alternate
> +   signal stack with only MINSIGSTKSZ bytes of space.
> +
> +   This test program tests calls to _Exit.  */

OK.

> +
> +#define EXPECTED_STATUS 3
> +
> +static void
> +handler (int unused)
> +{
> +  _Exit (EXPECTED_STATUS);

OK. Good use an odd exist status of 3.

> +}
> +
> +int
> +do_test (void)
> +{
> +  void *sstk = xalloc_sigstack (0);
> +  struct sigaction sa;
> +
> +  sa.sa_handler = handler;
> +  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
> +  sigfillset (&sa.sa_mask);
> +  if (sigaction (SIGUSR1, &sa, 0))
> +    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
> +
> +  raise (SIGUSR1);
> +
> +  xfree_sigstack (sstk);
> +  FAIL_RET ("test process was not terminated by _Exit in signal handler");
> +}
> +
> +#include <support/test-driver.c>

OK.

> diff --git a/signal/tst-minsigstksz-3a.c b/signal/tst-minsigstksz-3a.c
> new file mode 100644
> index 0000000000..b58b8d01ba
> --- /dev/null
> +++ b/signal/tst-minsigstksz-3a.c
> @@ -0,0 +1,69 @@
> +/* Tests of signal delivery on an alternate stack (_exit).

OK. Nice to use 3 and 3a for both _exit/_Exit since they are going to be
the same, but it's OK to test them distinctly.

> +   Copyright (C) 2019 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/check.h>
> +#include <unistd.h>
> +
> +/* C2011 7.4.1.1p5 specifies that only the following operations are
> +   guaranteed to be well-defined inside an asynchronous signal handler:
> +     * any operation on a lock-free atomic object
> +     * assigning a value to an object declared as volatile sig_atomic_t
> +     * calling abort, _Exit, quick_exit, or signal
> +       * signal may only be called with its first argument equal to the
> +         number of the signal that caused the handler to be called
> +
> +   We use this list as a guideline for the set of operations that ought
> +   also to be safe in a _synchronous_ signal delivered on an alternate
> +   signal stack with only MINSIGSTKSZ bytes of space.
> +
> +   This test program tests calls to _exit, which is the same function
> +   as _Exit, but specified by POSIX rather than ISO C.  For reasons
> +   unknown to the author of this program, the C committee did not
> +   think it could standardize _exit under that name; regardless, in a
> +   POSIX-conformant environment, they should be completely
> +   interchangeable.  */

OK. Agreed.

> +
> +#define EXPECTED_STATUS 3

OK.

> +
> +static void
> +handler (int unused)
> +{
> +  _exit (EXPECTED_STATUS);

OK.

> +}
> +
> +int
> +do_test (void)
> +{
> +  void *sstk = xalloc_sigstack (0);
> +  struct sigaction sa;
> +
> +  sa.sa_handler = handler;
> +  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
> +  sigfillset (&sa.sa_mask);
> +  if (sigaction (SIGUSR1, &sa, 0))
> +    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
> +
> +  raise (SIGUSR1);
> +
> +  xfree_sigstack (sstk);
> +  FAIL_RET ("test process was not terminated by _exit in signal handler");
> +}

OK.

> +
> +#include <support/test-driver.c>
> diff --git a/signal/tst-minsigstksz-4.c b/signal/tst-minsigstksz-4.c
> new file mode 100644
> index 0000000000..0dc63b4dd4
> --- /dev/null
> +++ b/signal/tst-minsigstksz-4.c
> @@ -0,0 +1,65 @@
> +/* Tests of signal delivery on an alternate stack (quick_exit).

OK.

> +   Copyright (C) 2019 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/check.h>
> +#include <stdlib.h>
> +
> +/* C2011 7.4.1.1p5 specifies that only the following operations are
> +   guaranteed to be well-defined inside an asynchronous signal handler:
> +     * any operation on a lock-free atomic object
> +     * assigning a value to an object declared as volatile sig_atomic_t
> +     * calling abort, _Exit, quick_exit, or signal
> +       * signal may only be called with its first argument equal to the
> +         number of the signal that caused the handler to be called
> +
> +   We use this list as a guideline for the set of operations that ought
> +   also to be safe in a _synchronous_ signal delivered on an alternate
> +   signal stack with only MINSIGSTKSZ bytes of space.
> +
> +   This test program tests calls to quick_exit.  Note that this is only
> +   safe when there are no at_quick_exit callbacks.  */

OK.

> +
> +#define EXPECTED_STATUS 3
> +
> +static void
> +handler (int unused)
> +{
> +  quick_exit (EXPECTED_STATUS);

OK.

> +}
> +
> +int
> +do_test (void)
> +{
> +  void *sstk = xalloc_sigstack (0);
> +  struct sigaction sa;
> +
> +  sa.sa_handler = handler;
> +  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
> +  sigfillset (&sa.sa_mask);
> +  if (sigaction (SIGUSR1, &sa, 0))
> +    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
> +
> +  raise (SIGUSR1);
> +
> +  xfree_sigstack (sstk);
> +  FAIL_RET ("test process was not terminated by quick_exit in signal handler");

OK.

> +}
> +
> +#include <support/test-driver.c>
> diff --git a/support/Makefile b/support/Makefile
> index 93a5143016..7fb2de8cee 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -145,6 +145,7 @@ libsupport-routines = \
>    xsetsockopt \
>    xsigaction \
>    xsignal \
> +  xsigstack \

OK.

>    xsocket \
>    xstrdup \
>    xstrndup \
> @@ -205,6 +206,7 @@ tests = \
>    tst-test_compare_blob \
>    tst-test_compare_string \
>    tst-xreadlink \
> +  tst-xsigstack \

OK.

>  
>  ifeq ($(run-built-tests),yes)
>  tests-special = \
> diff --git a/support/tst-xsigstack.c b/support/tst-xsigstack.c
> new file mode 100644
> index 0000000000..42859c79e9
> --- /dev/null
> +++ b/support/tst-xsigstack.c
> @@ -0,0 +1,64 @@
> +/* Test of sigaltstack wrappers.

OK.

> +   Copyright (C) 2019 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/xunistd.h>
> +#include <support/check.h>
> +
> +#include <stdint.h>
> +#include <stdio.h>
> +
> +static volatile uintptr_t handler_stackaddr;
> +
> +static void
> +handler (int unused)
> +{
> +  int var;
> +  handler_stackaddr = (uintptr_t) &var;
> +}
> +
> +int
> +do_test (void)
> +{
> +  void *sstk = xalloc_sigstack (0);

OK.

> +
> +  unsigned char *sp;
> +  size_t size;
> +  xget_sigstack_location (sstk, &sp, &size);

OK.

> +  printf ("signal stack installed: sp=%p size=%zu\n", sp, size);
> +
> +  struct sigaction sa;
> +  sa.sa_handler = handler;
> +  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
> +  sigfillset (&sa.sa_mask);
> +  if (sigaction (SIGUSR1, &sa, 0))
> +    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
> +
> +  raise (SIGUSR1);
> +
> +  uintptr_t haddr = handler_stackaddr;
> +  printf ("address of handler local variable: %p\n", (void *)haddr);
> +  TEST_VERIFY ((uintptr_t)sp < haddr);

OK.

> +  TEST_VERIFY (haddr < (uintptr_t)sp + size);

OK. Probably the best we can do.

> +
> +  xfree_sigstack (sstk);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/support/xsignal.h b/support/xsignal.h
> index 9ab8d1bfdd..3cf2a6e434 100644
> --- a/support/xsignal.h
> +++ b/support/xsignal.h
> @@ -37,6 +37,24 @@ void xsigaction (int sig, const struct sigaction *newact,
>  
>  void xpthread_sigmask (int how, const sigset_t *set, sigset_t *oldset);
>  
> +/* Allocate and activate an alternate signal stack.  This stack will
> +   have at least MINSIGSTKSZ + size bytes of space, rounded up to a
> +   whole number of pages.  There will be large (at least 1 MiB)
> +   inaccessible guard bands on either side of it.  The return value is
> +   a cookie that can be passed to xfree_sigstack to deactivate and
> +   deallocate the stack again.
> +   It is _not_ necessary to call sigaltstack() after calling this function.
> +   Terminates the process on error.  */
> +void *xalloc_sigstack (size_t size);
> +
> +/* Deactivate and deallocate a signal stack created by xalloc_sigstack.  */
> +void xfree_sigstack (void *stack);
> +
> +/* Extract the actual address and size of the alternate signal stack from
> +   the cookie returned by xalloc_sigstack.  */
> +void xget_sigstack_location (const void *stack, unsigned char **addrp,
> +                             size_t *sizep);

OK.

> +
>  __END_DECLS
>  
>  #endif /* SUPPORT_SIGNAL_H */
> diff --git a/support/xsigstack.c b/support/xsigstack.c
> new file mode 100644
> index 0000000000..74b459e2e6
> --- /dev/null
> +++ b/support/xsigstack.c
> @@ -0,0 +1,95 @@
> +/* sigaltstack wrappers.
> +   Copyright (C) 2019 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/xunistd.h>
> +#include <support/check.h>
> +
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <sys/param.h> /* roundup, MAX */
> +
> +/* The "cookie" returned by xalloc_sigstack points to one of these
> +   structures.  */
> +struct sigstack_desc
> +{
> +  void *alloc_base;  /* Base address of the complete allocation.  */
> +  size_t alloc_size; /* Size of the complete allocation.  */
> +  stack_t alt_stack; /* The address and size of the stack itself.  */
> +  stack_t old_stack; /* The previous signal stack.  */
> +};

OK.

> +
> +void *
> +xalloc_sigstack (size_t size)
> +{
> +  size_t pagesize = sysconf (_SC_PAGESIZE);
> +  if (pagesize == -1)
> +    FAIL_EXIT1 ("sysconf (_SC_PAGESIZE): %m\n");
> +

Both of these choices need a comment explaining why or if they are
arbitrary, that they were just chosen at random.

/* Per the API we allocate MINSIGSTKSZ when xalloc_sigstack(0)
   is called.  */

> +  size_t stacksize = roundup (size + MINSIGSTKSZ, pagesize);

Suggest:

/* We want a large enough guard to catch any jumps around the guard
   page that might take us to another mapping. The choice of a 1MiB
   guard as a minimum is somewhat arbitrary, but if the stacksize is
   large we use that as the guard size on the assumption that an index
   into an array of a stack sized variable might be off by one and jump
   the entire stack.  */

> +  size_t guardsize = MAX (stacksize, roundup (1024 * 1024, pagesize));
> +
> +  struct sigstack_desc *desc = xmalloc (sizeof (struct sigstack_desc));
> +  desc->alloc_size = guardsize + stacksize + guardsize;
> +  /* Use MAP_NORESERVE so that RAM will not be wasted on the guard
> +     bands; touch all the pages of the actual stack before returning,
> +     so we know they are allocated.  */
> +  desc->alloc_base = xmmap (0,
> +                            desc->alloc_size,
> +                            PROT_READ|PROT_WRITE,
> +                            MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE|MAP_STACK,
> +                            -1);

OK.

> +
> +  xmprotect (desc->alloc_base, guardsize, PROT_NONE);
> +  xmprotect (desc->alloc_base + guardsize + stacksize, guardsize, PROT_NONE);
> +  memset (desc->alloc_base + guardsize, 0xA5, stacksize);

OK.

> +
> +  desc->alt_stack.ss_sp    = desc->alloc_base + guardsize;
> +  desc->alt_stack.ss_flags = 0;
> +  desc->alt_stack.ss_size  = stacksize;

OK.

> +
> +  if (sigaltstack (&desc->alt_stack, &desc->old_stack))
> +    FAIL_EXIT1 ("sigaltstack (new stack: sp=%p, size=%zu, flags=%u): %m\n",
> +                desc->alt_stack.ss_sp, desc->alt_stack.ss_size,
> +                desc->alt_stack.ss_flags);

OK.

> +
> +  return desc;
> +}
> +
> +void
> +xfree_sigstack (void *stack)
> +{
> +  struct sigstack_desc *desc = stack;
> +
> +  if (sigaltstack (&desc->old_stack, 0))
> +    FAIL_EXIT1 ("sigaltstack (restore old stack: sp=%p, size=%zu, flags=%u): "
> +                "%m\n", desc->old_stack.ss_sp, desc->old_stack.ss_size,
> +                desc->old_stack.ss_flags);
> +  xmunmap (desc->alloc_base, desc->alloc_size);

OK.

> +  free (desc);
> +}
> +
> +void
> +xget_sigstack_location (const void *stack, unsigned char **addrp, size_t *sizep)
> +{
> +  const struct sigstack_desc *desc = stack;
> +  *addrp = desc->alt_stack.ss_sp;
> +  *sizep = desc->alt_stack.ss_size;

OK.

> +}
> 


-- 
Cheers,
Carlos.

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Zack Weinberg]

On Tue, Jan 15, 2019 at 4:15 PM Carlos O'Donell <carlos@redhat.com> wrote:
> Both of these choices need a comment explaining why or if they are
> arbitrary, that they were just chosen at random.

How's this look?  You made me realize that the guards need to be
*twice* as big as a large stack to be guaranteed to catch an offset by
the entire size of a large stack-allocated array.

+void *
+xalloc_sigstack (size_t size)
+{
+  size_t pagesize = sysconf (_SC_PAGESIZE);
+  if (pagesize == -1)
+    FAIL_EXIT1 ("sysconf (_SC_PAGESIZE): %m\n");
+
+  /* Always supply at least MINSIGSTKSZ space; passing 0 as size means
+     only that much space.  No matter what the number is, round it up
+     to a whole number of pages.  */
+  size_t stacksize = roundup (size + MINSIGSTKSZ, pagesize);
+
+  /* The guard bands need to be large enough to intercept offset
+     accesses from a stack address that might otherwise hit another
+     mapping.  Make them at least twice as big as the stack itself, to
+     defend against an offset by the entire size of a large
+     stack-allocated array.  The minimum is 1MiB, which is arbitrarily
+     chosen to be larger than any "typical" wild pointer offset.
+     Again, no matter what the number is, round it up to a whole
+     number of pages.  */
+  size_t guardsize = roundup (MAX (2 * stacksize, 1024 * 1024), pagesize);

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Siddhesh Poyarekar]

On 16/01/19 2:45 AM, Carlos O'Donell wrote:
> Thank you for doing this.
> 
> OK for master with comment addition in implementation.
> 
> You need Siddhesh to approve the extra tests.
> 

OK to add these tests to master.

Siddhesh

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Carlos O'Donell]

On 1/15/19 5:16 PM, Zack Weinberg wrote:
> On Tue, Jan 15, 2019 at 4:15 PM Carlos O'Donell <carlos@redhat.com> wrote:
>> Both of these choices need a comment explaining why or if they are
>> arbitrary, that they were just chosen at random.
> 
> How's this look?  You made me realize that the guards need to be
> *twice* as big as a large stack to be guaranteed to catch an offset by
> the entire size of a large stack-allocated array.

Perfect. You document intent, and that's all I wanted.

> +void *
> +xalloc_sigstack (size_t size)
> +{
> +  size_t pagesize = sysconf (_SC_PAGESIZE);
> +  if (pagesize == -1)
> +    FAIL_EXIT1 ("sysconf (_SC_PAGESIZE): %m\n");
> +
> +  /* Always supply at least MINSIGSTKSZ space; passing 0 as size means
> +     only that much space.  No matter what the number is, round it up
> +     to a whole number of pages.  */
> +  size_t stacksize = roundup (size + MINSIGSTKSZ, pagesize);

OK.

> +
> +  /* The guard bands need to be large enough to intercept offset
> +     accesses from a stack address that might otherwise hit another
> +     mapping.  Make them at least twice as big as the stack itself, to
> +     defend against an offset by the entire size of a large
> +     stack-allocated array.  The minimum is 1MiB, which is arbitrarily
> +     chosen to be larger than any "typical" wild pointer offset.
> +     Again, no matter what the number is, round it up to a whole
> +     number of pages.  */
> +  size_t guardsize = roundup (MAX (2 * stacksize, 1024 * 1024), pagesize);
> 

OK.

-- 
Cheers,
Carlos.

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Zack Weinberg]

[-- Attachment #1: Type: text/plain, Size: 1889 bytes --]

The patch I just pushed is attached to this message.  Thanks for the
quick reviews.  A couple of notes:

> I'm not sure if it's explicitly spelled out that the default disposition
> for SIGABRT is to core

This is indeed specified in POSIX, see the table of default signal actions in
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html .

> +/* C2011 7.4.1.1p5 specifies that only the following operations are
> +   guaranteed to be well-defined inside an asynchronous signal handler:
> ...
> +     * calling abort, _Exit, quick_exit, or signal
> +       * signal may only be called with its first argument equal to the
> +         number of the signal that caused the handler to be called

It occurred to me this morning that signal is implemented using
sigaction, and a 'struct sigaction' is 152 bytes on x86-64 (because of
the sigset_t).  Normally that's nothing to worry about, but when
there's only 2048 bytes of stack in total and the kernel has filled
more than half of that with ucontext data, it _could_ be a problem.
I left the test as is for now, but I suggest that if it does crash on
any architecture we should drop that part of tst-minsigstksz-1.c
without hesitation.

It also occurs to me that on some architectures MINSIGSTKSZ is less
than a page; on those architectures, the rounding done in xsigstack.c
means we aren't _really_ testing this stuff in MINSIGSTKSZ space.
Since overflow is much more of a concern than underflow, what do
people think of adjusting the code in xsigstack.c so that the area
actually passed to sigaltstack will not be rounded and will be right
up against the guard in the direction of overflow?  This would mean
xsigstack.c has to know which direction the stack grows, but I think
we already have internal macros for that, so it's not a huge problem.
I would do this as a follow-up patch if it's agreed to be a good idea.

zw

[-- Attachment #2: 0001-Tests-for-minimal-signal-handler-functionality-in-MI.patch --]
[-- Type: text/x-patch, Size: 25989 bytes --]

From fbbc9a4e347dabb2d1662744e6a2e83b569ea3a4 Mon Sep 17 00:00:00 2001
From: Zack Weinberg <zackw@panix.com>
Date: Tue, 15 Jan 2019 14:58:15 -0500
Subject: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ
 space.

There is general agreement that the very short list of things that ISO
C says you can do in an async signal handler should all work when the
handler is running on an alternate signal stack with only MINSIGSTKSZ
space.  This patch adds tests to make sure those things do work.

To facilitate this, there is a new set of test support routines for
setting up alternate signal stacks; see support/xsignal.h for the API.

         * support/xsignal.h (xalloc_sigstack, xfree_sigstack)
         (xget_sigstack_location): New test support functions.
         * support/xsigstack.c: New file, implementing them.
         * support/tst-xsigstack.c: New test for them.
         * support/Makefile: Update.

         * signal/tst-minsigstksz-1.c
         * signal/tst-minsigstksz-2.c
         * signal/tst-minsigstksz-3.c
         * signal/tst-minsigstksz-3a.c
         * signal/tst-minsigstksz-4.c: New tests.
         * signal/Makefile: Run them.
---
 ChangeLog                   |  15 +++++
 signal/Makefile             |   2 +
 signal/tst-minsigstksz-1.c  | 131 ++++++++++++++++++++++++++++++++++++
 signal/tst-minsigstksz-2.c  |  66 ++++++++++++++++++
 signal/tst-minsigstksz-3.c  |  64 ++++++++++++++++++
 signal/tst-minsigstksz-3a.c |  69 +++++++++++++++++++
 signal/tst-minsigstksz-4.c  |  65 ++++++++++++++++++
 support/Makefile            |   2 +
 support/tst-xsigstack.c     |  64 ++++++++++++++++++
 support/xsignal.h           |  17 +++++
 support/xsigstack.c         | 107 +++++++++++++++++++++++++++++
 11 files changed, 602 insertions(+)
 create mode 100644 signal/tst-minsigstksz-1.c
 create mode 100644 signal/tst-minsigstksz-2.c
 create mode 100644 signal/tst-minsigstksz-3.c
 create mode 100644 signal/tst-minsigstksz-3a.c
 create mode 100644 signal/tst-minsigstksz-4.c
 create mode 100644 support/tst-xsigstack.c
 create mode 100644 support/xsigstack.c

diff --git a/ChangeLog b/ChangeLog
index 36bb3ed885..7cc7916a17 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2019-01-16  Zack Weinberg  <zackw@panix.com>
+
+	 * support/xsignal.h (xalloc_sigstack, xfree_sigstack)
+	 (xget_sigstack_location): New test support functions.
+	 * support/xsigstack.c: New file, implementing them.
+	 * support/tst-xsigstack.c: New test for them.
+	 * support/Makefile: Update.
+
+	 * signal/tst-minsigstksz-1.c
+	 * signal/tst-minsigstksz-2.c
+	 * signal/tst-minsigstksz-3.c
+	 * signal/tst-minsigstksz-3a.c
+	 * signal/tst-minsigstksz-4.c: New tests.
+	 * signal/Makefile: Run them.
+
 2019-01-16  Siddhesh Poyarekar  <siddhesh@sourceware.org>
 
 	* po/libc.pot: Regenerate.
diff --git a/signal/Makefile b/signal/Makefile
index 647ce242e6..9597287bca 100644
--- a/signal/Makefile
+++ b/signal/Makefile
@@ -47,6 +47,8 @@ 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 \
 
 include ../Rules
 
diff --git a/signal/tst-minsigstksz-1.c b/signal/tst-minsigstksz-1.c
new file mode 100644
index 0000000000..00344d5fbf
--- /dev/null
+++ b/signal/tst-minsigstksz-1.c
@@ -0,0 +1,131 @@
+/* Tests of signal delivery on an alternate stack (nonlethal).
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/check.h>
+
+/* C2011 7.4.1.1p5 specifies that only the following operations are
+   guaranteed to be well-defined inside an asynchronous signal handler:
+     * any operation on a lock-free atomic object
+     * assigning a value to an object declared as volatile sig_atomic_t
+     * calling abort, _Exit, quick_exit, or signal
+       * signal may only be called with its first argument equal to the
+         number of the signal that caused the handler to be called
+
+   We use this list as a guideline for the set of operations that ought
+   also to be safe in a _synchronous_ signal delivered on an alternate
+   signal stack with only MINSIGSTKSZ bytes of space.
+
+   This test program tests all of the above operations that do not,
+   one way or another, cause the program to be terminated.  */
+
+/* We do not try to test atomic operations exhaustively, only a simple
+   atomic counter increment.  This is only safe if atomic_[u]int is
+   unconditionally lock-free.  */
+#ifdef __STDC_NO_ATOMICS__
+# define TEST_ATOMIC_OPS 0
+#else
+# include <stdatomic.h>
+# if ATOMIC_INT_LOCK_FREE != 2
+#  define TEST_ATOMIC_OPS 0
+# else
+#  define TEST_ATOMIC_OPS 1
+# endif
+#endif
+
+static volatile sig_atomic_t signal_flag = 0;
+static volatile sig_atomic_t signal_err = 0;
+static void
+handler_set_flag (int unused)
+{
+  signal_flag = 1;
+}
+
+static void
+handler_set_flag_once (int sig)
+{
+  signal_flag = 1;
+  if (signal (sig, SIG_IGN) == SIG_ERR)
+    /* It is not safe to call FAIL_EXIT1 here.  Set another flag instead.  */
+    signal_err = 1;
+}
+
+#if TEST_ATOMIC_OPS
+static atomic_uint signal_count = 0;
+static void
+handler_count_up_1 (int unused)
+{
+  atomic_fetch_add (&signal_count, 1);
+}
+#endif
+
+int
+do_test (void)
+{
+  void *sstk = xalloc_sigstack (0);
+  struct sigaction sa;
+
+  /* Test 1: setting a volatile sig_atomic_t flag.  */
+  sa.sa_handler = handler_set_flag;
+  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
+  sigfillset (&sa.sa_mask);
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_EXIT1 ("sigaction (SIGUSR1, handler_set_flag): %m\n");
+
+  TEST_VERIFY_EXIT (signal_flag == 0);
+  raise (SIGUSR1);
+  TEST_VERIFY_EXIT (signal_flag == 1);
+  signal_flag = 0;
+  raise (SIGUSR1);
+  TEST_VERIFY_EXIT (signal_flag == 1);
+  signal_flag = 0;
+
+  /* Test 1: setting a volatile sig_atomic_t flag and then ignoring
+     further delivery of the signal. */
+  sa.sa_handler = handler_set_flag_once;
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_EXIT1 ("sigaction (SIGUSR1, handler_set_flag_once): %m\n");
+
+  raise (SIGUSR1);
+  TEST_VERIFY_EXIT (signal_flag == 1);
+  /* Note: if signal_err is 1, a system call failed, but we can't
+     report the error code because errno is indeterminate.  */
+  TEST_VERIFY_EXIT (signal_err == 0);
+
+  signal_flag = 0;
+  raise (SIGUSR1);
+  TEST_VERIFY_EXIT (signal_flag == 0);
+  TEST_VERIFY_EXIT (signal_err == 0);
+
+#if TEST_ATOMIC_OPS
+  sa.sa_handler = handler_count_up_1;
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_EXIT1 ("sigaction (SIGUSR1, handler_count_up_1): %m\n");
+
+  raise (SIGUSR1);
+  TEST_VERIFY_EXIT (atomic_load (&signal_count) == 1);
+  raise (SIGUSR1);
+  TEST_VERIFY_EXIT (atomic_load (&signal_count) == 2);
+#endif
+
+  xfree_sigstack (sstk);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/signal/tst-minsigstksz-2.c b/signal/tst-minsigstksz-2.c
new file mode 100644
index 0000000000..3368dde6b8
--- /dev/null
+++ b/signal/tst-minsigstksz-2.c
@@ -0,0 +1,66 @@
+/* Tests of signal delivery on an alternate stack (abort).
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/check.h>
+#include <stdlib.h>
+
+/* C2011 7.4.1.1p5 specifies that only the following operations are
+   guaranteed to be well-defined inside an asynchronous signal handler:
+     * any operation on a lock-free atomic object
+     * assigning a value to an object declared as volatile sig_atomic_t
+     * calling abort, _Exit, quick_exit, or signal
+       * signal may only be called with its first argument equal to the
+         number of the signal that caused the handler to be called
+
+   We use this list as a guideline for the set of operations that ought
+   also to be safe in a _synchronous_ signal delivered on an alternate
+   signal stack with only MINSIGSTKSZ bytes of space.
+
+   This test program tests calls to abort.  Note that it does _not_
+   install a handler for SIGABRT, because that signal would also be
+   delivered on the alternate stack and MINSIGSTKSZ does not provide
+   enough space for delivery of nested signals.  */
+
+static void
+handler (int unused)
+{
+  abort ();
+}
+
+int
+do_test (void)
+{
+  void *sstk = xalloc_sigstack (0);
+  struct sigaction sa;
+
+  sa.sa_handler = handler;
+  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
+  sigfillset (&sa.sa_mask);
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
+
+  raise (SIGUSR1);
+
+  xfree_sigstack (sstk);
+  FAIL_RET ("test process was not terminated by abort in signal handler");
+}
+
+#define EXPECTED_SIGNAL SIGABRT
+#include <support/test-driver.c>
diff --git a/signal/tst-minsigstksz-3.c b/signal/tst-minsigstksz-3.c
new file mode 100644
index 0000000000..a8d9a6369c
--- /dev/null
+++ b/signal/tst-minsigstksz-3.c
@@ -0,0 +1,64 @@
+/* Tests of signal delivery on an alternate stack (_Exit).
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/check.h>
+#include <stdlib.h>
+
+/* C2011 7.4.1.1p5 specifies that only the following operations are
+   guaranteed to be well-defined inside an asynchronous signal handler:
+     * any operation on a lock-free atomic object
+     * assigning a value to an object declared as volatile sig_atomic_t
+     * calling abort, _Exit, quick_exit, or signal
+       * signal may only be called with its first argument equal to the
+         number of the signal that caused the handler to be called
+
+   We use this list as a guideline for the set of operations that ought
+   also to be safe in a _synchronous_ signal delivered on an alternate
+   signal stack with only MINSIGSTKSZ bytes of space.
+
+   This test program tests calls to _Exit.  */
+
+#define EXPECTED_STATUS 3
+
+static void
+handler (int unused)
+{
+  _Exit (EXPECTED_STATUS);
+}
+
+int
+do_test (void)
+{
+  void *sstk = xalloc_sigstack (0);
+  struct sigaction sa;
+
+  sa.sa_handler = handler;
+  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
+  sigfillset (&sa.sa_mask);
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
+
+  raise (SIGUSR1);
+
+  xfree_sigstack (sstk);
+  FAIL_RET ("test process was not terminated by _Exit in signal handler");
+}
+
+#include <support/test-driver.c>
diff --git a/signal/tst-minsigstksz-3a.c b/signal/tst-minsigstksz-3a.c
new file mode 100644
index 0000000000..b58b8d01ba
--- /dev/null
+++ b/signal/tst-minsigstksz-3a.c
@@ -0,0 +1,69 @@
+/* Tests of signal delivery on an alternate stack (_exit).
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/check.h>
+#include <unistd.h>
+
+/* C2011 7.4.1.1p5 specifies that only the following operations are
+   guaranteed to be well-defined inside an asynchronous signal handler:
+     * any operation on a lock-free atomic object
+     * assigning a value to an object declared as volatile sig_atomic_t
+     * calling abort, _Exit, quick_exit, or signal
+       * signal may only be called with its first argument equal to the
+         number of the signal that caused the handler to be called
+
+   We use this list as a guideline for the set of operations that ought
+   also to be safe in a _synchronous_ signal delivered on an alternate
+   signal stack with only MINSIGSTKSZ bytes of space.
+
+   This test program tests calls to _exit, which is the same function
+   as _Exit, but specified by POSIX rather than ISO C.  For reasons
+   unknown to the author of this program, the C committee did not
+   think it could standardize _exit under that name; regardless, in a
+   POSIX-conformant environment, they should be completely
+   interchangeable.  */
+
+#define EXPECTED_STATUS 3
+
+static void
+handler (int unused)
+{
+  _exit (EXPECTED_STATUS);
+}
+
+int
+do_test (void)
+{
+  void *sstk = xalloc_sigstack (0);
+  struct sigaction sa;
+
+  sa.sa_handler = handler;
+  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
+  sigfillset (&sa.sa_mask);
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
+
+  raise (SIGUSR1);
+
+  xfree_sigstack (sstk);
+  FAIL_RET ("test process was not terminated by _exit in signal handler");
+}
+
+#include <support/test-driver.c>
diff --git a/signal/tst-minsigstksz-4.c b/signal/tst-minsigstksz-4.c
new file mode 100644
index 0000000000..0dc63b4dd4
--- /dev/null
+++ b/signal/tst-minsigstksz-4.c
@@ -0,0 +1,65 @@
+/* Tests of signal delivery on an alternate stack (quick_exit).
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/check.h>
+#include <stdlib.h>
+
+/* C2011 7.4.1.1p5 specifies that only the following operations are
+   guaranteed to be well-defined inside an asynchronous signal handler:
+     * any operation on a lock-free atomic object
+     * assigning a value to an object declared as volatile sig_atomic_t
+     * calling abort, _Exit, quick_exit, or signal
+       * signal may only be called with its first argument equal to the
+         number of the signal that caused the handler to be called
+
+   We use this list as a guideline for the set of operations that ought
+   also to be safe in a _synchronous_ signal delivered on an alternate
+   signal stack with only MINSIGSTKSZ bytes of space.
+
+   This test program tests calls to quick_exit.  Note that this is only
+   safe when there are no at_quick_exit callbacks.  */
+
+#define EXPECTED_STATUS 3
+
+static void
+handler (int unused)
+{
+  quick_exit (EXPECTED_STATUS);
+}
+
+int
+do_test (void)
+{
+  void *sstk = xalloc_sigstack (0);
+  struct sigaction sa;
+
+  sa.sa_handler = handler;
+  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
+  sigfillset (&sa.sa_mask);
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
+
+  raise (SIGUSR1);
+
+  xfree_sigstack (sstk);
+  FAIL_RET ("test process was not terminated by quick_exit in signal handler");
+}
+
+#include <support/test-driver.c>
diff --git a/support/Makefile b/support/Makefile
index 6ac4447def..432cf2fe6c 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -145,6 +145,7 @@ libsupport-routines = \
   xsetsockopt \
   xsigaction \
   xsignal \
+  xsigstack \
   xsocket \
   xstrdup \
   xstrndup \
@@ -205,6 +206,7 @@ tests = \
   tst-test_compare_blob \
   tst-test_compare_string \
   tst-xreadlink \
+  tst-xsigstack \
 
 ifeq ($(run-built-tests),yes)
 tests-special = \
diff --git a/support/tst-xsigstack.c b/support/tst-xsigstack.c
new file mode 100644
index 0000000000..42859c79e9
--- /dev/null
+++ b/support/tst-xsigstack.c
@@ -0,0 +1,64 @@
+/* Test of sigaltstack wrappers.
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/xunistd.h>
+#include <support/check.h>
+
+#include <stdint.h>
+#include <stdio.h>
+
+static volatile uintptr_t handler_stackaddr;
+
+static void
+handler (int unused)
+{
+  int var;
+  handler_stackaddr = (uintptr_t) &var;
+}
+
+int
+do_test (void)
+{
+  void *sstk = xalloc_sigstack (0);
+
+  unsigned char *sp;
+  size_t size;
+  xget_sigstack_location (sstk, &sp, &size);
+  printf ("signal stack installed: sp=%p size=%zu\n", sp, size);
+
+  struct sigaction sa;
+  sa.sa_handler = handler;
+  sa.sa_flags   = SA_RESTART | SA_ONSTACK;
+  sigfillset (&sa.sa_mask);
+  if (sigaction (SIGUSR1, &sa, 0))
+    FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
+
+  raise (SIGUSR1);
+
+  uintptr_t haddr = handler_stackaddr;
+  printf ("address of handler local variable: %p\n", (void *)haddr);
+  TEST_VERIFY ((uintptr_t)sp < haddr);
+  TEST_VERIFY (haddr < (uintptr_t)sp + size);
+
+  xfree_sigstack (sstk);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/support/xsignal.h b/support/xsignal.h
index f3308883a4..d89e2bc575 100644
--- a/support/xsignal.h
+++ b/support/xsignal.h
@@ -37,6 +37,23 @@ void xsigaction (int sig, const struct sigaction *newact,
 
 void xpthread_sigmask (int how, const sigset_t *set, sigset_t *oldset);
 
+/* Allocate and activate an alternate signal stack.  This stack will
+   have SIZE + MINSIGSTKSZ bytes of space, rounded up to a whole
+   number of pages.  There will be large (at least 1 MiB) inaccessible
+   guard bands on either side of it.  The return value is a cookie
+   that can be passed to xfree_sigstack to deactivate and deallocate
+   the stack again.  It is not necessary to call sigaltstack after
+   calling this function.  Terminates the process on error.  */
+void *xalloc_sigstack (size_t size);
+
+/* Deactivate and deallocate a signal stack created by xalloc_sigstack.  */
+void xfree_sigstack (void *stack);
+
+/* Extract the actual address and size of the alternate signal stack from
+   the cookie returned by xalloc_sigstack.  */
+void xget_sigstack_location (const void *stack, unsigned char **addrp,
+                             size_t *sizep);
+
 __END_DECLS
 
 #endif /* SUPPORT_SIGNAL_H */
diff --git a/support/xsigstack.c b/support/xsigstack.c
new file mode 100644
index 0000000000..cebfa19aa5
--- /dev/null
+++ b/support/xsigstack.c
@@ -0,0 +1,107 @@
+/* sigaltstack wrappers.
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/support.h>
+#include <support/xunistd.h>
+#include <support/check.h>
+
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/param.h> /* roundup, MAX */
+
+/* The "cookie" returned by xalloc_sigstack points to one of these
+   structures.  */
+struct sigstack_desc
+{
+  void *alloc_base;  /* Base address of the complete allocation.  */
+  size_t alloc_size; /* Size of the complete allocation.  */
+  stack_t alt_stack; /* The address and size of the stack itself.  */
+  stack_t old_stack; /* The previous signal stack.  */
+};
+
+void *
+xalloc_sigstack (size_t size)
+{
+  size_t pagesize = sysconf (_SC_PAGESIZE);
+  if (pagesize == -1)
+    FAIL_EXIT1 ("sysconf (_SC_PAGESIZE): %m\n");
+
+  /* Always supply at least MINSIGSTKSZ space; passing 0 as size means
+     only that much space.  No matter what the number is, round it up
+     to a whole number of pages.  */
+  size_t stacksize = roundup (size + MINSIGSTKSZ, pagesize);
+
+  /* The guard bands need to be large enough to intercept offset
+     accesses from a stack address that might otherwise hit another
+     mapping.  Make them at least twice as big as the stack itself, to
+     defend against an offset by the entire size of a large
+     stack-allocated array.  The minimum is 1MiB, which is arbitrarily
+     chosen to be larger than any "typical" wild pointer offset.
+     Again, no matter what the number is, round it up to a whole
+     number of pages.  */
+  size_t guardsize = roundup (MAX (2 * stacksize, 1024 * 1024), pagesize);
+
+  struct sigstack_desc *desc = xmalloc (sizeof (struct sigstack_desc));
+  desc->alloc_size = guardsize + stacksize + guardsize;
+  /* Use MAP_NORESERVE so that RAM will not be wasted on the guard
+     bands; touch all the pages of the actual stack before returning,
+     so we know they are allocated.  */
+  desc->alloc_base = xmmap (0,
+                            desc->alloc_size,
+                            PROT_READ|PROT_WRITE,
+                            MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE|MAP_STACK,
+                            -1);
+
+  xmprotect (desc->alloc_base, guardsize, PROT_NONE);
+  xmprotect (desc->alloc_base + guardsize + stacksize, guardsize, PROT_NONE);
+  memset (desc->alloc_base + guardsize, 0xA5, stacksize);
+
+  desc->alt_stack.ss_sp    = desc->alloc_base + guardsize;
+  desc->alt_stack.ss_flags = 0;
+  desc->alt_stack.ss_size  = stacksize;
+
+  if (sigaltstack (&desc->alt_stack, &desc->old_stack))
+    FAIL_EXIT1 ("sigaltstack (new stack: sp=%p, size=%zu, flags=%u): %m\n",
+                desc->alt_stack.ss_sp, desc->alt_stack.ss_size,
+                desc->alt_stack.ss_flags);
+
+  return desc;
+}
+
+void
+xfree_sigstack (void *stack)
+{
+  struct sigstack_desc *desc = stack;
+
+  if (sigaltstack (&desc->old_stack, 0))
+    FAIL_EXIT1 ("sigaltstack (restore old stack: sp=%p, size=%zu, flags=%u): "
+                "%m\n", desc->old_stack.ss_sp, desc->old_stack.ss_size,
+                desc->old_stack.ss_flags);
+  xmunmap (desc->alloc_base, desc->alloc_size);
+  free (desc);
+}
+
+void
+xget_sigstack_location (const void *stack, unsigned char **addrp, size_t *sizep)
+{
+  const struct sigstack_desc *desc = stack;
+  *addrp = desc->alt_stack.ss_sp;
+  *sizep = desc->alt_stack.ss_size;
+}
-- 
2.20.1

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Florian Weimer]

* Zack Weinberg:

> It also occurs to me that on some architectures MINSIGSTKSZ is less
> than a page; on those architectures, the rounding done in xsigstack.c
> means we aren't _really_ testing this stuff in MINSIGSTKSZ space.
> Since overflow is much more of a concern than underflow, what do
> people think of adjusting the code in xsigstack.c so that the area
> actually passed to sigaltstack will not be rounded and will be right
> up against the guard in the direction of overflow?  This would mean
> xsigstack.c has to know which direction the stack grows, but I think
> we already have internal macros for that,

_STACK_GROWS_DOWN, _STACK_GROWS_UP, and NEED_SEPARATE_REGISTER_STACK on
ia64.  But ia64 uses stack sizes which are a multiple of the page size,
so you are lucky. 8-)

Thanks,
Florian

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Carlos O'Donell]

On 1/16/19 9:06 AM, Zack Weinberg wrote:
> The patch I just pushed is attached to this message.  Thanks for the
> quick reviews.  A couple of notes:
> 
>> I'm not sure if it's explicitly spelled out that the default disposition
>> for SIGABRT is to core
> 
> This is indeed specified in POSIX, see the table of default signal actions in
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html .
> 
>> +/* C2011 7.4.1.1p5 specifies that only the following operations are
>> +   guaranteed to be well-defined inside an asynchronous signal handler:
>> ...
>> +     * calling abort, _Exit, quick_exit, or signal
>> +       * signal may only be called with its first argument equal to the
>> +         number of the signal that caused the handler to be called
> 
> It occurred to me this morning that signal is implemented using
> sigaction, and a 'struct sigaction' is 152 bytes on x86-64 (because of
> the sigset_t).  Normally that's nothing to worry about, but when
> there's only 2048 bytes of stack in total and the kernel has filled
> more than half of that with ucontext data, it _could_ be a problem.
> I left the test as is for now, but I suggest that if it does crash on
> any architecture we should drop that part of tst-minsigstksz-1.c
> without hesitation.
> 
> It also occurs to me that on some architectures MINSIGSTKSZ is less
> than a page; on those architectures, the rounding done in xsigstack.c
> means we aren't _really_ testing this stuff in MINSIGSTKSZ space.
> Since overflow is much more of a concern than underflow, what do
> people think of adjusting the code in xsigstack.c so that the area
> actually passed to sigaltstack will not be rounded and will be right
> up against the guard in the direction of overflow?  This would mean
> xsigstack.c has to know which direction the stack grows, but I think
> we already have internal macros for that, so it's not a huge problem.
> I would do this as a follow-up patch if it's agreed to be a good idea.

I worried about this during the original review, but my conclusion was
that no sane user would do such an alignment to push their own stack
against the guard. At least I've never seen it done. This does mean that
the user has a variable amount of trailing stack space that depends on
the configured kernel page size, which is different from the userspace
configured static page size (but the same as the dynamically available
value from sysconf / AT_PAGESZ). If the kernel uses THP you also loose
because the page may be 2MiB and from userspace you don't know that.
So the test adjustment you propose is sensible, and I think would add
value to the test and catch out the currently silly values of 2048
(in most cases).

I expect to see some failures, so I think such an adjustment should
be made once 2.30 is open for development :-) Also keep in mind that
to be pedantic you need to know that the machine stack pointer
alignment requirement is not equal to the page alignment, maybe a note
there that MINSIGSTKSZ is such that it is a multiple of the stack
alignment?

-- 
Cheers,
Carlos.

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Adhemerval Zanella]

On 16/01/2019 01:40, Siddhesh Poyarekar wrote:
> On 16/01/19 2:45 AM, Carlos O'Donell wrote:
>> Thank you for doing this.
>>
>> OK for master with comment addition in implementation.
>>
>> You need Siddhesh to approve the extra tests.
>>
> 
> OK to add these tests to master.
> 
> Siddhesh

We need something like below for Hurd:

diff --git a/support/xsigstack.c b/support/xsigstack.c
index cebfa19..5656e84 100644
--- a/support/xsigstack.c
+++ b/support/xsigstack.c
@@ -63,6 +63,12 @@ xalloc_sigstack (size_t size)
   /* Use MAP_NORESERVE so that RAM will not be wasted on the guard
      bands; touch all the pages of the actual stack before returning,
      so we know they are allocated.  */
+#ifndef MAP_NORESERVE
+# define MAP_NORESERVE 0
+#endif
+#ifndef MAP_STACK
+# define MAP_STACK 0
+#endif
   desc->alloc_base = xmmap (0,
                             desc->alloc_size,
                             PROT_READ|PROT_WRITE,

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Richard Henderson]

On 1/16/19 7:05 AM, Zack Weinberg wrote:
> +xalloc_sigstack (size_t size)
> +{
> +  size_t pagesize = sysconf (_SC_PAGESIZE);
> +  if (pagesize == -1)
> +    FAIL_EXIT1 ("sysconf (_SC_PAGESIZE): %m\n");
> +
> +  size_t stacksize = roundup (size + MINSIGSTKSZ, pagesize);
...
> +  desc->alt_stack.ss_sp    = desc->alloc_base + guardsize;
> +  desc->alt_stack.ss_flags = 0;
> +  desc->alt_stack.ss_size  = stacksize;

While I understand that the actual allocation from the system must round up to
pages, why do you want to round up the amount as seen by ss_{sp,size}?

It seems to me that if you really want to test MINSIGSTKSZ, then you should do
exactly that.  E.g.

  desc->alt_stack.ss_size = size;
  if (_STACK_GROWS_DOWN)
    desc->alt_stack.ss_sp = desc->alloc_base + guardsize;
  else
    desc->alt_stack.ss_sp = desc->alloc_base + guardsize + stacksize - size;


r~

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Richard Henderson]

On 1/17/19 1:06 AM, Zack Weinberg wrote:
> It also occurs to me that on some architectures MINSIGSTKSZ is less
> than a page; on those architectures, the rounding done in xsigstack.c
> means we aren't _really_ testing this stuff in MINSIGSTKSZ space.
> Since overflow is much more of a concern than underflow, what do
> people think of adjusting the code in xsigstack.c so that the area
> actually passed to sigaltstack will not be rounded and will be right
> up against the guard in the direction of overflow?  This would mean
> xsigstack.c has to know which direction the stack grows, but I think
> we already have internal macros for that, so it's not a huge problem.
> I would do this as a follow-up patch if it's agreed to be a good idea.

Heh.  Should have read the whole thread first.
>From my other mail, you can clearly tell that I vote yes for this.  ;-)


r~

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Richard Henderson]

On 1/17/19 4:06 AM, Carlos O'Donell wrote:
> I worried about this during the original review, but my conclusion was
> that no sane user would do such an alignment to push their own stack
> against the guard. At least I've never seen it done.

I have seen users use malloc to allocate stacks.  Which is basically the same
as putting the allocation next to the guard, except it corrupts the arena
instead.  :-P


r~

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[H.J. Lu]

On Wed, Jan 16, 2019 at 9:06 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 1/16/19 9:06 AM, Zack Weinberg wrote:
> > The patch I just pushed is attached to this message.  Thanks for the
> > quick reviews.  A couple of notes:
> >
> >> I'm not sure if it's explicitly spelled out that the default disposition
> >> for SIGABRT is to core
> >
> > This is indeed specified in POSIX, see the table of default signal actions in
> > http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html .
> >
> >> +/* C2011 7.4.1.1p5 specifies that only the following operations are
> >> +   guaranteed to be well-defined inside an asynchronous signal handler:
> >> ...
> >> +     * calling abort, _Exit, quick_exit, or signal
> >> +       * signal may only be called with its first argument equal to the
> >> +         number of the signal that caused the handler to be called
> >
> > It occurred to me this morning that signal is implemented using
> > sigaction, and a 'struct sigaction' is 152 bytes on x86-64 (because of
> > the sigset_t).  Normally that's nothing to worry about, but when
> > there's only 2048 bytes of stack in total and the kernel has filled
> > more than half of that with ucontext data, it _could_ be a problem.
> > I left the test as is for now, but I suggest that if it does crash on
> > any architecture we should drop that part of tst-minsigstksz-1.c
> > without hesitation.
> >
> > It also occurs to me that on some architectures MINSIGSTKSZ is less
> > than a page; on those architectures, the rounding done in xsigstack.c
> > means we aren't _really_ testing this stuff in MINSIGSTKSZ space.
> > Since overflow is much more of a concern than underflow, what do
> > people think of adjusting the code in xsigstack.c so that the area
> > actually passed to sigaltstack will not be rounded and will be right
> > up against the guard in the direction of overflow?  This would mean
> > xsigstack.c has to know which direction the stack grows, but I think
> > we already have internal macros for that, so it's not a huge problem.
> > I would do this as a follow-up patch if it's agreed to be a good idea.
>
> I worried about this during the original review, but my conclusion was
> that no sane user would do such an alignment to push their own stack
> against the guard. At least I've never seen it done. This does mean that
> the user has a variable amount of trailing stack space that depends on
> the configured kernel page size, which is different from the userspace
> configured static page size (but the same as the dynamically available
> value from sysconf / AT_PAGESZ). If the kernel uses THP you also loose
> because the page may be 2MiB and from userspace you don't know that.
> So the test adjustment you propose is sensible, and I think would add
> value to the test and catch out the currently silly values of 2048
> (in most cases).
>
> I expect to see some failures, so I think such an adjustment should
> be made once 2.30 is open for development :-) Also keep in mind that
> to be pedantic you need to know that the machine stack pointer
> alignment requirement is not equal to the page alignment, maybe a note
> there that MINSIGSTKSZ is such that it is a multiple of the stack
> alignment?
>

The new tests failed on AVX512 machines:

Program received signal SIGUSR1, User defined signal 1.
__GI_raise (sig=sig@entry=10) at ../sysdeps/unix/sysv/linux/raise.c:50
50   return ret;
(gdb) c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
_dl_runtime_resolve_xsavec () at ../sysdeps/x86_64/dl-trampoline.h:93
93 movq %rax, REGISTER_SAVE_RAX(%rsp)
(gdb) bt
#0  _dl_runtime_resolve_xsavec () at ../sysdeps/x86_64/dl-trampoline.h:93
#1  0x0040248d in handler (unused=<optimized out>) at tst-minsigstksz-4.c:44
#2  <signal handler called>
#3  __GI_raise (sig=sig@entry=10) at ../sysdeps/unix/sysv/linux/raise.c:50
#4  0x004024da in do_test () at tst-minsigstksz-4.c:59
#5  0x00402cd6 in support_test_main (argc=1, argv=0xffffcef8,
    config=config@entry=0xffffcdf0) at support_test_main.c:350
#6  0x00402348 in main (argc=<optimized out>, argv=<optimized out>)
    at ../support/test-driver.c:168
(gdb)

AVX512 needs 2560 bytes to save processor state.

-- 
H.J.

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Zack Weinberg]

On Wed, Jan 16, 2019 at 10:31 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> The new tests failed on AVX512 machines:
>
> Program received signal SIGUSR1, User defined signal 1.
> __GI_raise (sig=sig@entry=10) at ../sysdeps/unix/sysv/linux/raise.c:50
> 50   return ret;
> (gdb) c
> Continuing.
>
> Program received signal SIGSEGV, Segmentation fault.
> _dl_runtime_resolve_xsavec () at ../sysdeps/x86_64/dl-trampoline.h:93
> 93 movq %rax, REGISTER_SAVE_RAX(%rsp)
> (gdb) bt
> #0  _dl_runtime_resolve_xsavec () at ../sysdeps/x86_64/dl-trampoline.h:93
> #1  0x0040248d in handler (unused=<optimized out>) at tst-minsigstksz-4.c:44
> #2  <signal handler called>
> #3  __GI_raise (sig=sig@entry=10) at ../sysdeps/unix/sysv/linux/raise.c:50
> #4  0x004024da in do_test () at tst-minsigstksz-4.c:59
> #5  0x00402cd6 in support_test_main (argc=1, argv=0xffffcef8,
>     config=config@entry=0xffffcdf0) at support_test_main.c:350
> #6  0x00402348 in main (argc=<optimized out>, argv=<optimized out>)
>     at ../support/test-driver.c:168
> (gdb)
>
> AVX512 needs 2560 bytes to save processor state.

Well, this is the problem that we knew existed and can't fix quickly.
If I'm reading http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
correctly, we can't introduce new sysconf constants unilaterally
(there is no license to define system-specific _SC_* symbols).

I wonder if this test passes if you run it with LD_BIND_NOW=t in the
environment.  Forcing -z now for these tests might be the best we can
do for 2.29.

zw

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[H.J. Lu]

On Thu, Jan 17, 2019 at 7:12 PM Zack Weinberg <zackw@panix.com> wrote:
>
> On Wed, Jan 16, 2019 at 10:31 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > The new tests failed on AVX512 machines:
> >
> > Program received signal SIGUSR1, User defined signal 1.
> > __GI_raise (sig=sig@entry=10) at ../sysdeps/unix/sysv/linux/raise.c:50
> > 50   return ret;
> > (gdb) c
> > Continuing.
> >
> > Program received signal SIGSEGV, Segmentation fault.
> > _dl_runtime_resolve_xsavec () at ../sysdeps/x86_64/dl-trampoline.h:93
> > 93 movq %rax, REGISTER_SAVE_RAX(%rsp)
> > (gdb) bt
> > #0  _dl_runtime_resolve_xsavec () at ../sysdeps/x86_64/dl-trampoline.h:93
> > #1  0x0040248d in handler (unused=<optimized out>) at tst-minsigstksz-4.c:44
> > #2  <signal handler called>
> > #3  __GI_raise (sig=sig@entry=10) at ../sysdeps/unix/sysv/linux/raise.c:50
> > #4  0x004024da in do_test () at tst-minsigstksz-4.c:59
> > #5  0x00402cd6 in support_test_main (argc=1, argv=0xffffcef8,
> >     config=config@entry=0xffffcdf0) at support_test_main.c:350
> > #6  0x00402348 in main (argc=<optimized out>, argv=<optimized out>)
> >     at ../support/test-driver.c:168
> > (gdb)
> >
> > AVX512 needs 2560 bytes to save processor state.
>
> Well, this is the problem that we knew existed and can't fix quickly.
> If I'm reading http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
> correctly, we can't introduce new sysconf constants unilaterally
> (there is no license to define system-specific _SC_* symbols).
>
> I wonder if this test passes if you run it with LD_BIND_NOW=t in the
> environment.  Forcing -z now for these tests might be the best we can
> do for 2.29.
>

This works:

diff --git a/signal/Makefile b/signal/Makefile
index 9597287bca..9c65c26887 100644
--- a/signal/Makefile
+++ b/signal/Makefile
@@ -59,3 +59,9 @@ CFLAGS-sigwait.c += -fexceptions -fasynchronous-unwind-tables
 CFLAGS-sigwaitinfo.c += -fexceptions -fasynchronous-unwind-tables

 CFLAGS-sigreturn.c += $(no-stack-protector)
+
+LDFLAGS-tst-minsigstksz-1 = -Wl,-z,now
+LDFLAGS-tst-minsigstksz-2 = -Wl,-z,now
+LDFLAGS-tst-minsigstksz-3 = -Wl,-z,now
+LDFLAGS-tst-minsigstksz-3a = -Wl,-z,now
+LDFLAGS-tst-minsigstksz-4 = -Wl,-z,now

-- 
H.J.

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Carlos O'Donell]

On 1/18/19 9:53 AM, H.J. Lu wrote:
> On Thu, Jan 17, 2019 at 7:12 PM Zack Weinberg <zackw@panix.com> wrote:
>>
>> On Wed, Jan 16, 2019 at 10:31 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>
>>> The new tests failed on AVX512 machines:
>>>
>>> Program received signal SIGUSR1, User defined signal 1.
>>> __GI_raise (sig=sig@entry=10) at ../sysdeps/unix/sysv/linux/raise.c:50
>>> 50   return ret;
>>> (gdb) c
>>> Continuing.
>>>
>>> Program received signal SIGSEGV, Segmentation fault.
>>> _dl_runtime_resolve_xsavec () at ../sysdeps/x86_64/dl-trampoline.h:93
>>> 93 movq %rax, REGISTER_SAVE_RAX(%rsp)
>>> (gdb) bt
>>> #0  _dl_runtime_resolve_xsavec () at ../sysdeps/x86_64/dl-trampoline.h:93
>>> #1  0x0040248d in handler (unused=<optimized out>) at tst-minsigstksz-4.c:44
>>> #2  <signal handler called>
>>> #3  __GI_raise (sig=sig@entry=10) at ../sysdeps/unix/sysv/linux/raise.c:50
>>> #4  0x004024da in do_test () at tst-minsigstksz-4.c:59
>>> #5  0x00402cd6 in support_test_main (argc=1, argv=0xffffcef8,
>>>     config=config@entry=0xffffcdf0) at support_test_main.c:350
>>> #6  0x00402348 in main (argc=<optimized out>, argv=<optimized out>)
>>>     at ../support/test-driver.c:168
>>> (gdb)
>>>
>>> AVX512 needs 2560 bytes to save processor state.
>>
>> Well, this is the problem that we knew existed and can't fix quickly.
>> If I'm reading http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
>> correctly, we can't introduce new sysconf constants unilaterally
>> (there is no license to define system-specific _SC_* symbols).
>>
>> I wonder if this test passes if you run it with LD_BIND_NOW=t in the
>> environment.  Forcing -z now for these tests might be the best we can
>> do for 2.29.
>>
> 
> This works:
> 
> diff --git a/signal/Makefile b/signal/Makefile
> index 9597287bca..9c65c26887 100644
> --- a/signal/Makefile
> +++ b/signal/Makefile
> @@ -59,3 +59,9 @@ CFLAGS-sigwait.c += -fexceptions -fasynchronous-unwind-tables
>  CFLAGS-sigwaitinfo.c += -fexceptions -fasynchronous-unwind-tables
> 
>  CFLAGS-sigreturn.c += $(no-stack-protector)
> +

OK with comment:

# We don't want to test the lazy resolution stack usage
# just the execution of the handler and the functions
> +LDFLAGS-tst-minsigstksz-1 = -Wl,-z,now
> +LDFLAGS-tst-minsigstksz-2 = -Wl,-z,now
> +LDFLAGS-tst-minsigstksz-3 = -Wl,-z,now
> +LDFLAGS-tst-minsigstksz-3a = -Wl,-z,now
> +LDFLAGS-tst-minsigstksz-4 = -Wl,-z,now
> 

I'd say this is OK for master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

All of this raises an interesting point. Should MINSIGSTKSZ
have included enough space for the lazy resolution to happen?
I think we have to, because you're never going to have already
called abort, quick_exit, or _exit, so they will all go through
lazy binding resolution if you're not BIND_NOW. Which means we
need an average estimate from all arches about the lazy binding
stack usage.

-- 
Cheers,
Carlos.

[PATCH] Disable lazy binding on tests for minimal signal handler

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 3169 bytes --]

On Fri, Jan 18, 2019 at 8:40 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 1/18/19 9:53 AM, H.J. Lu wrote:
> > On Thu, Jan 17, 2019 at 7:12 PM Zack Weinberg <zackw@panix.com> wrote:
> >>
> >> On Wed, Jan 16, 2019 at 10:31 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>>
> >>> The new tests failed on AVX512 machines:
> >>>
> >>> Program received signal SIGUSR1, User defined signal 1.
> >>> __GI_raise (sig=sig@entry=10) at ../sysdeps/unix/sysv/linux/raise.c:50
> >>> 50   return ret;
> >>> (gdb) c
> >>> Continuing.
> >>>
> >>> Program received signal SIGSEGV, Segmentation fault.
> >>> _dl_runtime_resolve_xsavec () at ../sysdeps/x86_64/dl-trampoline.h:93
> >>> 93 movq %rax, REGISTER_SAVE_RAX(%rsp)
> >>> (gdb) bt
> >>> #0  _dl_runtime_resolve_xsavec () at ../sysdeps/x86_64/dl-trampoline.h:93
> >>> #1  0x0040248d in handler (unused=<optimized out>) at tst-minsigstksz-4.c:44
> >>> #2  <signal handler called>
> >>> #3  __GI_raise (sig=sig@entry=10) at ../sysdeps/unix/sysv/linux/raise.c:50
> >>> #4  0x004024da in do_test () at tst-minsigstksz-4.c:59
> >>> #5  0x00402cd6 in support_test_main (argc=1, argv=0xffffcef8,
> >>>     config=config@entry=0xffffcdf0) at support_test_main.c:350
> >>> #6  0x00402348 in main (argc=<optimized out>, argv=<optimized out>)
> >>>     at ../support/test-driver.c:168
> >>> (gdb)
> >>>
> >>> AVX512 needs 2560 bytes to save processor state.
> >>
> >> Well, this is the problem that we knew existed and can't fix quickly.
> >> If I'm reading http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
> >> correctly, we can't introduce new sysconf constants unilaterally
> >> (there is no license to define system-specific _SC_* symbols).
> >>
> >> I wonder if this test passes if you run it with LD_BIND_NOW=t in the
> >> environment.  Forcing -z now for these tests might be the best we can
> >> do for 2.29.
> >>
> >
> > This works:
> >
> > diff --git a/signal/Makefile b/signal/Makefile
> > index 9597287bca..9c65c26887 100644
> > --- a/signal/Makefile
> > +++ b/signal/Makefile
> > @@ -59,3 +59,9 @@ CFLAGS-sigwait.c += -fexceptions -fasynchronous-unwind-tables
> >  CFLAGS-sigwaitinfo.c += -fexceptions -fasynchronous-unwind-tables
> >
> >  CFLAGS-sigreturn.c += $(no-stack-protector)
> > +
>
> OK with comment:
>
> # We don't want to test the lazy resolution stack usage
> # just the execution of the handler and the functions
> > +LDFLAGS-tst-minsigstksz-1 = -Wl,-z,now
> > +LDFLAGS-tst-minsigstksz-2 = -Wl,-z,now
> > +LDFLAGS-tst-minsigstksz-3 = -Wl,-z,now
> > +LDFLAGS-tst-minsigstksz-3a = -Wl,-z,now
> > +LDFLAGS-tst-minsigstksz-4 = -Wl,-z,now
> >
>
> I'd say this is OK for master.
>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> All of this raises an interesting point. Should MINSIGSTKSZ
> have included enough space for the lazy resolution to happen?
> I think we have to, because you're never going to have already
> called abort, quick_exit, or _exit, so they will all go through
> lazy binding resolution if you're not BIND_NOW. Which means we
> need an average estimate from all arches about the lazy binding
> stack usage.
>

This is the patch I am checking in.

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-Disable-lazy-binding-on-tests-for-minimal-signal-han.patch --]
[-- Type: text/x-patch, Size: 1951 bytes --]

From 562f43620dc4fd06e4d7abc7cd03c05cd8ea98ae Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 18 Jan 2019 08:56:51 -0800
Subject: [PATCH] Disable lazy binding on tests for minimal signal handler

Since MINSIGSTKSZ may not have sufficent stack space to allow lazy
binding, build tests for minimal signal handler with -Wl,-z,now to
disable lazy binding.

	* signal/Makefile (LDFLAGS-tst-minsigstksz-1): New.  Set to
	-Wl,-z,now.
	(LDFLAGS-tst-minsigstksz-2): Likewise.
	(LDFLAGS-tst-minsigstksz-3): Likewise.
	(LDFLAGS-tst-minsigstksz-3a): Likewise.
	(LDFLAGS-tst-minsigstksz-4): Likewise.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
---
 ChangeLog       | 9 +++++++++
 signal/Makefile | 8 ++++++++
 2 files changed, 17 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 1b13e62e88..59d8b83289 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2019-01-18  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* signal/Makefile (LDFLAGS-tst-minsigstksz-1): New.  Set to
+	-Wl,-z,now.
+	(LDFLAGS-tst-minsigstksz-2): Likewise.
+	(LDFLAGS-tst-minsigstksz-3): Likewise.
+	(LDFLAGS-tst-minsigstksz-3a): Likewise.
+	(LDFLAGS-tst-minsigstksz-4): Likewise.
+
 2019-01-18  TAMUKI Shoichi  <tamuki@linet.gr.jp>
 
 	* manual/time.texi (strftime): Fix the wording to "alternative" rather
diff --git a/signal/Makefile b/signal/Makefile
index 9597287bca..06034fee8e 100644
--- a/signal/Makefile
+++ b/signal/Makefile
@@ -59,3 +59,11 @@ CFLAGS-sigwait.c += -fexceptions -fasynchronous-unwind-tables
 CFLAGS-sigwaitinfo.c += -fexceptions -fasynchronous-unwind-tables
 
 CFLAGS-sigreturn.c += $(no-stack-protector)
+
+# We don't want to test the lazy resolution stack usage, just the
+# execution of the handler and the functions.
+LDFLAGS-tst-minsigstksz-1 = -Wl,-z,now
+LDFLAGS-tst-minsigstksz-2 = -Wl,-z,now
+LDFLAGS-tst-minsigstksz-3 = -Wl,-z,now
+LDFLAGS-tst-minsigstksz-3a = -Wl,-z,now
+LDFLAGS-tst-minsigstksz-4 = -Wl,-z,now
-- 
2.20.1

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Zack Weinberg]

On Fri, Jan 18, 2019 at 11:40 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> All of this raises an interesting point. Should MINSIGSTKSZ
> have included enough space for the lazy resolution to happen?
> I think we have to, because you're never going to have already
> called abort, quick_exit, or _exit, so they will all go through
> lazy binding resolution if you're not BIND_NOW. Which means we
> need an average estimate from all arches about the lazy binding
> stack usage.

in the long term, this strikes me as another reason we should be
thinking about making eager symbol resolution the default ... (and
then we could start thinking about moving the dynamic loader out of
process ...)

in the medium term, though, I completely agree

in the 2.29 term, though, perhaps the best we can do is some
documentation, I'll have a go at writing that in the next couple days

zw

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[H.J. Lu]

On Fri, Jan 18, 2019 at 10:08 AM Zack Weinberg <zackw@panix.com> wrote:
>
> On Fri, Jan 18, 2019 at 11:40 AM Carlos O'Donell <carlos@redhat.com> wrote:
> >
> > All of this raises an interesting point. Should MINSIGSTKSZ
> > have included enough space for the lazy resolution to happen?
> > I think we have to, because you're never going to have already
> > called abort, quick_exit, or _exit, so they will all go through
> > lazy binding resolution if you're not BIND_NOW. Which means we
> > need an average estimate from all arches about the lazy binding
> > stack usage.
>
> in the long term, this strikes me as another reason we should be
> thinking about making eager symbol resolution the default ... (and
> then we could start thinking about moving the dynamic loader out of
> process ...)
>
> in the medium term, though, I completely agree
>
> in the 2.29 term, though, perhaps the best we can do is some
> documentation, I'll have a go at writing that in the next couple days
>

The 32-bit signal/tst-minsigstksz-1 failed on AVX512 machine:

Program received signal SIGSEGV, Segmentation fault.
0xf7e2e366 in __GI___libc_sigaction (sig=10, act=0xf7cf70a8, oact=0xf7cf7134)
    at ../sysdeps/unix/sysv/linux/sigaction.c:48
48   if (act)
(gdb) disass
Dump of assembler code for function __GI___libc_sigaction:
   0xf7e2e350 <+0>: sub    $0x14c,%esp
   0xf7e2e356 <+6>: xor    %ecx,%ecx
   0xf7e2e358 <+8>: mov    %esi,0x140(%esp)
   0xf7e2e35f <+15>: mov    0x154(%esp),%edx
=> 0xf7e2e366 <+22>: call   0xf7f44c2f <__x86.get_pc_thunk.si>

For 64-bit:

Breakpoint 3, __GI___libc_sigaction (sig=sig@entry=10,
    act=act@entry=0x7fffffffdb40, oact=oact@entry=0x0)
    at ../sysdeps/unix/sysv/linux/sigaction.c:48
48   if (act)
(gdb) disass
Dump of assembler code for function __GI___libc_sigaction:
=> 0x00007ffff7e4dc50 <+0>: sub    $0xd0,%rsp
   0x00007ffff7e4dc57 <+7>: mov    %rdx,%r8
   0x00007ffff7e4dc5a <+10>: test   %rsi,%rsi
   0x00007ffff7e4dc5d <+13>: je     0x7ffff7e4ddb0 <__GI___libc_sigaction+352>
   0x00007ffff7e4dc63 <+19>: mov    (%rsi),%rax

64-bit allocates smaller stack and it doesn't need to call
__x86.get_pc_thunk.si.  On AVX512, kernel needs larger
stack space to save signal context.

-- 
H.J.

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Joseph Myers]

On Wed, 16 Jan 2019, Adhemerval Zanella wrote:

> We need something like below for Hurd:

This patch is OK (we shouldn't be leaving the build broken...).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Carlos O'Donell]

On 1/18/19 2:49 PM, H.J. Lu wrote:
> On Fri, Jan 18, 2019 at 10:08 AM Zack Weinberg <zackw@panix.com> wrote:
>>
>> On Fri, Jan 18, 2019 at 11:40 AM Carlos O'Donell <carlos@redhat.com> wrote:
>>>
>>> All of this raises an interesting point. Should MINSIGSTKSZ
>>> have included enough space for the lazy resolution to happen?
>>> I think we have to, because you're never going to have already
>>> called abort, quick_exit, or _exit, so they will all go through
>>> lazy binding resolution if you're not BIND_NOW. Which means we
>>> need an average estimate from all arches about the lazy binding
>>> stack usage.
>>
>> in the long term, this strikes me as another reason we should be
>> thinking about making eager symbol resolution the default ... (and
>> then we could start thinking about moving the dynamic loader out of
>> process ...)
>>
>> in the medium term, though, I completely agree
>>
>> in the 2.29 term, though, perhaps the best we can do is some
>> documentation, I'll have a go at writing that in the next couple days
>>
> 
> The 32-bit signal/tst-minsigstksz-1 failed on AVX512 machine:
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0xf7e2e366 in __GI___libc_sigaction (sig=10, act=0xf7cf70a8, oact=0xf7cf7134)
>     at ../sysdeps/unix/sysv/linux/sigaction.c:48
> 48   if (act)
> (gdb) disass
> Dump of assembler code for function __GI___libc_sigaction:
>    0xf7e2e350 <+0>: sub    $0x14c,%esp
>    0xf7e2e356 <+6>: xor    %ecx,%ecx
>    0xf7e2e358 <+8>: mov    %esi,0x140(%esp)
>    0xf7e2e35f <+15>: mov    0x154(%esp),%edx
> => 0xf7e2e366 <+22>: call   0xf7f44c2f <__x86.get_pc_thunk.si>
> 
> For 64-bit:
> 
> Breakpoint 3, __GI___libc_sigaction (sig=sig@entry=10,
>     act=act@entry=0x7fffffffdb40, oact=oact@entry=0x0)
>     at ../sysdeps/unix/sysv/linux/sigaction.c:48
> 48   if (act)
> (gdb) disass
> Dump of assembler code for function __GI___libc_sigaction:
> => 0x00007ffff7e4dc50 <+0>: sub    $0xd0,%rsp
>    0x00007ffff7e4dc57 <+7>: mov    %rdx,%r8
>    0x00007ffff7e4dc5a <+10>: test   %rsi,%rsi
>    0x00007ffff7e4dc5d <+13>: je     0x7ffff7e4ddb0 <__GI___libc_sigaction+352>
>    0x00007ffff7e4dc63 <+19>: mov    (%rsi),%rax
> 
> 64-bit allocates smaller stack and it doesn't need to call
> __x86.get_pc_thunk.si.  On AVX512, kernel needs larger
> stack space to save signal context.
 
I think these should be marked as XFAIL for x86, if the tests
can't pass with BIND_NOW then there is a real problem here
that needs fixing in the next release cycle.

-- 
Cheers,
Carlos.

Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.

[Zack Weinberg]

On Mon, Jan 21, 2019 at 1:07 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Wed, 16 Jan 2019, Adhemerval Zanella wrote:
>
> > We need something like below for Hurd:
>
> This patch is OK (we shouldn't be leaving the build broken...).

Argh, this fell off the bottom of my inbox.  Can I request one change?
 Put the ifdefs right after the include block, not in the middle of a
function.

zw