Re: libsigsegv on LinuxFromScratch

[Paul Eggert <eggert@cs.ucla.edu>]

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

On 9/19/20 4:47 PM, Bruno Haible wrote:

> 	havelib: Avoid linking with libc.a on GNU systems.

Thanks for fixing the bug. This caused me to look at the c-stack module for the 
first time in a while, and I found some old-fashioned code and some unlikely 
bugs and fixed one misfeature when libsigsegv is not in use. I installed the 
attached patches to the c-stack module in Gnulib to try to fix it. These changes 
shouldn't affect how c-stack behaves when libsigsegv is in use.

While looking into this I discovered pthread_getattr_np + pthread_attr_getstack 
which might have been nice for the GNU/Linux part of c-stack.c, except they're 
not async-signal-safe. As I understand it, libsigsegv works around the 
async-signal-safe problem by parsing /proc/self/maps with async-signal-safe 
functions, which is quite a feat and is probably beyond what c-stack should do.

PS. I also found this circa-2015 Linux kernel bug related to PIE that looks like 
it might be of interest to the libsigsegv developers

https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-1000253

This bug causes /proc/self/maps to report the wrong VMA (actually, overlapping 
VMAs) for the stack. This could be worth a comment in the libsigsegv sources. 
For more commentary in this area please see:

https://stackoverflow.com/questions/56893353/analyzing-memory-mapping-of-a-process-with-pmap-stack/56920770

PPS. Given the longstanding security problems with stack overflow (as witness 
the name stackoverflow.com!) it is somewhat disturbing that there is still no 
reliable way in GNU/Linux to answer the simple question "Where's my stack?" or 
to detect and recover from stack overflow reliably. What's up with that?

[-- Attachment #2: 0001-c-stack-improve-checking-if-libsigsegv.patch --]
[-- Type: text/x-patch, Size: 18794 bytes --]

From 8ba9126d00bfe1ab77a5c820c58c68933d4df85c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 20 Sep 2020 11:48:17 -0700
Subject: [PATCH 1/2] c-stack: improve checking if !libsigsegv
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If SIGINFO_WORKS, do not treat a null pointer dereference as if it
were a stack overflow.  Use uintptr_t and INT_ADD_WRAPV to avoid
unlikely pointer overflow.  Also, fix some obsolete code and typos.
I found these problems while looking into this bug report:
https://lists.gnu.org/r/grep-devel/2020-09/msg00053.html
* lib/c-stack.c: Include c-stack.h first, to test interface.
Include inttypes.h for UINTPTR_MAX, stdbool.h, stddef.h for
max_align_t, intprops.h for INT_ADD_WRAPV.
(USE_LIBSIGSEGV): New macro; use it to simplify later code.
(SIGSTKSZ): Simplify setup.  Work around libsigsegv bug only
for libsigsegv 2.8 and earlier since the bug should be fixed
after that.
(alternate_signal_stack): Use max_align_t instead of doing it by hand.
(segv_handler, overflow_handler, segv_handler) [DEBUG]:
Assume sprintf returns byte count; this assumption is safe now.
(page_size): New static volatile variable, since sysconf isn’t
documented to be async-signal-safe on Solaris.  This variable is
present and used if (!USE_LIBSIGSEGV && HAVE_SIGALTSTACK &&
HAVE_DECL_SIGALTSTACK && HAVE_STACK_OVERFLOW_HANDLING &&
SIGINFO_WORKS).
(segv_handler): Use it if present.  Never report null pointer
dereference as a stack overflow.  Check for (unlikely) unsigned
and/or pointer overflow.
* m4/c-stack.m4 (AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC):
Rename cache variables to gl_cv_sys_stack_overflow_works
and gl_cv_sys_xsi_stack_overflow_heuristic.
All uses changed.
(gl_PREREQ_C_STACK): Do not require AC_FUNC_ALLOCA, since
c-stack no longer uses STACK_DIRECTION.
Do not check for unistd.h, since we depend on unistd.
Fix shell typo ‘$"ac_cv_sys_xsi_stack_overflow_heuristic"’.
* modules/c-stack (Depends-on): Sort.  Add intprops, inttypes,
stdbool, stddef.
---
 ChangeLog       |  37 +++++++++++++
 lib/c-stack.c   | 135 ++++++++++++++++++++++++++++--------------------
 m4/c-stack.m4   |  33 ++++++------
 modules/c-stack |  12 +++--
 4 files changed, 139 insertions(+), 78 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index cc1cd4a40..087b9232f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,40 @@
+2020-09-20  Paul Eggert  <eggert@cs.ucla.edu>
+
+	c-stack: improve checking if !libsigsegv
+	If SIGINFO_WORKS, do not treat a null pointer dereference as if it
+	were a stack overflow.  Use uintptr_t and INT_ADD_WRAPV to avoid
+	unlikely pointer overflow.  Also, fix some obsolete code and typos.
+	I found these problems while looking into this bug report:
+	https://lists.gnu.org/r/grep-devel/2020-09/msg00053.html
+	* lib/c-stack.c: Include c-stack.h first, to test interface.
+	Include inttypes.h for UINTPTR_MAX, stdbool.h, stddef.h for
+	max_align_t, intprops.h for INT_ADD_WRAPV.
+	(USE_LIBSIGSEGV): New macro; use it to simplify later code.
+	(SIGSTKSZ): Simplify setup.  Work around libsigsegv bug only
+	for libsigsegv 2.8 and earlier since the bug should be fixed
+	after that.
+	(alternate_signal_stack): Use max_align_t instead of doing it by hand.
+	(segv_handler, overflow_handler, segv_handler) [DEBUG]:
+	Assume sprintf returns byte count; this assumption is safe now.
+	(page_size): New static volatile variable, since sysconf isn’t
+	documented to be async-signal-safe on Solaris.  This variable is
+	present and used if (!USE_LIBSIGSEGV && HAVE_SIGALTSTACK &&
+	HAVE_DECL_SIGALTSTACK && HAVE_STACK_OVERFLOW_HANDLING &&
+	SIGINFO_WORKS).
+	(segv_handler): Use it if present.  Never report null pointer
+	dereference as a stack overflow.  Check for (unlikely) unsigned
+	and/or pointer overflow.
+	* m4/c-stack.m4 (AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC):
+	Rename cache variables to gl_cv_sys_stack_overflow_works
+	and gl_cv_sys_xsi_stack_overflow_heuristic.
+	All uses changed.
+	(gl_PREREQ_C_STACK): Do not require AC_FUNC_ALLOCA, since
+	c-stack no longer uses STACK_DIRECTION.
+	Do not check for unistd.h, since we depend on unistd.
+	Fix shell typo ‘$"ac_cv_sys_xsi_stack_overflow_heuristic"’.
+	* modules/c-stack (Depends-on): Sort.  Add intprops, inttypes,
+	stdbool, stddef.
+
 2020-09-20  Bruno Haible  <bruno@clisp.org>
 
 	Revert now-unnecessary override of config.guess on Alpine Linux 3.10.
diff --git a/lib/c-stack.c b/lib/c-stack.c
index 59606299b..3649c1bfe 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -35,30 +35,25 @@
 
 #include <config.h>
 
+#include "c-stack.h"
+
 #include "gettext.h"
 #define _(msgid) gettext (msgid)
 
 #include <errno.h>
+#include <inttypes.h>
 
 #include <signal.h>
 #if ! HAVE_STACK_T && ! defined stack_t
 typedef struct sigaltstack stack_t;
 #endif
-#ifndef SIGSTKSZ
-# define SIGSTKSZ 16384
-#elif HAVE_LIBSIGSEGV && SIGSTKSZ < 16384
-/* libsigsegv 2.6 through 2.8 have a bug where some architectures use
-   more than the Linux default of an 8k alternate stack when deciding
-   if a fault was caused by stack overflow.  */
-# undef SIGSTKSZ
-# define SIGSTKSZ 16384
-#endif
 
+#include <stdbool.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <string.h>
 
-/* Posix 2001 declares ucontext_t in <ucontext.h>, Posix 200x in
-   <signal.h>.  */
+/* Pre-2008 POSIX declared ucontext_t in ucontext.h instead of signal.h.  */
 #if HAVE_UCONTEXT_H
 # include <ucontext.h>
 #endif
@@ -69,13 +64,26 @@ typedef struct sigaltstack stack_t;
 # include <stdio.h>
 #endif
 
-#if HAVE_LIBSIGSEGV
+/* Use libsigsegv only if needed; kernels like Solaris can detect
+   stack overflow without the overhead of an external library.  */
+#define USE_LIBSIGSEGV (HAVE_XSI_STACK_OVERFLOW_HEURISTIC && HAVE_LIBSIGSEGV)
+
+#if USE_LIBSIGSEGV
 # include <sigsegv.h>
+/* libsigsegv 2.6 through 2.8 have a bug where some architectures use
+   more than the Linux default of an 8k alternate stack when deciding
+   if a fault was caused by stack overflow.  */
+# if LIBSIGSEGV_VERSION <= 0x0208 && SIGSTKSZ < 16384
+#  undef SIGSTKSZ
+# endif
+#endif
+#ifndef SIGSTKSZ
+# define SIGSTKSZ 16384
 #endif
 
-#include "c-stack.h"
 #include "exitfail.h"
 #include "ignore-value.h"
+#include "intprops.h"
 #include "getprogname.h"
 
 #if defined SA_ONSTACK && defined SA_SIGINFO
@@ -97,7 +105,7 @@ static _GL_ASYNC_SAFE void (* volatile segv_action) (int);
 static char const * volatile program_error_message;
 static char const * volatile stack_overflow_message;
 
-#if ((HAVE_LIBSIGSEGV && ! HAVE_XSI_STACK_OVERFLOW_HEURISTIC) \
+#if (USE_LIBSIGSEGV                                           \
      || (HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK            \
          && HAVE_STACK_OVERFLOW_HANDLING))
 
@@ -111,12 +119,12 @@ static _GL_ASYNC_SAFE _Noreturn void
 die (int signo)
 {
   char const *message;
-#if !SIGINFO_WORKS && !HAVE_LIBSIGSEGV
+# if !SIGINFO_WORKS && !USE_LIBSIGSEGV
   /* We can't easily determine whether it is a stack overflow; so
      assume that the rest of our program is perfect (!) and that
      this segmentation violation is a stack overflow.  */
   signo = 0;
-#endif /* !SIGINFO_WORKS && !HAVE_LIBSIGSEGV */
+# endif
   segv_action (signo);
   message = signo ? program_error_message : stack_overflow_message;
   ignore_value (write (STDERR_FILENO, progname, strlen (progname)));
@@ -128,22 +136,12 @@ die (int signo)
   raise (signo);
   abort ();
 }
-#endif
-
-#if (HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK \
-     && HAVE_STACK_OVERFLOW_HANDLING) || HAVE_LIBSIGSEGV
 
 /* Storage for the alternate signal stack.  */
 static union
 {
   char buffer[SIGSTKSZ];
-
-  /* These other members are for proper alignment.  There's no
-     standard way to guarantee stack alignment, but this seems enough
-     in practice.  */
-  long double ld;
-  long l;
-  void *p;
+  max_align_t align;
 } alternate_signal_stack;
 
 static _GL_ASYNC_SAFE void
@@ -153,10 +151,7 @@ null_action (int signo _GL_UNUSED)
 
 #endif /* SIGALTSTACK || LIBSIGSEGV */
 
-/* Only use libsigsegv if we need it; platforms like Solaris can
-   detect stack overflow without the overhead of an external
-   library.  */
-#if HAVE_LIBSIGSEGV && ! HAVE_XSI_STACK_OVERFLOW_HEURISTIC
+#if USE_LIBSIGSEGV
 
 /* Nonzero if general segv handler could not be installed.  */
 static volatile int segv_handler_missing;
@@ -171,8 +166,8 @@ segv_handler (void *address _GL_UNUSED, int serious)
   {
     char buf[1024];
     int saved_errno = errno;
-    sprintf (buf, "segv_handler serious=%d\n", serious);
-    ignore_value (write (STDERR_FILENO, buf, strlen (buf)));
+    ignore_value (write (STDERR_FILENO, buf,
+                         sprintf (buf, "segv_handler serious=%d\n", serious)));
     errno = saved_errno;
   }
 # endif
@@ -193,9 +188,10 @@ overflow_handler (int emergency, stackoverflow_context_t context _GL_UNUSED)
 # if DEBUG
   {
     char buf[1024];
-    sprintf (buf, "overflow_handler emergency=%d segv_handler_missing=%d\n",
-             emergency, segv_handler_missing);
-    ignore_value (write (STDERR_FILENO, buf, strlen (buf)));
+    ignore_value (write (STDERR_FILENO, buf,
+                         sprintf (buf, ("overflow_handler emergency=%d"
+                                        " segv_handler_missing=%d\n"),
+                                  emergency, segv_handler_missing)));
   }
 # endif
 
@@ -228,6 +224,8 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
 
 # if SIGINFO_WORKS
 
+static size_t volatile page_size;
+
 /* Handle a segmentation violation and exit.  This function is
    async-signal-safe.  */
 
@@ -235,8 +233,17 @@ static _GL_ASYNC_SAFE _Noreturn void
 segv_handler (int signo, siginfo_t *info, void *context _GL_UNUSED)
 {
   /* Clear SIGNO if it seems to have been a stack overflow.  */
-#  if ! HAVE_XSI_STACK_OVERFLOW_HEURISTIC
-  /* We can't easily determine whether it is a stack overflow; so
+
+  /* If si_code is nonpositive, something like raise (SIGSEGV) occurred
+     so it cannot be a stack overflow.  */
+  bool cannot_be_stack_overflow = info->si_code <= 0;
+
+  /* An unaligned address cannot be a stack overflow.  */
+#  if FAULT_YIELDS_SIGBUS
+  cannot_be_stack_overflow |= signo == SIGBUS && info->si_code == BUS_ADRALN;
+#  endif
+
+  /* If we can't easily determine that it is not a stack overflow,
      assume that the rest of our program is perfect (!) and that
      this segmentation violation is a stack overflow.
 
@@ -246,33 +253,44 @@ segv_handler (int signo, siginfo_t *info, void *context _GL_UNUSED)
      Solaris populates uc_stack with the details of the
      interrupted stack, while Linux populates it with the details
      of the current stack.  */
-  signo = 0;
-#  else
-  if (0 < info->si_code)
+  if (!cannot_be_stack_overflow)
     {
       /* If the faulting address is within the stack, or within one
          page of the stack, assume that it is a stack overflow.  */
+      uintptr_t faulting_address = (uintptr_t) info->si_addr;
+
+      /* On all platforms we know of, the first page is not in the
+         stack to catch null pointer dereferening.  However, all other
+         pages might be in the stack.  */
+      void *stack_base = (void *) (uintptr_t) page_size;
+      uintptr_t stack_size = 0; stack_size -= page_size;
+#  if HAVE_XSI_STACK_OVERFLOW_HEURISTIC
+      /* Tighten the stack bounds via the XSI heuristic.  */
       ucontext_t const *user_context = context;
-      char const *stack_base = user_context->uc_stack.ss_sp;
-      size_t stack_size = user_context->uc_stack.ss_size;
-      char const *faulting_address = info->si_addr;
-      size_t page_size = sysconf (_SC_PAGESIZE);
-      size_t s = faulting_address - stack_base + page_size;
-      if (s < stack_size + 2 * page_size)
+      stack_base = user_context->uc_stack.ss_sp;
+      stack_size = user_context->uc_stack.ss_size;
+#  endif
+      uintptr_t base = (uintptr_t) stack_base,
+        lo = (INT_SUBTRACT_WRAPV (base, page_size, &lo) || lo < page_size
+              ? page_size : lo),
+        hi = ((INT_ADD_WRAPV (base, stack_size, &hi)
+               || INT_ADD_WRAPV (hi, page_size - 1, &hi))
+              ? UINTPTR_MAX : hi);
+      if (lo <= faulting_address && faulting_address <= hi)
         signo = 0;
 
 #   if DEBUG
       {
         char buf[1024];
-        sprintf (buf,
-                 "segv_handler fault=%p base=%p size=%lx page=%lx signo=%d\n",
-                 faulting_address, stack_base, (unsigned long) stack_size,
-                 (unsigned long) page_size, signo);
-        ignore_value (write (STDERR_FILENO, buf, strlen (buf)));
+        ignore_value (write (STDERR_FILENO, buf,
+                             sprintf (buf,
+                                      ("segv_handler code=%d fault=%p base=%p"
+                                       " size=0x%zx page=0x%zx signo=%d\n"),
+                                      info->si_code, info->si_addr, stack_base,
+                                      stack_size, page_size, signo)));
       }
-#   endif
-    }
 #  endif
+    }
 
   die (signo);
 }
@@ -303,6 +321,10 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
   stack_overflow_message = _("stack overflow");
   progname = getprogname ();
 
+# if SIGINFO_WORKS
+  page_size = sysconf (_SC_PAGESIZE);
+# endif
+
   sigemptyset (&act.sa_mask);
 
 # if SIGINFO_WORKS
@@ -323,8 +345,9 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
   return sigaction (SIGSEGV, &act, NULL);
 }
 
-#else /* ! ((HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK
-             && HAVE_STACK_OVERFLOW_HANDLING) || HAVE_LIBSIGSEGV) */
+#else /* ! (USE_LIBSIGSEGV
+            || (HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK
+                && HAVE_STACK_OVERFLOW_HANDLING)) */
 
 int
 c_stack_action (_GL_ASYNC_SAFE void (*action) (int)  _GL_UNUSED)
diff --git a/m4/c-stack.m4 b/m4/c-stack.m4
index c3e2bddd3..e98f80fff 100644
--- a/m4/c-stack.m4
+++ b/m4/c-stack.m4
@@ -7,7 +7,7 @@
 
 # Written by Paul Eggert.
 
-# serial 17
+# serial 18
 
 AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
   [
@@ -34,7 +34,7 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
      [Define to 1 if an invalid memory address access may yield a SIGBUS.])
 
    AC_CACHE_CHECK([for working C stack overflow detection],
-     [ac_cv_sys_stack_overflow_works],
+     [gl_cv_sys_stack_overflow_works],
      [AC_RUN_IFELSE([AC_LANG_SOURCE(
            [[
             #include <unistd.h>
@@ -121,17 +121,17 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
               return recurse (0);
             }
            ]])],
-        [ac_cv_sys_stack_overflow_works=yes],
-        [ac_cv_sys_stack_overflow_works=no],
+        [gl_cv_sys_stack_overflow_works=yes],
+        [gl_cv_sys_stack_overflow_works=no],
         [case "$host_os" in
                    # Guess no on native Windows.
-           mingw*) ac_cv_sys_stack_overflow_works="guessing no" ;;
-           *)      ac_cv_sys_stack_overflow_works=cross-compiling ;;
+           mingw*) gl_cv_sys_stack_overflow_works="guessing no" ;;
+           *)      gl_cv_sys_stack_overflow_works=cross-compiling ;;
          esac
         ])
      ])
 
-  if test "$ac_cv_sys_stack_overflow_works" = yes; then
+  if test "$gl_cv_sys_stack_overflow_works" = yes; then
    AC_DEFINE([HAVE_STACK_OVERFLOW_HANDLING], [1],
      [Define to 1 if extending the stack slightly past the limit causes
       a SIGSEGV which can be handled on an alternate stack established
@@ -200,14 +200,14 @@ int main ()
     fi
 
    AC_CACHE_CHECK([for precise C stack overflow detection],
-     [ac_cv_sys_xsi_stack_overflow_heuristic],
+     [gl_cv_sys_xsi_stack_overflow_heuristic],
      [dnl On Linux/sparc64 (both in 32-bit and 64-bit mode), it would be wrong
       dnl to set HAVE_XSI_STACK_OVERFLOW_HEURISTIC to 1, because the third
       dnl argument passed to the segv_handler is a 'struct sigcontext *', not
       dnl an 'ucontext_t *'.  It would lead to a failure of test-c-stack2.sh.
       case "${host_os}--${host_cpu}" in
         linux*--sparc*)
-          ac_cv_sys_xsi_stack_overflow_heuristic=no
+          gl_cv_sys_xsi_stack_overflow_heuristic=no
           ;;
         *)
           AC_RUN_IFELSE(
@@ -329,14 +329,14 @@ int main ()
                   return recurse (0);
                 }
                ]])],
-            [ac_cv_sys_xsi_stack_overflow_heuristic=yes],
-            [ac_cv_sys_xsi_stack_overflow_heuristic=no],
-            [ac_cv_sys_xsi_stack_overflow_heuristic=cross-compiling])
+            [gl_cv_sys_xsi_stack_overflow_heuristic=yes],
+            [gl_cv_sys_xsi_stack_overflow_heuristic=no],
+            [gl_cv_sys_xsi_stack_overflow_heuristic=cross-compiling])
           ;;
       esac
      ])
 
-   if test $ac_cv_sys_xsi_stack_overflow_heuristic = yes; then
+   if test "$gl_cv_sys_xsi_stack_overflow_heuristic" = yes; then
      AC_DEFINE([HAVE_XSI_STACK_OVERFLOW_HEURISTIC], [1],
        [Define to 1 if extending the stack slightly past the limit causes
         a SIGSEGV, and an alternate stack can be established with sigaltstack,
@@ -353,19 +353,16 @@ AC_DEFUN([gl_PREREQ_C_STACK],
   [AC_REQUIRE([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC])
    AC_REQUIRE([gl_LIBSIGSEGV])
 
-   # for STACK_DIRECTION
-   AC_REQUIRE([AC_FUNC_ALLOCA])
-
    AC_CHECK_FUNCS_ONCE([sigaltstack])
    AC_CHECK_DECLS([sigaltstack], , , [[#include <signal.h>]])
 
-   AC_CHECK_HEADERS_ONCE([unistd.h ucontext.h])
+   AC_CHECK_HEADERS_ONCE([ucontext.h])
 
    AC_CHECK_TYPES([stack_t], , , [#include <signal.h>])
 
    dnl c-stack does not need -lsigsegv if the system has XSI heuristics.
    if test "$gl_cv_lib_sigsegv" = yes \
-       && test $"ac_cv_sys_xsi_stack_overflow_heuristic" != yes ; then
+       && test "$gl_cv_sys_xsi_stack_overflow_heuristic" != yes; then
      AC_SUBST([LIBCSTACK], [$LIBSIGSEGV])
      AC_SUBST([LTLIBCSTACK], [$LTLIBSIGSEGV])
    fi
diff --git a/modules/c-stack b/modules/c-stack
index ca2c208e1..8c898eb9e 100644
--- a/modules/c-stack
+++ b/modules/c-stack
@@ -7,15 +7,19 @@ lib/c-stack.c
 m4/c-stack.m4
 
 Depends-on:
-gettext-h
 errno
 exitfail
+getprogname
+gettext-h
 ignore-value
-unistd
+intprops
+inttypes
+libsigsegv
 raise
 sigaction
-libsigsegv
-getprogname
+stdbool
+stddef
+unistd
 
 configure.ac:
 gl_C_STACK
-- 
2.17.1


[-- Attachment #3: 0002-c-stack-output-diagnostic-in-single-write.patch --]
[-- Type: text/x-patch, Size: 4292 bytes --]

From 649a39bbd0641c8bb48ba25e5d62d03f8f36125f Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 20 Sep 2020 12:52:15 -0700
Subject: [PATCH 2/2] c-stack: output diagnostic in single 'write'

* lib/c-stack.c (die): In the typical case, use just one 'write'
syscall to output the diagnostic, as this lessens interleaving.
(die, c_stack_action): Assume C99.
* modules/c-stack (Depends-on): Add c99, mempcpy.
---
 ChangeLog       |  6 ++++++
 lib/c-stack.c   | 39 ++++++++++++++++++++++++++++++---------
 modules/c-stack |  2 ++
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 087b9232f..a43c32eb8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2020-09-20  Paul Eggert  <eggert@cs.ucla.edu>
 
+	c-stack: output diagnostic in single 'write'
+	* lib/c-stack.c (die): In the typical case, use just one 'write'
+	syscall to output the diagnostic, as this lessens interleaving.
+	(die, c_stack_action): Assume C99.
+	* modules/c-stack (Depends-on): Add c99, mempcpy.
+
 	c-stack: improve checking if !libsigsegv
 	If SIGINFO_WORKS, do not treat a null pointer dereference as if it
 	were a stack overflow.  Use uintptr_t and INT_ADD_WRAPV to avoid
diff --git a/lib/c-stack.c b/lib/c-stack.c
index 3649c1bfe..742eb023e 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -118,7 +118,6 @@ static char const * volatile progname;
 static _GL_ASYNC_SAFE _Noreturn void
 die (int signo)
 {
-  char const *message;
 # if !SIGINFO_WORKS && !USE_LIBSIGSEGV
   /* We can't easily determine whether it is a stack overflow; so
      assume that the rest of our program is perfect (!) and that
@@ -126,11 +125,34 @@ die (int signo)
   signo = 0;
 # endif
   segv_action (signo);
-  message = signo ? program_error_message : stack_overflow_message;
-  ignore_value (write (STDERR_FILENO, progname, strlen (progname)));
-  ignore_value (write (STDERR_FILENO, ": ", 2));
-  ignore_value (write (STDERR_FILENO, message, strlen (message)));
-  ignore_value (write (STDERR_FILENO, "\n", 1));
+  char const *message = signo ? program_error_message : stack_overflow_message;
+
+  /* If the message is short, write it all at once to avoid
+     interleaving with other messages.  Avoid writev as it is not
+     documented to be async-signal-safe.  */
+  size_t prognamelen = strlen (progname);
+  size_t messagelen = strlen (message);
+  static char const separator[] = {':', ' '};
+  char buf[SIGSTKSZ / 16 + sizeof separator];
+  ptrdiff_t buflen;
+  if (prognamelen + messagelen < sizeof buf - sizeof separator)
+    {
+      char *p = mempcpy (buf, progname, prognamelen);
+      p = mempcpy (p, separator, sizeof separator);
+      p = mempcpy (p, message, messagelen);
+      *p++ = '\n';
+      buflen = p - buf;
+    }
+  else
+    {
+      ignore_value (write (STDERR_FILENO, progname, prognamelen));
+      ignore_value (write (STDERR_FILENO, separator, sizeof separator));
+      ignore_value (write (STDERR_FILENO, message, messagelen));
+      buf[0] = '\n';
+      buflen = 1;
+    }
+  ignore_value (write (STDERR_FILENO, buf, buflen));
+
   if (! signo)
     _exit (exit_failure);
   raise (signo);
@@ -299,9 +321,7 @@ segv_handler (int signo, siginfo_t *info, void *context _GL_UNUSED)
 int
 c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
 {
-  int r;
   stack_t st;
-  struct sigaction act;
   st.ss_flags = 0;
 # if SIGALTSTACK_SS_REVERSED
   /* Irix mistakenly treats ss_sp as the upper bound, rather than
@@ -312,7 +332,7 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
   st.ss_sp = alternate_signal_stack.buffer;
   st.ss_size = sizeof alternate_signal_stack.buffer;
 # endif
-  r = sigaltstack (&st, NULL);
+  int r = sigaltstack (&st, NULL);
   if (r != 0)
     return r;
 
@@ -325,6 +345,7 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
   page_size = sysconf (_SC_PAGESIZE);
 # endif
 
+  struct sigaction act;
   sigemptyset (&act.sa_mask);
 
 # if SIGINFO_WORKS
diff --git a/modules/c-stack b/modules/c-stack
index 8c898eb9e..5ddf6e6f5 100644
--- a/modules/c-stack
+++ b/modules/c-stack
@@ -7,6 +7,7 @@ lib/c-stack.c
 m4/c-stack.m4
 
 Depends-on:
+c99
 errno
 exitfail
 getprogname
@@ -15,6 +16,7 @@ ignore-value
 intprops
 inttypes
 libsigsegv
+mempcpy
 raise
 sigaction
 stdbool
-- 
2.17.1