Re: c-stack: use libsigsegv when possible

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Eric Blake on 7/16/2008 9:11 PM:
| According to Bruno Haible on 7/16/2008 7:29 PM:
| | If you want to save linking with libsigsegv, just do the test
| | '#if HAVE_LIBSIGSEGV && ! HAVE_XSI_STACK_OVERFLOW_HEURISTIC' at configure
| | time rather than at compile-time, create two new variables LIBCSTACK and
| | LTLIBCSTACK, AC_SUBST them, and document in modudes/c-stack that the
| | appropriate of these two variables needs to be used while linking.
|
| OK, I'll rework the patch along those lines.

Committed as follows (and this is what exposed the libsigsegv on OpenBSD 4.0).

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkh/SiQACgkQ84KuGfSFAYBUMACdG3gs071yC4mEGenrCshrGZf9
WMIAoL6L3OaUSFmjz9QlXLUrbfZ1M0yK
=tZPv
-----END PGP SIGNATURE-----

>From f6a65c23356574113f91060f82cef2119af4949a Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 17 Jul 2008 06:23:47 -0600
Subject: [PATCH] Make c-stack use libsigsegv, when available.

* modules/c-stack (Depends-on): Add libsigsegv.
* modules/c-stack-tests (Makefile.am): Link with libsigsegv, if
needed.
* lib/c-stack.c (SIGSTKSZ): Define fallback.
(segv_handler, overflow_handler, c_stack_action)
[HAVE_LIBSIGSEGV && !HAVE_XSI_STACK_OVERFLOW_HEURISTIC]: Add new
implementation when libsigsegv is available, but only when using
the library is necessary.
* m4/c-stack.m4 (AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC): Add
comment, explaining why XSI check fails on Linux.
(gl_PREREQ_C_STACK): Supply LIBCSTACK, LTLIBCSTACK.
* tests/test-c-stack2.sh: Tweak skip message.
* NEWS: Document new link-time requirements.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog              |   17 ++++++
 NEWS                   |    4 +
 lib/c-stack.c          |  150 +++++++++++++++++++++++++++++++++++++++++-------
 m4/c-stack.m4          |   16 +++++-
 modules/c-stack        |    4 +
 modules/c-stack-tests  |    2 +-
 tests/test-c-stack2.sh |    2 +-
 7 files changed, 169 insertions(+), 26 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0772f2d..01f0084 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2008-07-17  Eric Blake  <address@hidden>
+
+       Make c-stack use libsigsegv, when available.
+       * modules/c-stack (Depends-on): Add libsigsegv.
+       * modules/c-stack-tests (Makefile.am): Link with libsigsegv, if
+       needed.
+       * lib/c-stack.c (SIGSTKSZ): Define fallback.
+       (segv_handler, overflow_handler, c_stack_action)
+       [HAVE_LIBSIGSEGV && !HAVE_XSI_STACK_OVERFLOW_HEURISTIC]: Add new
+       implementation when libsigsegv is available, but only when using
+       the library is necessary.
+       * m4/c-stack.m4 (AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC): Add
+       comment, explaining why XSI check fails on Linux.
+       (gl_PREREQ_C_STACK): Supply LIBCSTACK, LTLIBCSTACK.
+       * tests/test-c-stack2.sh: Tweak skip message.
+       * NEWS: Document new link-time requirements.
+
 2008-07-16  Eric Blake  <address@hidden>
 
        c-stack: Expose false positives when not using libsigsegv.
diff --git a/NEWS b/NEWS
index 74ca234..da75d7f 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,10 @@ User visible incompatible changes
 
 Date        Modules         Changes
 
+2008-07-17  c-stack         The module now requires the addition of
+                            $(LIBCSTACK) or $(LTLIBCSTACK) in Makefile.am,
+                            since it may depend on linking with libsigsegv.
+
 2008-07-07  isnanf-nolibm   The include file is changed from "isnanf.h"
                             to "isnanf-nolibm.h".
             isnand-nolibm   The include file is changed from "isnand.h"
diff --git a/lib/c-stack.c b/lib/c-stack.c
index 96bd2bf..c81fbfa 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -53,6 +53,9 @@
 #if ! HAVE_STACK_T && ! defined stack_t
 typedef struct sigaltstack stack_t;
 #endif
+#ifndef SIGSTKSZ
+# define SIGSTKSZ 16384
+#endif
 
 #include <stdlib.h>
 #include <string.h>
@@ -68,6 +71,10 @@ typedef struct sigaltstack stack_t;
 # define STDERR_FILENO 2
 #endif
 
+#if HAVE_LIBSIGSEGV
+# include <sigsegv.h>
+#endif
+
 #include "c-stack.h"
 #include "exitfail.h"
 
@@ -110,7 +117,115 @@ die (int signo)
   abort ();
 }
 
-#if HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK
+#if (HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK) || 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;
+} alternate_signal_stack;
+
+static void
+null_action (int signo __attribute__ ((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
+
+/* Nonzero if general segv handler could not be installed.  */
+static volatile int segv_handler_missing;
+
+/* Handle a segmentation violation and exit if it cannot be stack
+   overflow.  This function is async-signal-safe.  */
+
+static int segv_handler (void *address __attribute__ ((unused)),
+                        int serious)
+{
+# if DEBUG
+  {
+    char buf[1024];
+    sprintf (buf, "segv_handler serious=%d\n", serious);
+    write (STDERR_FILENO, buf, strlen (buf));
+  }
+# endif
+
+  /* If this fault is not serious, return 0 to let the stack overflow
+     handler take a shot at it.  */
+  if (!serious)
+    return 0;
+  die (SIGSEGV);
+}
+
+/* Handle a segmentation violation that is likely to be a stack
+   overflow and exit.  This function is async-signal-safe.  */
+
+static void overflow_handler (int, stackoverflow_context_t)
+  __attribute__ ((noreturn));
+static void
+overflow_handler (int emergency,
+                 stackoverflow_context_t context __attribute__ ((unused)))
+{
+# if DEBUG
+  {
+    char buf[1024];
+    sprintf (buf, "overflow_handler emergency=%d segv_handler_missing=%d\n",
+            emergency, segv_handler_missing);
+    write (STDERR_FILENO, buf, strlen (buf));
+  }
+# endif
+
+  die ((!emergency || segv_handler_missing) ? 0 : SIGSEGV);
+}
+
+/* Set up ACTION so that it is invoked on C stack overflow.  Return -1
+   (setting errno) if this cannot be done.
+
+   When ACTION is called, it is passed an argument equal to SIGSEGV
+   for a segmentation violation that does not appear related to stack
+   overflow, and is passed zero otherwise.  On many platforms it is
+   hard to tell; when in doubt, zero is passed.
+
+   A null ACTION acts like an action that does nothing.
+
+   ACTION must be async-signal-safe.  ACTION together with its callees
+   must not require more than SIGSTKSZ bytes of stack space.  Also,
+   ACTION should not call longjmp, because this implementation does
+   not guarantee that it is safe to return to the original stack.  */
+
+int
+c_stack_action (void (*action) (int))
+{
+  segv_action = action ? action : null_action;
+  program_error_message = _("program error");
+  stack_overflow_message = _("stack overflow");
+
+  /* Always install the overflow handler.  */
+  if (stackoverflow_install_handler (overflow_handler,
+                                     alternate_signal_stack.buffer,
+                                     sizeof alternate_signal_stack.buffer))
+    {
+      errno = ENOTSUP;
+      return -1;
+    }
+  /* Try installing a general handler; if it fails, then treat all
+     segv as stack overflow.  */
+  segv_handler_missing = sigsegv_install_handler (segv_handler);
+  return 0;
+}
+
+#elif HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK
 
 /* Direction of the C runtime stack.  This function is
    async-signal-safe.  */
@@ -126,19 +241,6 @@ find_stack_direction (char const *addr)
 }
 # endif
 
-/* 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;
-} alternate_signal_stack;
-
 # if SIGACTION_WORKS
 
 /* Handle a segmentation violation and exit.  This function is
@@ -155,7 +257,14 @@ segv_handler (int signo, siginfo_t *info,
 #  if ! HAVE_XSI_STACK_OVERFLOW_HEURISTIC
       /* 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.  */
+        this segmentation violation is a stack overflow.
+
+        Note that although both Linux and Solaris provide
+        sigaltstack, SA_ONSTACK, and SA_SIGINFO, currently only
+        Solaris satisfies the XSI heueristic.  This is because
+        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 the faulting address is within the stack, or within one
@@ -189,11 +298,6 @@ segv_handler (int signo, siginfo_t *info,
 }
 # endif
 
-static void
-null_action (int signo __attribute__ ((unused)))
-{
-}
-
 /* Set up ACTION so that it is invoked on C stack overflow.  Return -1
    (setting errno) if this cannot be done.
 
@@ -205,7 +309,9 @@ null_action (int signo __attribute__ ((unused)))
    A null ACTION acts like an action that does nothing.
 
    ACTION must be async-signal-safe.  ACTION together with its callees
-   must not require more than SIGSTKSZ bytes of stack space.  */
+   must not require more than SIGSTKSZ bytes of stack space.  Also,
+   ACTION should not call longjmp, because this implementation does
+   not guarantee that it is safe to return to the original stack.  */
 
 int
 c_stack_action (void (*action) (int))
@@ -240,7 +346,7 @@ c_stack_action (void (*action) (int))
   return sigaction (SIGSEGV, &act, NULL);
 }
 
-#else /* ! (HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK) */
+#else /* ! ((HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK) || HAVE_LIBSIGSEGV) */
 
 int
 c_stack_action (void (*action) (int)  __attribute__ ((unused)))
diff --git a/m4/c-stack.m4 b/m4/c-stack.m4
index f142171..b1f93ed 100644
--- a/m4/c-stack.m4
+++ b/m4/c-stack.m4
@@ -7,7 +7,7 @@
 
 # Written by Paul Eggert.
 
-# serial 2
+# serial 3
 
 AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
   [# for STACK_DIRECTION
@@ -55,6 +55,9 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
         {
           if (0 < info->si_code)
             {
+               /* For XSI heuristics to work, we need uc_stack to describe
+                 the interrupted stack (as on Solaris), and not the
+                 currently executing stack (as on Linux).  */
               ucontext_t const *user_context = context;
               char const *stack_min = user_context->uc_stack.ss_sp;
               size_t stack_size = user_context->uc_stack.ss_size;
@@ -133,6 +136,7 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
 
 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])
@@ -144,7 +148,15 @@ AC_DEFUN([gl_PREREQ_C_STACK],
 
    AC_CHECK_MEMBERS([struct sigaction.sa_sigaction], , , [#include <signal.h>])
 
-   AC_CHECK_TYPES([stack_t], , , [#include <signal.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
+     AC_SUBST([LIBCSTACK], [$LIBSIGSEGV])
+     AC_SUBST([LTLIBCSTACK], [$LTLIBSIGSEGV])
+   fi
+])
 
 AC_DEFUN([gl_C_STACK],
 [
diff --git a/modules/c-stack b/modules/c-stack
index d3e7e09..a01b8c7 100644
--- a/modules/c-stack
+++ b/modules/c-stack
@@ -12,6 +12,7 @@ exitfail
 unistd
 raise
 sigaction
+libsigsegv
 
 configure.ac:
 gl_C_STACK
@@ -22,6 +23,9 @@ lib_SOURCES += c-stack.h c-stack.c
 Include:
 "c-stack.h"
 
+Link:
+$(LTCSTACK) when linking with libtool, $(LIBCSTACK) otherwise
+
 License:
 GPL
 
diff --git a/modules/c-stack-tests b/modules/c-stack-tests
index 5d3bc58..4cf15f2 100644
--- a/modules/c-stack-tests
+++ b/modules/c-stack-tests
@@ -12,5 +12,5 @@ Makefile.am:
 TESTS += test-c-stack.sh test-c-stack2.sh
 TESTS_ENVIRONMENT += EXEEXT='@EXEEXT@'
 check_PROGRAMS += test-c-stack
-test_c_stack_LDADD = $(LDADD) @LIBINTL@
+test_c_stack_LDADD = $(LDADD) $(LIBCSTACK) @LIBINTL@
 MOSTLYCLEANFILES += t-c-stack.tmp t-c-stack2.tmp
diff --git a/tests/test-c-stack2.sh b/tests/test-c-stack2.sh
index e55ff17..039a819 100755
--- a/tests/test-c-stack2.sh
+++ b/tests/test-c-stack2.sh
@@ -11,7 +11,7 @@ tmpfiles="t-c-stack2.tmp"
 
 case $? in
   77) if grep 'stack overflow' t-c-stack2.tmp >/dev/null ; then
-       echo 'cannot distinguish stack overflow from crash' >&2
+       echo 'cannot tell stack overflow from crash; consider installing 
libsigsegv' >&2
       else
        cat t-c-stack2.tmp >&2
       fi
-- 
1.5.6