Skip to content

Commit

Permalink
Partial workaround for the Cygwin-specific issue described at
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
embray committed Feb 28, 2019
1 parent cfbd43b commit 36e767f
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 1 deletion.
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions src/cysignals/implementation_cygwin.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions src/cysignals/tests.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -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():
"""
Expand Down
36 changes: 36 additions & 0 deletions src/cysignals/tests_helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 36e767f

Please sign in to comment.