Re: stackovf implementation [Was: regex.c not 64-bit clean (?)]

[Eric Blake]

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

According to Eric Blake on 6/15/2006 6:18 AM:
> According to Santiago Vila on 6/15/2006 2:53 AM:
>>> if gcc -DHAVE_CONFIG_H -I. -I. -I..  -I ../lib   -Wall -Werror -MT 
>>> stackovf.o -MD -MP -MF ".deps/stackovf.Tpo" -c -o stackovf.o stackovf.c; \
>>>     then mv -f ".deps/stackovf.Tpo" ".deps/stackovf.Po"; else rm -f 
>>> ".deps/stackovf.Tpo"; exit 1; fi
>>> cc1: warnings being treated as errors
>>> stackovf.c: In function 'setup_stackovf_trap':
>>> stackovf.c:344: warning: 'sigstack' is deprecated (declared at 
>>> /usr/include/signal.h:358)
>>> make[2]: *** [stackovf.o] Error 1
> 
> If that compiles warning-free, then I will see about changing stackovf to
> favor sigaltstack over sigstack; I can find a Linux machine to test on.

After looking more at POSIX, and testing on a Solaris 8 machine, I propose
the following patch.  Santiago, can you test it on ia64, and see if it
gets rid of the warning you were seeing?

2006-06-20  Eric Blake  <address@hidden>

        Avoid obsolete sigstack when POSIX sigaltstack is available:
        * src/m4.c: Blindly assume signal.h, since stackovf.c and gnulib
        do likewise.
        * configure.ac (AC_CHECK_HEADERS): Likewise.
        (AC_CHECK_TYPES): New check for siginfo_t, since siginfo.h is
        obsolete and most hosts now have it in signal.h.
        (AC_CHECK_MEMBERS): New check for sigaction.sa_sigaction.
        (AC_CACHE_CHECK): Cache decision to use stackovf.
        (AC_EGREP_HEADER): Switch to AC_CHECK_TYPES.
        * src/stackovf.c (DEBUG_STACKOVF): Remove unused define.
        (SA_RESETHAND, SA_SIGINFO): Provide fallback definitions, to
        simplify later code.
        (PARAM_STACKOVF, PARAM_NOSTACKOVF): Move further away from NULL,
        in case of dereferencing a member of a NULL pointer.
        (sigsegv_handler) [HAVE_STRUCT_SIGACTION_SA_SIGACTION]: Define a
        POSIX handler.
        (setup_stackovf_trap): Use NULL instead of 0 for pointers, use
        EXIT_FAILURE in error, indent preprocessor directives.
        [HAVE_SIGALTSTACK && HAVE_SIGINFO_T]: Depend on siginfo_t, not
        siginfo.h.
        [HAVE_SIGACTION && defined SA_ONSTACK]: Prefer POSIX handler.
        Reported by Santiago Vila.

- --
Life is short - so eat dessert first!

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

iD8DBQFEmEl884KuGfSFAYARAhiiAJsFhs5pu9EgVqAkddHa6N+tucKs7gCfdFXm
Qe9DXOMpSXO+tmWm9hdMQw8=
=Wi8L
-----END PGP SIGNATURE-----

Index: ChangeLog
===================================================================
RCS file: /sources/m4/m4/ChangeLog,v
retrieving revision 1.1.1.1.2.76
diff -u -r1.1.1.1.2.76 ChangeLog
--- ChangeLog   20 Jun 2006 03:22:53 -0000      1.1.1.1.2.76
+++ ChangeLog   20 Jun 2006 18:20:42 -0000
@@ -1,3 +1,28 @@
+2006-06-20  Eric Blake  <address@hidden>
+
+       Avoid obsolete sigstack when POSIX sigaltstack is available:
+       * src/m4.c: Blindly assume signal.h, since stackovf.c and gnulib
+       do likewise.
+       * configure.ac (AC_CHECK_HEADERS): Likewise.
+       (AC_CHECK_TYPES): New check for siginfo_t, since siginfo.h is
+       obsolete and most hosts now have it in signal.h.
+       (AC_CHECK_MEMBERS): New check for sigaction.sa_sigaction.
+       (AC_CACHE_CHECK): Cache decision to use stackovf.
+       (AC_EGREP_HEADER): Switch to AC_CHECK_TYPES.
+       * src/stackovf.c (DEBUG_STACKOVF): Remove unused define.
+       (SA_RESETHAND, SA_SIGINFO): Provide fallback definitions, to
+       simplify later code.
+       (PARAM_STACKOVF, PARAM_NOSTACKOVF): Move further away from NULL,
+       in case of dereferencing a member of a NULL pointer.
+       (sigsegv_handler) [HAVE_STRUCT_SIGACTION_SA_SIGACTION]: Define a
+       POSIX handler.
+       (setup_stackovf_trap): Use NULL instead of 0 for pointers, use
+       EXIT_FAILURE in error, indent preprocessor directives.
+       [HAVE_SIGALTSTACK && HAVE_SIGINFO_T]: Depend on siginfo_t, not
+       siginfo.h.
+       [HAVE_SIGACTION && defined SA_ONSTACK]: Prefer POSIX handler.
+       Reported by Santiago Vila.
+
 2006-06-19  Eric Blake  <address@hidden>
 
        * THANKS: Update.
Index: configure.ac
===================================================================
RCS file: /sources/m4/m4/configure.ac,v
retrieving revision 1.36.2.14
diff -u -r1.36.2.14 configure.ac
--- configure.ac        18 Jun 2006 20:38:02 -0000      1.36.2.14
+++ configure.ac        20 Jun 2006 18:20:42 -0000
@@ -24,6 +24,7 @@
 VERSION=$PACKAGE_VERSION; AC_SUBST([VERSION])
 
 m4_pattern_forbid([^M4_])
+m4_pattern_allow([^M4_cv_])
 
 AC_CONFIG_SRCDIR([src/m4.h])
 AC_CONFIG_HEADERS([config.h:config-h.in])
@@ -32,7 +33,16 @@
 M4_EARLY
 
 AC_CHECK_HEADERS([limits.h siginfo.h])
-AC_CHECK_HEADERS([signal.h sys/signal.h], [break])
+AC_CHECK_TYPES([siginfo_t], [], [],
+[[#include <signal.h>
+#if HAVE_SIGINFO_H
+# include <siginfo.h>
+#endif
+]])
+AC_CHECK_MEMBERS([struct sigaction.sa_sigaction], [], [],
+[[#include <signal.h>
+]])
+
 AC_TYPE_SIGNAL
 AC_TYPE_SIZE_T
 
@@ -50,42 +60,43 @@
   [AC_MSG_RESULT([no]); AC_CHECK_FUNCS([ecvt])])
 
 
-AC_MSG_CHECKING([if stack overflow is detectable])
 # Code from Jim Avera <address@hidden>.
 # stackovf.c requires:
 #  1. Either sigaction with SA_ONSTACK, or sigvec with SV_ONSTACK
 #  2. Either sigaltstack or sigstack
 #  3. getrlimit, including support for RLIMIT_STACK
-use_stackovf=no
+AC_CACHE_CHECK([if stack overflow is detectable], [M4_cv_use_stackovf],
+[M4_cv_use_stackovf=no
 if test "$ac_cv_func_sigaction" = yes || test "$ac_cv_func_sigvec" = yes; then
   if test "$ac_cv_func_sigaltstack" = yes || test "$ac_cv_func_sigstack" = 
yes; then
-    AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include <sys/time.h>
+    AC_LINK_IFELSE([AC_LANG_PROGRAM([[
+#include <sys/time.h>
 #include <sys/resource.h>
-#include <signal.h>]],
-      [[struct rlimit r; int i; getrlimit (RLIMIT_STACK, &r)
+#include <signal.h>
+]], [[struct rlimit r; getrlimit (RLIMIT_STACK, &r);
 #if (!defined(HAVE_SIGACTION) || !defined(SA_ONSTACK)) \
     && (!defined(HAVE_SIGVEC) || !defined(SV_ONSTACK))
 choke me               /* SA_ONSTACK and/or SV_ONSTACK are not defined */
-#endif]])],[use_stackovf=yes],[])
+#endif
+]])], [M4_cv_use_stackovf=yes])
   fi
-fi
-AC_MSG_RESULT([$use_stackovf])
-AM_CONDITIONAL([STACKOVF], [test "$use_stackovf" = yes])
-if test "$use_stackovf" = yes; then
+fi])
+AM_CONDITIONAL([STACKOVF], [test "$M4_cv_use_stackovf" = yes])
+if test "$M4_cv_use_stackovf" = yes; then
   AC_DEFINE([USE_STACKOVF], [1],
     [Define to 1 if using stack overflow detection])
-  AC_EGREP_HEADER([rlim_t], [sys/resource.h], [],
+  AC_CHECK_TYPES([rlim_t], [],
     [AC_DEFINE([rlim_t], [int],
-      [Define to int if rlim_t is not defined in sys/resource.h])
-  ])
-  AC_EGREP_HEADER([stack_t], [signal.h], [],
+      [Define to int if rlim_t is not defined in sys/resource.h])],
+    [[#include <sys/resource.h>
+  ]])
+  AC_CHECK_TYPES([stack_t], [],
     [AC_DEFINE([stack_t], [struct sigaltstack],
-      [Define to struct sigaltstack if stack_t is not in sys/signal.h])
-  ])
-  AC_EGREP_HEADER([sigcontext], [signal.h],
-    [AC_DEFINE([HAVE_SIGCONTEXT], [1],
-      [Define to 1 if signal.h declares struct sigcontext])
-  ])
+      [Define to struct sigaltstack if stack_t is not in signal.h])],
+    [[#include <signal.h>
+  ]])
+  AC_CHECK_TYPES([sigcontext], [], [], [[#include <signal.h>
+  ]])
 fi
 
 dnl Don't let changeword get in our way, if bootstrapping with a version of
Index: src/m4.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/m4.c,v
retrieving revision 1.1.1.1.2.10
diff -u -r1.1.1.1.2.10 m4.c
--- src/m4.c    15 Jun 2006 13:08:46 -0000      1.1.1.1.2.10
+++ src/m4.c    20 Jun 2006 18:20:42 -0000
@@ -22,13 +22,7 @@
 #include "m4.h"
 
 #include <getopt.h>
-
-#if defined(HAVE_SYS_SIGNAL_H)
-#  include <sys/signal.h>
-#endif
-#if defined(HAVE_SIGNAL_H)
-#  include <signal.h>
-#endif
+#include <signal.h>
 
 static void usage _((int));
 
Index: src/stackovf.c
===================================================================
RCS file: /sources/m4/m4/src/stackovf.c,v
retrieving revision 1.1.1.1.2.1
diff -u -r1.1.1.1.2.1 stackovf.c
--- src/stackovf.c      1 May 2005 11:54:12 -0000       1.1.1.1.2.1
+++ src/stackovf.c      20 Jun 2006 18:20:42 -0000
@@ -1,5 +1,5 @@
 /* Detect stack overflow (when getrlimit and sigaction or sigvec are available)
-   Copyright (C) 1993, 1994 Free Software Foundation, Inc.
+   Copyright (C) 1993, 1994, 2006 Free Software Foundation, Inc.
    Jim Avera <address@hidden>, October 1993.
 
    This program is free software; you can redistribute it and/or modify
@@ -75,8 +75,6 @@
       dump in the usual way.  It seems important (to me) that internal m4
       bugs not be reported as user recursion errors, or vice-versa."  */
 
-#define DEBUG_STACKOVF
-
 #include "m4.h"                        /* stdlib.h, xmalloc() */
 
 #include <assert.h>
@@ -88,6 +86,13 @@
 # include <siginfo.h>
 #endif
 
+#ifndef SA_RESETHAND
+# define SA_RESETHAND 0
+#endif
+#ifndef SA_SIGINFO
+# define SA_SIGINFO 0
+#endif
+
 #ifndef SIGSTKSZ
 # define SIGSTKSZ 8192
 #endif
@@ -136,8 +141,8 @@
    message and abort with a core dump.  This only occurs on systems which
    provide no information, but is better than nothing.  */
 
-#define PARAM_STACKOVF ((const char *) 1)
-#define PARAM_NOSTACKOVF ((const char *) 2)
+#define PARAM_STACKOVF ((const char *) (1 + STACKOVF_DETECT))
+#define PARAM_NOSTACKOVF ((const char *) (2 + STACKOVF_DETECT))
 
 static void
 process_sigsegv (int signo, const char *p)
@@ -200,20 +205,31 @@
   signal (signo, SIG_DFL);
 }
 
-#if HAVE_SIGINFO_H
+#if HAVE_STRUCT_SIGACTION_SA_SIGACTION
+
+/* POSIX.  */
+
+static void
+sigsegv_handler (int signo, siginfo_t *ip, void *context)
+{
+  process_sigsegv
+    (signo, (ip != NULL
+            && ip->si_signo == SIGSEGV ? (char *) ip->si_addr : NULL));
+}
+
+#elif HAVE_SIGINFO_T
 
 /* SVR4.  */
 
 static void
-sigsegv_handler (int signo, siginfo_t * ip)
+sigsegv_handler (int signo, siginfo_t *ip)
 {
   process_sigsegv
-    (signo, (ip != (siginfo_t *) 0
+    (signo, (ip != NULL
             && ip->si_signo == SIGSEGV ? (char *) ip->si_addr : NULL));
 }
 
-#else /* not HAVE_SIGINFO_H */
-#if HAVE_SIGCONTEXT
+#elif HAVE_SIGCONTEXT
 
 /* SunOS 4.x (and BSD?).  (not tested) */
 
@@ -234,7 +250,6 @@
 }
 
 #endif /* not HAVE_SIGCONTEXT */
-#endif /* not HAVE_SIGINFO */
 
 /* Arrange to trap a stack-overflow and call a specified handler.  The
    call is on a dedicated signal stack.
@@ -259,20 +274,23 @@
   int grows_upward;
   register char *const *v;
   register char *p;
-#if HAVE_SIGACTION && defined(SA_ONSTACK)
+#if HAVE_SIGACTION && defined SA_ONSTACK
   struct sigaction act;
-#else
+#elif HAVE_SIGVEC && defined SV_ONSTACK
   struct sigvec vec;
+#else
+
+Error - Do not know how to set up stack-ovf trap handler...
+
 #endif
 
-  grows_upward = ((char *) argv < (char *) &stack_len);
   arg0 = argv[0];
   stackovf_handler = handler;
 
   /* Calculate the approximate expected addr for a stack-ovf trap.  */
 
   if (getrlimit (RLIMIT_STACK, &rl) < 0)
-    error (1, errno, "getrlimit");
+    error (EXIT_FAILURE, errno, "getrlimit");
   stack_len = (rl.rlim_cur < rl.rlim_max ? rl.rlim_cur : rl.rlim_max);
   stackbot = (char *) argv;
   grows_upward = ((char *) &stack_len > stackbot);
@@ -281,14 +299,14 @@
 
       /* Grows toward increasing addresses.  */
 
-      for (v = argv; (p = (char *) *v) != (char *) 0; v++)
+      for (v = argv; (p = (char *) *v) != NULL; v++)
        {
          if (p < stackbot)
            stackbot = p;
        }
       if ((char *) envp < stackbot)
        stackbot = (char *) envp;
-      for (v = envp; (p = (char *) *v) != (char *) 0; v++)
+      for (v = envp; (p = (char *) *v) != NULL; v++)
        {
          if (p < stackbot)
            stackbot = p;
@@ -300,14 +318,14 @@
 
       /* The stack grows "downward" (toward decreasing addresses).  */
 
-      for (v = argv; (p = (char *) *v) != (char *) 0; v++)
+      for (v = argv; (p = (char *) *v) != NULL; v++)
        {
          if (p > stackbot)
            stackbot = p;
        }
       if ((char *) envp > stackbot)
        stackbot = (char *) envp;
-      for (v = envp; (p = (char *) *v) != (char *) 0; v++)
+      for (v = envp; (p = (char *) *v) != NULL; v++)
        {
          if (p > stackbot)
            stackbot = p;
@@ -317,9 +335,9 @@
 
   /* Allocate a separate signal-handler stack.  */
 
-#if HAVE_SIGALTSTACK && (defined(HAVE_SIGINFO_H) || !HAVE_SIGSTACK)
+#if HAVE_SIGALTSTACK && (HAVE_SIGINFO_T || ! HAVE_SIGSTACK)
 
-  /* Use sigaltstack only if siginfo is available, unless there is no
+  /* Use sigaltstack only if siginfo_t is available, unless there is no
      choice.  */
 
   {
@@ -328,12 +346,11 @@
     ss.ss_size = SIGSTKSZ;
     ss.ss_sp = xmalloc ((unsigned) ss.ss_size);
     ss.ss_flags = 0;
-    if (sigaltstack (&ss, (stack_t *) 0) < 0)
-      error (1, errno, "sigaltstack");
+    if (sigaltstack (&ss, NULL) < 0)
+      error (EXIT_FAILURE, errno, "sigaltstack");
   }
 
-#else /* not HAVE_SIGALTSTACK || not HAVE_SIGINFO_H && HAVE_SIGSTACK */
-#if HAVE_SIGSTACK
+#elif HAVE_SIGSTACK
 
   {
     struct sigstack ss;
@@ -342,7 +359,7 @@
     ss.ss_sp = stackbuf + SIGSTKSZ;
     ss.ss_onstack = 0;
     if (sigstack (&ss, NULL) < 0)
-      error (1, errno, "sigstack");
+      error (EXIT_FAILURE, errno, "sigstack");
   }
 
 #else /* not HAVE_SIGSTACK */
@@ -350,44 +367,30 @@
 Error - Do not know how to set up stack-ovf trap handler...
 
 #endif /* not HAVE_SIGSTACK */
-#endif /* not HAVE_SIGALTSTACK || not HAVE_SIGINFO_H && HAVE_SIGSTACK */
 
   /* Arm the SIGSEGV signal handler.  */
 
-#if HAVE_SIGACTION && defined(SA_ONSTACK)
+#if HAVE_SIGACTION && defined SA_ONSTACK
 
   sigaction (SIGSEGV, NULL, &act);
+# if HAVE_STRUCT_SIGACTION_SA_SIGACTION
+  act.sa_sigaction = sigsegv_handler;
+# else /* ! HAVE_STRUCT_SIGACTION_SA_SIGACTION */
   act.sa_handler = (RETSIGTYPE (*) _((int))) sigsegv_handler;
+# endif /* ! HAVE_STRUCT_SIGACTION_SA_SIGACTION */
   sigemptyset (&act.sa_mask);
-  act.sa_flags = (SA_ONSTACK
-#ifdef SA_RESETHAND
-                 | SA_RESETHAND
-#endif
-#ifdef SA_SIGINFO
-                 | SA_SIGINFO
-#endif
-                 );
+  act.sa_flags = (SA_ONSTACK | SA_RESETHAND | SA_SIGINFO);
   if (sigaction (SIGSEGV, &act, NULL) < 0)
-    error (1, errno, "sigaction");
+    error (EXIT_FAILURE, errno, "sigaction");
 
-#else /* not HAVE_SIGACTION */
-#if HAVE_SIGVEC && defined(SV_ONSTACK)
+#else /* ! HAVE_SIGACTION */
 
   vec.sv_handler = (RETSIGTYPE (*)_ ((int))) sigsegv_handler;
   vec.sv_mask = 0;
-  vec.sv_flags = (SV_ONSTACK
-#ifdef SV_RESETHAND
-                 | SV_RESETHAND
-#endif
-                );
+  vec.sv_flags = (SV_ONSTACK | SV_RESETHAND);
   if (sigvec (SIGSEGV, &vec, NULL) < 0)
-    error (1, errno, "sigvec");
-
-#else /* not HAVE_SIGVEC && defined(SV_ONSTACK) */
-
-Error - Do not know how to catch signals on an alternate stack...
+    error (EXIT_FAILURE, errno, "sigvec");
 
-#endif /* HAVE_SIGVEC && defined(SV_ONSTACK) */
-#endif /* HAVE_SIGALTSTACK && defined(SA_ONSTACK) */
+#endif /* ! HAVE_SIGACTION */
 
 }