[Bug-libsigsegv] Re: [PATCH] More robust detection of powerpc64-unknown-linux-gnu failure.

[Bruno Haible]

Hi Eric,

> What do you think of this patch?  On the powerpc64 machine I was testing
> on, where CFLAGS=-O2 somehow makes the stack usage go a lot higher than
> CFLAGS=-O1, this changes the failure from a hard-to-diagnose SIGILL:
> 
> Starting recursion pass 1.
> /bin/sh: line 5: 15944 Illegal instruction     (core dumped) ${dir}$tst
> FAIL: stackoverflow1
> 
> to a much nicer:
> 
> Starting recursion pass 1.
> Stack overflow 1 caught.
> Starting recursion pass 2.
> Stack overflow 2 caught.
> alternate stack was exceeded!
> FAIL: stackoverflow1

Very nice! And it diagnoses the problem also on x86_64, if I decrease
SIGSTKSZ below 4 KB.

I committed your patch, with a couple of modifications:
  - Apply the changes also to 3 other tests,
  - Avoid code duplication by moving the code to a separate file,
    altstack.h.
  - Do the proper thing to get 'uintptr_t'. AC_TYPE_UINTPTR_T doesn't
    arrange for <stdint.h> to exist.
  - Change the verification step so that it also prints by how many bytes
    the alternate stack area was exceeded.


2010-10-24  Eric Blake  <address@hidden>
            Bruno Haible  <address@hidden>

        Enhance the tests to detect overflow of the alternate stack.
        * tests/altstack.h: New file.
        * tests/Makefile.am (EXTRA_DIST): Add altstack.h.
        * configure.ac: Invoke AC_TYPE_UINTPTR_T.
        * tests/stackoverflow1.c: Include altstack.h.
        (SIGSTKSZ): Remove definition.
        (mystack): Remove definition.
        (main): Invoke prepare_alternate_stack and
        check_alternate_stack_no_overflow.
        * tests/stackoverflow2.c: Likewise.
        * tests/efault2.c: Likewise.
        * tests/efault3.c: Likewise.

--- configure.ac.orig   Sun Oct 24 20:59:00 2010
+++ configure.ac        Sun Oct 24 18:10:57 2010
@@ -939,7 +939,6 @@
 
 dnl ======================= Done with CFG_HANDLER fully =======================
 
-
 dnl Test whether to enable Cygwin specific tests.
 AM_CONDITIONAL([CYGWIN],
   [case "$host_os" in
@@ -967,6 +966,10 @@
 fi
 
 
+dnl Test for features used in tests.
+AC_TYPE_UINTPTR_T
+
+
 { echo; echo "${term_bold}Build Parameters:${term_norm}"; } >& AS_MESSAGE_FD
 
 dnl Relocatability is a nop for this package.
--- tests/Makefile.am.orig      Sun Oct 24 20:59:00 2010
+++ tests/Makefile.am   Sun Oct 24 19:25:15 2010
@@ -1,5 +1,5 @@
 ## Makefile for libsigsegv/tests.
-## Copyright (C) 2002-2003, 2008-2009 Bruno Haible <address@hidden>
+## Copyright (C) 2002-2003, 2008-2010 Bruno Haible <address@hidden>
 ##
 ## This program is free software; you can redistribute it and/or modify
 ## it under the terms of the GNU General Public License as published by
@@ -22,7 +22,7 @@
 
 TESTS = sigsegv1 sigsegv2 sigsegv3 stackoverflow1 stackoverflow2
 
-EXTRA_DIST = mmaputil.h
+EXTRA_DIST = mmaputil.h altstack.h
 
 INCLUDES = -I../src
 DEFS = @DEFS@
Changing permissions from . to 100644
--- tests/altstack.h.orig       Tue Apr 14 12:31:40 2009
+++ tests/altstack.h    Sun Oct 24 19:42:55 2010
@@ -0,0 +1,67 @@
+/* Some auxiliary stuff for defining an alternate stack.
+   Copyright (C) 2010  Eric Blake <address@hidden>
+   Copyright (C) 2010  Bruno Haible <address@hidden>
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software Foundation,
+   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
+
+#include <string.h> /* for memset */
+
+/* Get uintptr_t.  */
+#if HAVE_STDINT_H
+# include <stdint.h>
+#endif
+#if HAVE_INTTYPES_H
+# include <inttypes.h>
+#endif
+
+#ifndef SIGSTKSZ
+# define SIGSTKSZ 16384
+#endif
+
+/* glibc says: Users should use SIGSTKSZ as the size of user-supplied
+   buffers.  We want to detect stack overflow of the alternate stack
+   in a nicer manner than just crashing, so we overallocate in
+   comparison to what we hand libsigsegv.  Also, we intentionally hand
+   an unaligned pointer, to ensure the alternate stack still ends up
+   aligned.  */
+#define MYSTACK_CRUMPLE_ZONE 8192
+char mystack_storage[SIGSTKSZ + 2 * MYSTACK_CRUMPLE_ZONE + 31];
+char *mystack; /* SIGSTKSZ bytes in the middle of storage. */
+
+static void
+prepare_alternate_stack (void)
+{
+  memset (mystack_storage, 's', sizeof mystack_storage);
+  mystack = (char *) ((uintptr_t) (mystack_storage + MYSTACK_CRUMPLE_ZONE) | 
31);
+}
+
+static void
+check_alternate_stack_no_overflow (void)
+{
+  unsigned int i;
+
+  for (i = MYSTACK_CRUMPLE_ZONE; i > 0; i--)
+    if (*(mystack - i) != 's')
+      {
+        printf ("Alternate stack was exceeded by %u bytes!!\n", i);
+        exit (1);
+      }
+  for (i = MYSTACK_CRUMPLE_ZONE; i > 0; i--)
+    if (*(mystack + SIGSTKSZ - 1 + i) != 's')
+      {
+        printf ("Alternate stack was exceeded by %u bytes!!\n", i);
+        exit (1);
+      }
+}
--- tests/efault2.c.orig        Sun Oct 24 20:59:00 2010
+++ tests/efault2.c     Sun Oct 24 19:15:40 2010
@@ -1,6 +1,6 @@
 /* Test that libsigsegv does not interfere with fault handling inside
    system calls.
-   Copyright (C) 2009  Eric Blake <address@hidden>
+   Copyright (C) 2009-2010  Eric Blake <address@hidden>
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -30,9 +30,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 
-#ifndef SIGSTKSZ
-# define SIGSTKSZ 16384
-#endif
+#include "altstack.h"
 
 /* A NULL pointer.
    If we were to use a literal NULL, gcc would give a warning on glibc systems:
@@ -47,10 +45,6 @@
   abort ();
 }
 
-/* glibc says: Users should use SIGSTKSZ as the size of user-supplied
-   buffers.  */
-char mystack[SIGSTKSZ];
-
 int
 main ()
 {
@@ -61,9 +55,11 @@
       exit (1);
     }
 
+  /* Prepare the storage for the alternate stack.  */
+  prepare_alternate_stack ();
+
   /* Install the stack overflow handler.  */
-  if (stackoverflow_install_handler (&handler, mystack, sizeof (mystack))
-      < 0)
+  if (stackoverflow_install_handler (&handler, mystack, SIGSTKSZ) < 0)
     exit (2);
 
   /* Test that the library does not interfere with OS faults.  */
@@ -73,6 +69,9 @@
       exit (1);
     }
 
+  /* Validate that the alternate stack did not overflow.  */
+  check_alternate_stack_no_overflow ();
+
   /* Test passed!  */
   printf ("Test passed.\n");
   return 0;
--- tests/efault3.c.orig        Sun Oct 24 20:59:00 2010
+++ tests/efault3.c     Sun Oct 24 19:15:41 2010
@@ -1,6 +1,6 @@
 /* Test that libsigsegv does not interfere with fault handling inside
    system calls.
-   Copyright (C) 2009  Eric Blake <address@hidden>
+   Copyright (C) 2009-2010  Eric Blake <address@hidden>
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -30,9 +30,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 
-#ifndef SIGSTKSZ
-# define SIGSTKSZ 16384
-#endif
+#include "altstack.h"
 
 /* A NULL pointer.
    If we were to use a literal NULL, gcc would give a warning on glibc systems:
@@ -55,10 +53,6 @@
   abort ();
 }
 
-/* glibc says: Users should use SIGSTKSZ as the size of user-supplied
-   buffers.  */
-char mystack[SIGSTKSZ];
-
 int
 main ()
 {
@@ -69,9 +63,12 @@
       exit (1);
     }
 
+  /* Prepare the storage for the alternate stack.  */
+  prepare_alternate_stack ();
+
   /* Install the stack overflow handler.  */
   if (stackoverflow_install_handler (&stackoverflow_handler,
-                                     mystack, sizeof (mystack))
+                                     mystack, SIGSTKSZ)
       < 0)
     exit (2);
 
@@ -86,6 +83,9 @@
       exit (1);
     }
 
+  /* Validate that the alternate stack did not overflow.  */
+  check_alternate_stack_no_overflow ();
+
   /* Test passed!  */
   printf ("Test passed.\n");
   return 0;
--- tests/stackoverflow1.c.orig Sun Oct 24 20:59:00 2010
+++ tests/stackoverflow1.c      Sun Oct 24 19:15:40 2010
@@ -1,5 +1,6 @@
 /* Test the stack overflow handler.
-   Copyright (C) 2002-2006, 2008  Bruno Haible <address@hidden>
+   Copyright (C) 2002-2006, 2008, 2010  Bruno Haible <address@hidden>
+   Copyright (C) 2010 Eric Blake <address@hidden>
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -41,9 +42,7 @@
 # include <sys/time.h>
 # include <sys/resource.h>
 #endif
-#ifndef SIGSTKSZ
-# define SIGSTKSZ 16384
-#endif
+#include "altstack.h"
 
 jmp_buf mainloop;
 sigset_t mainsigset;
@@ -88,10 +87,6 @@
   return *recurse_1 (n, &n);
 }
 
-/* glibc says: Users should use SIGSTKSZ as the size of user-supplied
-   buffers.  */
-char mystack[SIGSTKSZ];
-
 int
 main ()
 {
@@ -106,13 +101,16 @@
   setrlimit (RLIMIT_STACK, &rl);
 #endif
 
+  /* Prepare the storage for the alternate stack.  */
+  prepare_alternate_stack ();
+
   /* Install the stack overflow handler.  */
   if (stackoverflow_install_handler (&stackoverflow_handler,
-                                     mystack, sizeof (mystack))
+                                     mystack, SIGSTKSZ)
       < 0)
     exit (2);
   stack_lower_bound = mystack;
-  stack_upper_bound = mystack + sizeof (mystack) - 1;
+  stack_upper_bound = mystack + SIGSTKSZ - 1;
 
   /* Save the current signal mask.  */
   sigemptyset (&emptyset);
@@ -133,6 +131,9 @@
       abort ();
     }
 
+  /* Validate that the alternate stack did not overflow.  */
+  check_alternate_stack_no_overflow ();
+
   printf ("Test passed.\n");
   exit (0);
 }
--- tests/stackoverflow2.c.orig Sun Oct 24 20:59:00 2010
+++ tests/stackoverflow2.c      Sun Oct 24 19:15:40 2010
@@ -1,5 +1,6 @@
 /* Test that stack overflow and SIGSEGV are correctly distinguished.
    Copyright (C) 2002-2006, 2008  Bruno Haible <address@hidden>
+   Copyright (C) 2010 Eric Blake <address@hidden>
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -42,9 +43,7 @@
 # include <sys/time.h>
 # include <sys/resource.h>
 #endif
-#ifndef SIGSTKSZ
-# define SIGSTKSZ 16384
-#endif
+#include "altstack.h"
 
 jmp_buf mainloop;
 sigset_t mainsigset;
@@ -109,10 +108,6 @@
   return *recurse_1 (n, &n);
 }
 
-/* glibc says: Users should use SIGSTKSZ as the size of user-supplied
-   buffers.  */
-char mystack[SIGSTKSZ];
-
 int
 main ()
 {
@@ -128,9 +123,12 @@
   setrlimit (RLIMIT_STACK, &rl);
 #endif
 
+  /* Prepare the storage for the alternate stack.  */
+  prepare_alternate_stack ();
+
   /* Install the stack overflow handler.  */
   if (stackoverflow_install_handler (&stackoverflow_handler,
-                                     mystack, sizeof (mystack))
+                                     mystack, SIGSTKSZ)
       < 0)
     exit (2);
 
@@ -184,6 +182,9 @@
       abort ();
     }
 
+  /* Validate that the alternate stack did not overflow.  */
+  check_alternate_stack_no_overflow ();
+
   printf ("Test passed.\n");
   exit (0);
 }