From 2fbb09a3395da74d3965637ad183b7b4c2c23bf0 Mon Sep 17 00:00:00 2001
From: "Erik M. Bray" <erik.bray@lri.fr>
Date: Wed, 27 Feb 2019 15:29:14 +0100
Subject: [PATCH] Partial workaround for the Cygwin-specific issue described at
 https://trac.sagemath.org/ticket/27214#comment:11

Includes regression test.

As noted in the comment, this can still invoke undesired behavior in the
case where an unintialized region of an mmap created with MAP_NORESERVE
is accessed from within a signal handler.

At the time there is no good way to handle that case without extra
support from Cygwin's API which is currently not available as far as I
can tell.  A workaround, for the rare code where this might be
necessary, is to call mprotect (with the appropriate flags) on the
region of memory that needs to be accessed before attempting to access
it.
---
 configure.ac                          |  2 +-
 src/cysignals/implementation_cygwin.c | 14 +++++++++++
 src/cysignals/tests.pyx               | 25 +++++++++++++++++++
 src/cysignals/tests_helper.c          | 36 +++++++++++++++++++++++++++
 4 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 604a6ff5..7c4b3b1e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -21,7 +21,7 @@ fi
 
 AC_LANG(C)
 
-AC_CHECK_HEADERS([execinfo.h sys/prctl.h sys/time.h sys/wait.h windows.h])
+AC_CHECK_HEADERS([execinfo.h sys/mman.h sys/prctl.h sys/time.h sys/wait.h windows.h])
 AC_CHECK_FUNCS([fork kill sigprocmask sigaltstack backtrace])
 
 have_pari=no
diff --git a/src/cysignals/implementation_cygwin.c b/src/cysignals/implementation_cygwin.c
index 9597a884..3b181143 100644
--- a/src/cysignals/implementation_cygwin.c
+++ b/src/cysignals/implementation_cygwin.c
@@ -5,6 +5,20 @@
 LONG WINAPI win32_altstack_handler(EXCEPTION_POINTERS *exc)
 {
     int sig = 0;
+    /* If we're not handling a signal there is no reason to execute the
+     * following code; otherwise it can be run in inappropriate contexts
+     * such as when a STATUS_ACCESS_VIOLATION is raised when accessing
+     * uncommitted memory in an mmap created with MAP_NORESERVE. See
+     * discussion at https://trac.sagemath.org/ticket/27214#comment:11
+     *
+     * Unfortunately, when handling an exception that occurred while
+     * handling another signal, there is currently no way (through Cygwin)
+     * to distinguish this case from a legitimate segfault.
+     */
+    if (!cysigs.inside_signal_handler) {
+        return ExceptionContinueExecution;
+    }
+
     /* Logic cribbed from Cygwin for mapping common Windows exception
      * codes to the relevant signal numbers:
      * https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/exceptions.cc;h=77eff05707f95f7277974fadbccf0e74223d8d1c;hb=HEAD#l650
diff --git a/src/cysignals/tests.pyx b/src/cysignals/tests.pyx
index dbe14d58..e3bf993c 100644
--- a/src/cysignals/tests.pyx
+++ b/src/cysignals/tests.pyx
@@ -42,6 +42,7 @@ from __future__ import absolute_import
 from libc.signal cimport (SIGHUP, SIGINT, SIGABRT, SIGILL, SIGSEGV,
         SIGFPE, SIGBUS, SIGQUIT)
 from libc.stdlib cimport abort
+from libc.errno cimport errno
 from posix.signal cimport sigaltstack, stack_t, SS_ONSTACK
 
 from cpython cimport PyErr_SetString
@@ -54,6 +55,8 @@ cdef extern from "tests_helper.c" nogil:
     void ms_sleep(long ms)
     void signal_after_delay(int signum, long ms)
     void signals_after_delay(int signum, long ms, long interval, int n)
+    void* map_noreserve()
+    int unmap_noreserve(void* addr)
 
 cdef extern from *:
     ctypedef int volatile_int "volatile int"
@@ -616,6 +619,28 @@ def unguarded_dereference_null_pointer():
     with nogil:
         dereference_null_pointer()
 
+def test_access_mmap_noreserve():
+    """
+    TESTS:
+
+    Regression test for https://github.com/sagemath/cysignals/pull/108; if
+    the issue is fixed then ``test_access_mmap_noreserve()`` should have no
+    output.  Otherwise the subprocess will exit and report an error occurred
+    during signal handling::
+
+        >>> from cysignals.tests import test_access_mmap_noreserve
+        >>> subpython_err('from cysignals.tests import *; test_access_mmap_noreserve()')
+
+    """
+
+    cdef int* ptr = <int*>map_noreserve()
+    if ptr == NULL:
+        raise RuntimeError(f"map_noreserve() failed; errno: {errno}")
+
+    ptr[0] += 1  # Should just work
+
+    if unmap_noreserve(ptr) != 0:
+        raise RuntimeError(f"uname_noreserve() failed; errno: {errno}")
 
 def test_abort():
     """
diff --git a/src/cysignals/tests_helper.c b/src/cysignals/tests_helper.c
index 156479a5..68801108 100644
--- a/src/cysignals/tests_helper.c
+++ b/src/cysignals/tests_helper.c
@@ -28,6 +28,9 @@
 #if HAVE_UNISTD_H
 #include <unistd.h>
 #endif
+#if HAVE_SYS_MMAN_H
+#include <sys/mman.h>
+#endif
 #if HAVE_SYS_TYPES_H
 #include <sys/types.h>
 #endif
@@ -65,6 +68,39 @@ static void ms_sleep(long ms)
 }
 
 
+/* Calls mmap if available with the MAP_NORESERVE flag; if neither is
+ * available just mmap without MAP_NORESERVE or malloc--this is used currently
+ * just to test a regression on Cygwin (see test_read_mmap_noreserve),
+ * so if the required functionality is not available then the test should pass
+ * trivially.
+ */
+#define MAP_NORESERVE_LEN 4096
+#if HAVE_SYS_MMAN_H
+#ifndef MAP_NORESERVE
+#define MAP_NORESERVE 0
+#endif
+static void* map_noreserve(void)
+{
+    return mmap(NULL, MAP_NORESERVE_LEN, PROT_READ|PROT_WRITE,
+                MAP_ANONYMOUS|MAP_PRIVATE|MAP_NORESERVE, -1, 0);
+}
+
+static int unmap_noreserve(void* addr) {
+    return munmap(addr, MAP_NORESERVE_LEN);
+}
+#else
+static void* map_noreserve(void)
+{
+    return malloc(MAP_NORESERVE_LEN);
+}
+
+static int unmap_noreserve(void* addr) {
+    free(addr);
+    return 0;
+}
+#endif
+
+
 /* Signal the running process with signal ``signum`` after ``ms``
  * milliseconds.  Wait ``interval`` milliseconds, then signal again.
  * Repeat this until ``n`` signals have been sent.