Re: detecting stack overflow

[Eric Blake]

Eric Blake <ebb9 <at> byu.net> writes:

> Done - the stackovf branch now exists on savannah's m4.git, and includes this 
> subsequent patch that trades core dumps for a nicer error message in case of 
> programmer error (what, we aren't perfect?).  Should I also trap any of 
SIGBUS, 
> SIGILL, SIGFPE, and/or try to make the error message more explicit on which 
> signal was trapped?

Both done as follows, after repairing some bugs that this discovered in both 
gnulib and cygwin in the time since my last stackovf push.  C89 guarantees the 
existence of SIGILL and SIGFPE, but not SIGBUS (and whaddaya know, mingw lacks 
the latter).  I'm still working on a sigaction module that works on mingw, 
which can further simplify this code.  Writing portable async-safe signal 
handlers is tough!  And I still haven't tackled the idea of how to use two 
gnulib-tool libraries on the master branch, instead of our current hack of one 
library and one set of hand-pulled files, nor how to effectively use libsigsegv 
when present, so the stackovf branch can't be merged in to branch-1.6 yet.

>From e0408e69edb2e966f9a4c879124e14ec4fcacf00 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 18 Jun 2008 11:38:30 -0600
Subject: [PATCH] Also trap SIGILL, SIGFPE, SIGBUS.

* m4/gnulib-cache.m4: Import strsignal module.
* src/m4.c (main): Register more handlers, and prefer sigaction
when available.
(SIGBUS, NSIG): Provide fallback when lacking.
(signal_message): New variable, to keep async-safety.
(fault_handler): Display faulting signal description.
* configure.ac (gl_DISABLE_THREADS): Request gnulib modules to
optimize for single-threaded operation.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog          |   12 ++++++++++
 configure.ac       |    3 ++
 m4/gnulib-cache.m4 |    4 +-
 src/m4.c           |   58 ++++++++++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6dc4932..9913112 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2008-06-18  Eric Blake  <address@hidden>
+
+       Also trap SIGILL, SIGFPE, SIGBUS.
+       * m4/gnulib-cache.m4: Import strsignal module.
+       * src/m4.c (main): Register more handlers, and prefer sigaction
+       when available.
+       (SIGBUS, NSIG): Provide fallback when lacking.
+       (signal_message): New variable, to keep async-safety.
+       (fault_handler): Display faulting signal description.
+       * configure.ac (gl_DISABLE_THREADS): Request gnulib modules to
+       optimize for single-threaded operation.
+
 2008-06-06  Eric Blake  <address@hidden>
 
        Inform users what to do in case of programmer error.
diff --git a/configure.ac b/configure.ac
index 3456d8f..f23cc42 100644
--- a/configure.ac
+++ b/configure.ac
@@ -32,6 +32,9 @@ AC_CONFIG_HEADERS([lib/config.h:lib/config.hin])
 AC_PROG_CC
 M4_EARLY
 
+# M4 is single-threaded; so we can optimize gnulib code by using this:
+gl_DISABLE_THREADS
+
 # Tandem/NSK is broken - it has 'long long int' but not
 # 'unsigned long long int', which confuses assumptions made by gnulib.
 # Simply pretend that neither type exists if both do not work.
diff --git a/m4/gnulib-cache.m4 b/m4/gnulib-cache.m4
index 7c1fb0d..7f8d10b 100644
--- a/m4/gnulib-cache.m4
+++ b/m4/gnulib-cache.m4
@@ -15,11 +15,11 @@
 
 
 # Specification in the form of a command-line invocation:
-#   gnulib-tool --import --dir=. --local-dir=local --lib=libm4 --source-
base=lib --m4-base=m4 --doc-base=doc --aux-dir=build-aux --with-tests --no-
libtool --macro-prefix=M4 announce-gen assert autobuild avltree-oset binary-io 
c-stack clean-temp cloexec close-stream closein config-h error fdl fflush 
flexmember fopen-safer fseeko gendocs getopt git-version-gen gnumakefile 
gnupload gpl-3.0 intprops memchr2 memmem mkstemp obstack progname quote regex 
stdbool stdint stdlib-safer strtod strtol unlocked-io vasnprintf-posix verror 
version-etc version-etc-fsf xalloc xmemdup0 xprintf xvasprintf-posix
+#   gnulib-tool --import --dir=. --local-dir=local --lib=libm4 --source-
base=lib --m4-base=m4 --doc-base=doc --aux-dir=build-aux --with-tests --no-
libtool --macro-prefix=M4 announce-gen assert autobuild avltree-oset binary-io 
c-stack clean-temp cloexec close-stream closein config-h error fdl fflush 
flexmember fopen-safer fseeko gendocs getopt git-version-gen gnumakefile 
gnupload gpl-3.0 intprops memchr2 memmem mkstemp obstack progname quote regex 
stdbool stdint stdlib-safer strsignal strtod strtol unlocked-io vasnprintf-
posix verror version-etc version-etc-fsf xalloc xmemdup0 xprintf xvasprintf-
posix
 
 # Specification in the form of a few gnulib-tool.m4 macro invocations:
 gl_LOCAL_DIR([local])
-gl_MODULES([announce-gen assert autobuild avltree-oset binary-io c-stack clean-
temp cloexec close-stream closein config-h error fdl fflush flexmember fopen-
safer fseeko gendocs getopt git-version-gen gnumakefile gnupload gpl-3.0 
intprops memchr2 memmem mkstemp obstack progname quote regex stdbool stdint 
stdlib-safer strtod strtol unlocked-io vasnprintf-posix verror version-etc 
version-etc-fsf xalloc xmemdup0 xprintf xvasprintf-posix])
+gl_MODULES([announce-gen assert autobuild avltree-oset binary-io c-stack clean-
temp cloexec close-stream closein config-h error fdl fflush flexmember fopen-
safer fseeko gendocs getopt git-version-gen gnumakefile gnupload gpl-3.0 
intprops memchr2 memmem mkstemp obstack progname quote regex stdbool stdint 
stdlib-safer strsignal strtod strtol unlocked-io vasnprintf-posix verror 
version-etc version-etc-fsf xalloc xmemdup0 xprintf xvasprintf-posix])
 gl_AVOID([])
 gl_SOURCE_BASE([lib])
 gl_M4_BASE([m4])
diff --git a/src/m4.c b/src/m4.c
index f2c7c61..a6665ff 100644
--- a/src/m4.c
+++ b/src/m4.c
@@ -178,9 +178,23 @@ m4_warn (int errnum, const call_info *caller, const char 
*format, ...)
     }
 }
 
-/* Translated message for program errors.  Do not translate it in the
-   signal handler, since gettext is not async-signal-safe.  */
+#ifndef SIGBUS
+# define SIGBUS SIGILL
+#endif
+
+#ifndef NSIG
+# ifndef MAX
+#  define MAX(a,b) ((a) < (b) ? (b) : (a))
+# endif
+# define NSIG (MAX (SIGABRT, MAX (SIGILL, MAX (SIGFPE,  \
+                                               MAX (SIGSEGV, SIGBUS)))) + 1)
+#endif
+
+/* Pre-translated messages for program errors.  Do not translate in
+   the signal handler, since gettext and strsignal are not
+   async-signal-safe.  */
 static const char * volatile program_error_message;
+static const char * volatile signal_message[NSIG];
 
 /* Print a nicer message about any programmer errors, then exit.  This
    must be aysnc-signal safe, since it is executed as a signal
@@ -191,10 +205,21 @@ fault_handler (int signo)
 {
   if (signo)
     {
+      /* POSIX states that reading static memory is, in general, not
+         async-safe.  However, the static variables that we read are
+         never modified once this handler is installed, so this
+         particular usage is safe.  And it seems an oversight that
+         POSIX claims strlen is not async-safe.  */
       write (STDERR_FILENO, program_name, strlen (program_name));
       write (STDERR_FILENO, ": ", 2);
       write (STDERR_FILENO, program_error_message,
              strlen (program_error_message));
+      if (signal_message[signo])
+        {
+          write (STDERR_FILENO, ": ", 2);
+          write (STDERR_FILENO, signal_message[signo],
+                 strlen (signal_message[signo]));
+        }
       write (STDERR_FILENO, "\n", 1);
       _exit (EXIT_INTERNAL_ERROR);
     }
@@ -438,14 +463,39 @@ main (int argc, char *const *argv, char *const *envp)
   set_quoting_style (NULL, escape_quoting_style);
   set_char_quoting (NULL, ':', 1);
 
-  /* Stack overflow and program error handling.  */
+  /* Stack overflow and program error handling.  Ignore failure to
+     install a handler, since this is merely for improved output on
+     crash, and we should never crash ;).  */
   if (c_stack_action (fault_handler) == 0)
     nesting_limit = 0;
   program_error_message
     = xasprintf (_("internal error detected; please report this bug to <%s>"),
                  PACKAGE_BUGREPORT);
-  /* FIXME - use sigaction.  */
+  signal_message[SIGSEGV] = xstrdup (strsignal (SIGSEGV));
+  signal_message[SIGABRT] = xstrdup (strsignal (SIGABRT));
+  signal_message[SIGILL] = xstrdup (strsignal (SIGILL));
+  signal_message[SIGFPE] = xstrdup (strsignal (SIGFPE));
+  if (SIGBUS != SIGILL)
+    signal_message[SIGBUS] = xstrdup (strsignal (SIGBUS));
+#ifdef HAVE_SIGACTION
+  {
+    struct sigaction act;
+    sigemptyset (&act.sa_mask);
+    /* One-shot - if we fault while handling a fault, we want to
+       revert to default signal behavior.  */
+    act.sa_flags = SA_NODEFER | SA_RESETHAND;
+    act.sa_handler = fault_handler;
+    sigaction (SIGABRT, &act, NULL);
+    sigaction (SIGILL, &act, NULL);
+    sigaction (SIGFPE, &act, NULL);
+    sigaction (SIGBUS, &act, NULL);
+  }
+#else /* !HAVE_SIGACTION */
   signal (SIGABRT, fault_handler);
+  signal (SIGILL, fault_handler);
+  signal (SIGFPE, fault_handler);
+  signal (SIGBUS, fault_handler);
+#endif /* !HAVE_SIGACTION */
 
   /* First, we decode the arguments, to size up tables and stuff.  */
 
-- 
1.5.5.1