From ab0053628bff2bf1ae44c66ed87de4a557cb28a6 Mon Sep 17 00:00:00 2001 From: Jonathan Chambers Date: Thu, 20 Dec 2018 16:51:22 -0500 Subject: [PATCH] Fix reentrant exception handling. Store handled status in local that is passed into handler rather than a single thread local value which can be overwritten in case of reentrance. --- mono/mini/exceptions-amd64.c | 10 +++++----- mono/mini/exceptions-x86.c | 8 ++++---- mono/mini/mini-amd64.h | 3 ++- mono/mini/mini-runtime.h | 5 ----- mono/mini/mini-windows.c | 4 ++-- mono/mini/mini-x86.h | 2 +- mono/utils/mono-signal-handler.h | 16 ++++++++++++---- 7 files changed, 26 insertions(+), 22 deletions(-) diff --git a/mono/mini/exceptions-amd64.c b/mono/mini/exceptions-amd64.c index a20f3ae335e..938e8462281 100644 --- a/mono/mini/exceptions-amd64.c +++ b/mono/mini/exceptions-amd64.c @@ -58,7 +58,7 @@ LPTOP_LEVEL_EXCEPTION_FILTER mono_old_win_toplevel_exception_filter; void *mono_win_vectored_exception_handle; #define W32_SEH_HANDLE_EX(_ex) \ - if (_ex##_handler) _ex##_handler(0, ep, ctx) + if (_ex##_handler) _ex##_handler(er->ExceptionCode, &info, ctx) static LONG CALLBACK seh_unhandled_exception_filter(EXCEPTION_POINTERS* ep) { @@ -137,12 +137,12 @@ LONG CALLBACK seh_vectored_exception_handler(EXCEPTION_POINTERS* ep) LONG res; MonoJitTlsData *jit_tls = mono_tls_get_jit_tls (); MonoDomain* domain = mono_domain_get (); + MonoWindowsSigHandlerInfo info = { TRUE, ep }; /* If the thread is not managed by the runtime return early */ if (!jit_tls) return EXCEPTION_CONTINUE_SEARCH; - jit_tls->mono_win_chained_exception_needs_run = FALSE; res = EXCEPTION_CONTINUE_EXECUTION; er = ep->ExceptionRecord; @@ -159,7 +159,7 @@ LONG CALLBACK seh_vectored_exception_handler(EXCEPTION_POINTERS* ep) ctx->Rip = (guint64)restore_stack; } } else { - jit_tls->mono_win_chained_exception_needs_run = TRUE; + info.handled = FALSE; } break; case EXCEPTION_ACCESS_VIOLATION: @@ -177,11 +177,11 @@ LONG CALLBACK seh_vectored_exception_handler(EXCEPTION_POINTERS* ep) W32_SEH_HANDLE_EX(fpe); break; default: - jit_tls->mono_win_chained_exception_needs_run = TRUE; + info.handled = FALSE; break; } - if (jit_tls->mono_win_chained_exception_needs_run) { + if (!info.handled) { /* Don't copy context back if we chained exception * as the handler may have modfied the EXCEPTION_POINTERS * directly. We don't pass sigcontext to chained handlers. diff --git a/mono/mini/exceptions-x86.c b/mono/mini/exceptions-x86.c index 89ef7e57768..ca511434b37 100644 --- a/mono/mini/exceptions-x86.c +++ b/mono/mini/exceptions-x86.c @@ -55,7 +55,7 @@ extern int (*gUnhandledExceptionHandler)(EXCEPTION_POINTERS*); #endif #define W32_SEH_HANDLE_EX(_ex) \ - if (_ex##_handler) _ex##_handler(0, ep, ctx) + if (_ex##_handler) _ex##_handler(er->ExceptionCode, &info, ctx) LONG CALLBACK seh_unhandled_exception_filter(EXCEPTION_POINTERS* ep) { @@ -198,12 +198,12 @@ LONG CALLBACK seh_vectored_exception_handler(EXCEPTION_POINTERS* ep) CONTEXT* ctx; LONG res; MonoJitTlsData *jit_tls = mono_tls_get_jit_tls (); + MonoWindowsSigHandlerInfo info = { TRUE, ep }; /* If the thread is not managed by the runtime return early */ if (!jit_tls) return EXCEPTION_CONTINUE_SEARCH; - jit_tls->mono_win_chained_exception_needs_run = FALSE; res = EXCEPTION_CONTINUE_EXECUTION; er = ep->ExceptionRecord; @@ -228,11 +228,11 @@ LONG CALLBACK seh_vectored_exception_handler(EXCEPTION_POINTERS* ep) W32_SEH_HANDLE_EX(fpe); break; default: - jit_tls->mono_win_chained_exception_needs_run = TRUE; + info.handled = FALSE; break; } - if (jit_tls->mono_win_chained_exception_needs_run) { + if (!info.handled) { /* Don't copy context back if we chained exception * as the handler may have modfied the EXCEPTION_POINTERS * directly. We don't pass sigcontext to chained handlers. diff --git a/mono/mini/mini-amd64.h b/mono/mini/mini-amd64.h index 7fa4e184134..30291512d4f 100644 --- a/mono/mini/mini-amd64.h +++ b/mono/mini/mini-amd64.h @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -32,7 +33,7 @@ struct sigcontext { }; #endif -typedef void (* MonoW32ExceptionHandler) (int _dummy, EXCEPTION_POINTERS *info, void *context); +typedef void MONO_SIG_HANDLER_SIGNATURE ((*MonoW32ExceptionHandler)); void win32_seh_init(void); void win32_seh_cleanup(void); void win32_seh_set_handler(int type, MonoW32ExceptionHandler handler); diff --git a/mono/mini/mini-runtime.h b/mono/mini/mini-runtime.h index d9475ab61a2..8aab82d8811 100644 --- a/mono/mini/mini-runtime.h +++ b/mono/mini/mini-runtime.h @@ -101,11 +101,6 @@ struct MonoJitTlsData { MonoContext orig_ex_ctx; gboolean orig_ex_ctx_set; - /* - * Stores if we need to run a chained exception in Windows. - */ - gboolean mono_win_chained_exception_needs_run; - /* * The current exception in flight */ diff --git a/mono/mini/mini-windows.c b/mono/mini/mini-windows.c index a10343f44e2..25ed139e26f 100644 --- a/mono/mini/mini-windows.c +++ b/mono/mini/mini-windows.c @@ -255,8 +255,8 @@ mono_runtime_cleanup_handlers (void) gboolean MONO_SIG_HANDLER_SIGNATURE (mono_chain_signal) { - MonoJitTlsData *jit_tls = mono_tls_get_jit_tls (); - jit_tls->mono_win_chained_exception_needs_run = TRUE; + /* Set to FALSE to indicate that vectored exception handling should continue to look for handler */ + MONO_SIG_HANDLER_GET_INFO ()->handled = FALSE; return TRUE; } diff --git a/mono/mini/mini-x86.h b/mono/mini/mini-x86.h index 3cf44a06b46..59d74012d8b 100644 --- a/mono/mini/mini-x86.h +++ b/mono/mini/mini-x86.h @@ -16,7 +16,7 @@ #include #endif -typedef void (* MonoW32ExceptionHandler) (int _dummy, EXCEPTION_POINTERS *info, void *context); +typedef void MONO_SIG_HANDLER_SIGNATURE ((*MonoW32ExceptionHandler)); void win32_seh_init(void); void win32_seh_cleanup(void); diff --git a/mono/utils/mono-signal-handler.h b/mono/utils/mono-signal-handler.h index 4842cb8f72b..555719e9d54 100644 --- a/mono/utils/mono-signal-handler.h +++ b/mono/utils/mono-signal-handler.h @@ -78,12 +78,20 @@ */ #ifdef HOST_WIN32 -#define MONO_SIG_HANDLER_SIGNATURE(ftn) ftn (int _dummy, EXCEPTION_POINTERS *_info, void *context) -#define MONO_SIG_HANDLER_FUNC(access, ftn) MONO_SIGNAL_HANDLER_FUNC (access, ftn, (int _dummy, EXCEPTION_POINTERS *_info, void *context)) +#define MONO_SIG_HANDLER_SIGNATURE(ftn) ftn (int _dummy, MonoWindowsSigHandlerInfo *_info, void *context) +#define MONO_SIG_HANDLER_FUNC(access, ftn) MONO_SIGNAL_HANDLER_FUNC (access, ftn, (int _dummy, MonoWindowsSigHandlerInfo *_info, void *context)) #define MONO_SIG_HANDLER_PARAMS _dummy, _info, context -#define MONO_SIG_HANDLER_GET_SIGNO() (_dummy) +#define MONO_SIG_HANDLER_GET_SIGNO() (0) #define MONO_SIG_HANDLER_GET_INFO() (_info) -#define MONO_SIG_HANDLER_INFO_TYPE EXCEPTION_POINTERS +#define MONO_SIG_HANDLER_INFO_TYPE MonoWindowsSigHandlerInfo +typedef struct { + /* Set to FALSE to indicate chained signal handler needs run. + * With vectored exceptions Windows does that for us by returning + * EXCEPTION_CONTINUE_SEARCH from handler */ + gboolean handled; + EXCEPTION_POINTERS* ep; +} MonoWindowsSigHandlerInfo; + /* seh_vectored_exception_handler () passes in a CONTEXT* */ #define MONO_SIG_HANDLER_GET_CONTEXT \ void *ctx = context; -- GitLab