From fc29c08d93b2235f4f7a01767a978090eaa1dccc Mon Sep 17 00:00:00 2001 From: Marius Gerbershagen Date: Fri, 16 Feb 2018 19:07:27 +0100 Subject: [PATCH] threading: use safer method to disable interrupts when resizing stacks Due to the use of mprotect() for fast interrupt dispatch it is not possible to write in the thread local environment when interrupts are disabled. We need to use sigprocmask to block interrupts in this case. --- src/c/interpreter.d | 5 +++-- src/c/stacks.d | 8 ++++---- src/h/internal.h | 27 +++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/c/interpreter.d b/src/c/interpreter.d index 23cada023..dbfdfe906 100644 --- a/src/c/interpreter.d +++ b/src/c/interpreter.d @@ -38,14 +38,13 @@ ecl_stack_set_size(cl_env_ptr env, cl_index tentative_new_size) old_stack = env->stack; new_stack = (cl_object *)ecl_alloc_atomic(new_size * sizeof(cl_object)); - ecl_disable_interrupts_env(env); + ECL_STACK_RESIZE_DISABLE_INTERRUPTS(env); memcpy(new_stack, old_stack, env->stack_size * sizeof(cl_object)); env->stack_size = new_size; env->stack_limit_size = new_size - 2*safety_area; env->stack = new_stack; env->stack_top = env->stack + top; env->stack_limit = env->stack + (new_size - 2*safety_area); - ecl_enable_interrupts_env(env); /* A stack always has at least one element. This is assumed by cl__va_start * and friends, which take a sp=0 to have no arguments. @@ -53,6 +52,8 @@ ecl_stack_set_size(cl_env_ptr env, cl_index tentative_new_size) if (top == 0) { *(env->stack_top++) = ecl_make_fixnum(0); } + ECL_STACK_RESIZE_ENABLE_INTERRUPTS(env); + return env->stack_top; } diff --git a/src/c/stacks.d b/src/c/stacks.d index 94ddc54ce..1dd9b0078 100644 --- a/src/c/stacks.d +++ b/src/c/stacks.d @@ -139,13 +139,13 @@ ecl_bds_set_size(cl_env_ptr env, cl_index new_size) env->bds_limit_size = new_size - 2*margin; org = ecl_alloc_atomic(new_size * sizeof(*org)); - ecl_disable_interrupts_env(env); + ECL_STACK_RESIZE_DISABLE_INTERRUPTS(env); memcpy(org, old_org, (limit + 1) * sizeof(*org)); env->bds_top = org + limit; env->bds_org = org; env->bds_limit = org + (new_size - 2*margin); env->bds_size = new_size; - ecl_enable_interrupts_env(env); + ECL_STACK_RESIZE_ENABLE_INTERRUPTS(env); ecl_dealloc(old_org); } @@ -505,13 +505,13 @@ frs_set_size(cl_env_ptr env, cl_index new_size) env->frs_limit_size = new_size - 2*margin; org = ecl_alloc_atomic(new_size * sizeof(*org)); - ecl_disable_interrupts_env(env); + ECL_STACK_RESIZE_DISABLE_INTERRUPTS(the_env); memcpy(org, old_org, (limit + 1) * sizeof(*org)); env->frs_top = org + limit; env->frs_org = org; env->frs_limit = org + (new_size - 2*margin); env->frs_size = new_size; - ecl_enable_interrupts_env(env); + ECL_STACK_RESIZE_ENABLE_INTERRUPTS(the_env); ecl_dealloc(old_org); } diff --git a/src/h/internal.h b/src/h/internal.h index 55b5f5d96..5e21e6d46 100755 --- a/src/h/internal.h +++ b/src/h/internal.h @@ -642,6 +642,33 @@ static union { # endif /* _MSC_VER == 1600 */ #endif /* ~NAN */ +/* + * safe stack resizing + */ + +/* We can't block interrupts with ecl_disable_interrupts() and write + * in the thread local environment if we use fast interrupt dispatch + * via mprotect(), so we have to use sigprocmask instead. No + * performance problems, since this is only used for stack + * resizing. */ +#ifdef ECL_USE_MPROTECT +#ifdef HAVE_SIGPROCMASK +#include +#define ECL_STACK_RESIZE_DISABLE_INTERRUPTS(the_env) \ + sigset_t __sigset_new, __sigset_previous; \ + sigfillset(&__sigset_new); \ + pthread_sigmask(SIG_BLOCK, &__sigset_new, &__sigset_previous) +#define ECL_STACK_RESIZE_ENABLE_INTERRUPTS(the_env) \ + pthread_sigmask(SIG_SETMASK, &__sigset_previous, NULL) +#else +#error "Can't protect stack resizing from interrupts without sigprocmask. Either build ECL without mprotect() or live with possible race conditions." +#endif /* HAVE_SIGPROCMASK */ +#else +#define ECL_STACK_RESIZE_DISABLE_INTERRUPTS(the_env) ecl_disable_interrupts_env(the_env); +#define ECL_STACK_RESIZE_ENABLE_INTERRUPTS(the_env) ecl_enable_interrupts_env(env); +#endif /* ECL_USE_MPROTECT */ + + #ifdef __cplusplus } #endif