Re: detecting stack overflow

[Eric Blake]

Gary V. Vaughan <necro.no <at> mac.com> writes:

> > At any rate, here's the patch for playing with.  Should I create a new
> > branch in m4.git with this patch (and subsequent patches for  
> > libsigsegv),
> > to make it easier for others to play with this branch until it is  
> > ready to
> > merge into mainline branch-1.6?
> 
> Yes, please! :)

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 SIGBUG, 
SIGILL, SIGFPE, and/or try to make the error message more explicit on which 
signal was trapped?  Should I add an #ifndef DEBUG around registering the trap 
handlers, so that developers can get a core file for easier backtrace rather 
than just a cutesy handler message?  I've verified that on Solaris, where 
ucontext_t->uc_stack reports accurate information, that this code distinguishes 
between stack overflow:

$ src/m4; echo $?
define(a,a(a))a
src/m4: stack overflow
1

and a manually inserted "int foo = *(int*)0;" or assertion failure:

$ src/m4; echo $?
Assertion failed: !"oops", file m4.c, line 447
src/m4: internal error detected; please report this bug to <address@hidden>
2

On Linux, where ucontext_t->uc_stack is bogus, you can't distinguish random 
SEGV from stack overflow without libsigsegv, but the assertion failure works 
nicely.

On cygwin, where sigaltstack() is lacking, the code dumps core on SEGV (without 
an alternate stack, you can trap some SEGV, but not stack overflow, and since 
the assumption is our code is perfect! the only SEGV worth trapping is stack 
overflow), but that's no worse than before and will be fixed once I get 
libsigsegv integrated.  Likewise, the assertion failure changes the exit 
status, but because of a cygwin bug in abort(), it fails to print the bug 
report address because stderr is prematurely closed.

>From 0b2dea68c4f3e2f6cca84291536b80b3c4eac058 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 6 Jun 2008 16:37:21 -0600
Subject: [PATCH] Inform users what to do in case of programmer error.

* src/m4.h (EXIT_INTERNAL_ERROR): New macro.
* configure.ac (AC_TYPE_SIGNAL): Delete, now that we assume C89.
* src/m4.c (fault_handler): New method.
(program_error_message): New variable, for async-safety.
(main): Print bug reporting address rather than dump core on any
failed assertions or detected non-stack-overflow faults.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog    |    8 ++++++++
 configure.ac |    2 --
 src/m4.c     |   31 ++++++++++++++++++++++++++++++-
 src/m4.h     |    3 +++
 4 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 18c8301..6dc4932 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2008-06-06  Eric Blake  <address@hidden>
 
+       Inform users what to do in case of programmer error.
+       * src/m4.h (EXIT_INTERNAL_ERROR): New macro.
+       * configure.ac (AC_TYPE_SIGNAL): Delete, now that we assume C89.
+       * src/m4.c (fault_handler): New method.
+       (program_error_message): New variable, for async-safety.
+       (main): Print bug reporting address rather than dump core on any
+       failed assertions or detected non-stack-overflow faults.
+
        Replace stackovf with gnulib c-stack.
        * m4/gnulib.cache.m4: Import c-stack module.
        * configure.ac (AC_CHECK_HEADERS_ONCE): Remove check for
diff --git a/configure.ac b/configure.ac
index 3fb059b..3456d8f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -32,8 +32,6 @@ AC_CONFIG_HEADERS([lib/config.h:lib/config.hin])
 AC_PROG_CC
 M4_EARLY
 
-AC_TYPE_SIGNAL
-
 # 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/src/m4.c b/src/m4.c
index 63dcc20..f2c7c61 100644
--- a/src/m4.c
+++ b/src/m4.c
@@ -177,6 +177,28 @@ m4_warn (int errnum, const call_info *caller, const char 
*format, ...)
       va_end (args);
     }
 }
+
+/* Translated message for program errors.  Do not translate it in the
+   signal handler, since gettext is not async-signal-safe.  */
+static const char * volatile program_error_message;
+
+/* Print a nicer message about any programmer errors, then exit.  This
+   must be aysnc-signal safe, since it is executed as a signal
+   handler.  If SIGNO is zero, this represents a stack overflow; in
+   that case, we return to allow c_stack_action to handle things.  */
+static void
+fault_handler (int signo)
+{
+  if (signo)
+    {
+      write (STDERR_FILENO, program_name, strlen (program_name));
+      write (STDERR_FILENO, ": ", 2);
+      write (STDERR_FILENO, program_error_message,
+             strlen (program_error_message));
+      write (STDERR_FILENO, "\n", 1);
+      _exit (EXIT_INTERNAL_ERROR);
+    }
+}
 
 
 /*---------------------------------------------.
@@ -415,8 +437,15 @@ main (int argc, char *const *argv, char *const *envp)
   debug_init ();
   set_quoting_style (NULL, escape_quoting_style);
   set_char_quoting (NULL, ':', 1);
-  if (c_stack_action (NULL) == 0)
+
+  /* Stack overflow and program error handling.  */
+  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 (SIGABRT, fault_handler);
 
   /* First, we decode the arguments, to size up tables and stuff.  */
 
diff --git a/src/m4.h b/src/m4.h
index 6290122..a38a2f8 100644
--- a/src/m4.h
+++ b/src/m4.h
@@ -73,6 +73,9 @@
 # undef UNIX
 #endif
 
+/* Used if any programmer error is detected (not possible, right?)  */
+#define EXIT_INTERNAL_ERROR 2
+
 /* Used for version mismatch, when -R detects a frozen file it can't parse.  */
 #define EXIT_MISMATCH 63
 
-- 
1.5.5.1