From be2226b5a1c57df065efc4c1cf008d581e5cec7d Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Tue, 1 Sep 2020 11:37:43 -0400 Subject: Revert "[crashing] Improve crash chaining (#19973)" This reverts commit 7a0425e81ffe0e77a4ba22ff45e691934ecc49a1. This change was causing this android issue: https://github.com/mono/mono/issues/20275 --- mono/mini/exceptions-amd64.c | 4 ++-- mono/mini/exceptions-ppc.c | 2 +- mono/mini/exceptions-x86.c | 4 ++-- mono/mini/mini-exceptions.c | 25 +++++++++++-------------- mono/mini/mini-posix.c | 11 +++-------- mono/mini/mini-runtime.c | 36 ++++++++++++++++++++++++++++-------- mono/mini/mini-runtime.h | 2 +- mono/mini/mini-windows.c | 2 +- mono/mini/mini.h | 2 +- 9 files changed, 50 insertions(+), 38 deletions(-) diff --git a/mono/mini/exceptions-amd64.c b/mono/mini/exceptions-amd64.c index fa144e5d579..cd0397ef39e 100644 --- a/mono/mini/exceptions-amd64.c +++ b/mono/mini/exceptions-amd64.c @@ -66,7 +66,7 @@ static LONG CALLBACK seh_unhandled_exception_filter(EXCEPTION_POINTERS* ep) #endif if (mono_dump_start ()) - mono_handle_native_crash (mono_get_signame (SIGSEGV), NULL, NULL, NULL); + mono_handle_native_crash (mono_get_signame (SIGSEGV), NULL, NULL); return EXCEPTION_CONTINUE_SEARCH; } @@ -876,7 +876,7 @@ altstack_handle_and_restore (MonoContext *ctx, MonoObject *obj, guint32 flags) if (!ji || (!stack_ovf && !nullref)) { if (mono_dump_start ()) - mono_handle_native_crash (mono_get_signame (SIGSEGV), ctx, NULL, NULL); + mono_handle_native_crash (mono_get_signame (SIGSEGV), ctx, NULL); // if couldn't dump or if mono_handle_native_crash returns, abort abort (); } diff --git a/mono/mini/exceptions-ppc.c b/mono/mini/exceptions-ppc.c index 0769a255ad9..867dd1eae0f 100644 --- a/mono/mini/exceptions-ppc.c +++ b/mono/mini/exceptions-ppc.c @@ -676,7 +676,7 @@ mono_arch_handle_altstack_exception (void *sigctx, MONO_SIG_HANDLER_INFO_TYPE *s } if (!ji) if (mono_dump_start ()) - mono_handle_native_crash (mono_get_signame (SIGSEGV), (MonoContext*)sigctx, siginfo, sigctx); + mono_handle_native_crash (mono_get_signame (SIGSEGV), (MonoContext*)sigctx, siginfo); /* setup a call frame on the real stack so that control is returned there * and exception handling can continue. * The frame looks like: diff --git a/mono/mini/exceptions-x86.c b/mono/mini/exceptions-x86.c index adaa6680f9b..22e54bc9258 100644 --- a/mono/mini/exceptions-x86.c +++ b/mono/mini/exceptions-x86.c @@ -67,7 +67,7 @@ static LONG CALLBACK seh_unhandled_exception_filter(EXCEPTION_POINTERS* ep) } #endif if (mono_dump_start ()) - mono_handle_native_crash (mono_get_signame (SIGSEGV), NULL, NULL, NULL); + mono_handle_native_crash (mono_get_signame (SIGSEGV), NULL, NULL); return EXCEPTION_CONTINUE_SEARCH; } @@ -1140,7 +1140,7 @@ mono_arch_handle_altstack_exception (void *sigctx, MONO_SIG_HANDLER_INFO_TYPE *s MonoContext mctx; mono_sigctx_to_monoctx (sigctx, &mctx); if (mono_dump_start ()) - mono_handle_native_crash (mono_get_signame (SIGSEGV), &mctx, siginfo, sigctx); + mono_handle_native_crash (mono_get_signame (SIGSEGV), &mctx, siginfo); else abort (); } diff --git a/mono/mini/mini-exceptions.c b/mono/mini/mini-exceptions.c index 14b6fbba559..e712a623bb4 100644 --- a/mono/mini/mini-exceptions.c +++ b/mono/mini/mini-exceptions.c @@ -3374,7 +3374,7 @@ print_stack_frame_to_string (StackFrameInfo *frame, MonoContext *ctx, gpointer d * printing diagnostic information and aborting. */ void -mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, void *context) +mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info) { MonoJitTlsData *jit_tls = mono_tls_get_jit_tls (); @@ -3384,20 +3384,17 @@ mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLE sigemptyset (&sa.sa_mask); sa.sa_flags = 0; - /* Mono has crashed - remove our handlers except SIGTERM, which is used by crash reporting */ - /* TODO: Combine with mono_runtime_cleanup_handlers (but with an option to not de-allocate anything) */ - if (mini_debug_options.handle_sigint) { - g_assert (sigaction (SIGINT, &sa, NULL) != -1); - } - + /* Remove our SIGABRT handler */ g_assert (sigaction (SIGABRT, &sa, NULL) != -1); - g_assert (sigaction (SIGFPE, &sa, NULL) != -1); - g_assert (sigaction (SIGSYS, &sa, NULL) != -1); - g_assert (sigaction (SIGSEGV, &sa, NULL) != -1); - g_assert (sigaction (SIGQUIT, &sa, NULL) != -1); - g_assert (sigaction (SIGBUS, &sa, NULL) != -1); + + /* On some systems we get a SIGILL when calling abort (), because it might + * fail to raise SIGABRT */ g_assert (sigaction (SIGILL, &sa, NULL) != -1); + + /* Remove SIGCHLD, it uses the finalizer thread */ g_assert (sigaction (SIGCHLD, &sa, NULL) != -1); + + /* Remove SIGQUIT, we are already dumping threads */ g_assert (sigaction (SIGQUIT, &sa, NULL) != -1); #endif @@ -3437,13 +3434,13 @@ mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLE g_async_safe_printf ("=================================================================\n"); } - mono_post_native_crash_handler (signal, mctx, info, mono_do_crash_chaining, context); + mono_post_native_crash_handler (signal, mctx, info, mono_do_crash_chaining); } #else void -mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, void *context) +mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info) { g_assert_not_reached (); } diff --git a/mono/mini/mini-posix.c b/mono/mini/mini-posix.c index 89c890a01c0..7f9537423d1 100644 --- a/mono/mini/mini-posix.c +++ b/mono/mini/mini-posix.c @@ -231,7 +231,7 @@ MONO_SIG_HANDLER_FUNC (static, sigabrt_signal_handler) return; mono_sigctx_to_monoctx (ctx, &mctx); if (mono_dump_start ()) - mono_handle_native_crash (mono_get_signame (info->si_signo), &mctx, info, ctx); + mono_handle_native_crash (mono_get_signame (info->si_signo), &mctx, info); else abort (); } @@ -253,7 +253,7 @@ MONO_SIG_HANDLER_FUNC (static, sigterm_signal_handler) // running. Returns FALSE on unrecoverable error. if (mono_dump_start ()) { // Process was killed from outside since crash reporting wasn't running yet. - mono_handle_native_crash (mono_get_signame (info->si_signo), &mctx, NULL, ctx); + mono_handle_native_crash (mono_get_signame (info->si_signo), &mctx, NULL); } else { // Crash reporting already running and we got a second SIGTERM from as part of thread-summarizing if (!mono_threads_summarize_execute (&mctx, &output, &hashes, FALSE, NULL, 0)) @@ -1158,7 +1158,7 @@ mono_dump_native_crash_info (const char *signal, MonoContext *mctx, MONO_SIG_HAN } void -mono_post_native_crash_handler (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, gboolean crash_chaining, void *context) +mono_post_native_crash_handler (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, gboolean crash_chaining) { if (!crash_chaining) { /*Android abort is a fluke, it doesn't abort, it triggers another segv. */ @@ -1168,11 +1168,6 @@ mono_post_native_crash_handler (const char *signal, MonoContext *mctx, MONO_SIG_ abort (); #endif } - mono_chain_signal (info->si_signo, info, context); - - // we remove Mono's signal handlers from crashing signals in mono_handle_native_crash(), so re-raising will now allow the OS to handle the crash - // TODO: perhaps we can always use this to abort, instead of explicit exit()/abort() as we do above - raise (info->si_signo); } #endif /* !MONO_CROSS_COMPILE */ diff --git a/mono/mini/mini-runtime.c b/mono/mini/mini-runtime.c index f027be730c1..3b44af7adff 100644 --- a/mono/mini/mini-runtime.c +++ b/mono/mini/mini-runtime.c @@ -3228,8 +3228,11 @@ MONO_SIG_HANDLER_FUNC (, mono_sigfpe_signal_handler) mono_sigctx_to_monoctx (ctx, &mctx); if (mono_dump_start ()) - mono_handle_native_crash (mono_get_signame (SIGFPE), &mctx, info, ctx); - goto exit; + mono_handle_native_crash (mono_get_signame (SIGFPE), &mctx, info); + if (mono_do_crash_chaining) { + mono_chain_signal (MONO_SIG_HANDLER_PARAMS); + goto exit; + } } mono_arch_handle_exception (ctx, exc); @@ -3250,10 +3253,14 @@ MONO_SIG_HANDLER_FUNC (, mono_crashing_signal_handler) mono_sigctx_to_monoctx (ctx, &mctx); if (mono_dump_start ()) #if defined(HAVE_SIG_INFO) && !defined(HOST_WIN32) // info is a siginfo_t - mono_handle_native_crash (mono_get_signame (info->si_signo), &mctx, info, ctx); + mono_handle_native_crash (mono_get_signame (info->si_signo), &mctx, info); #else - mono_handle_native_crash (mono_get_signame (SIGTERM), &mctx, info, ctx); + mono_handle_native_crash (mono_get_signame (SIGTERM), &mctx, info); #endif + if (mono_do_crash_chaining) { + mono_chain_signal (MONO_SIG_HANDLER_PARAMS); + return; + } } #if defined(MONO_ARCH_USE_SIGACTION) || defined(HOST_WIN32) @@ -3333,7 +3340,11 @@ MONO_SIG_HANDLER_FUNC (, mono_sigsegv_signal_handler) if (!mono_do_crash_chaining && mono_chain_signal (MONO_SIG_HANDLER_PARAMS)) return; if (mono_dump_start()) - mono_handle_native_crash (mono_get_signame (signo), &mctx, info, ctx); + mono_handle_native_crash (mono_get_signame (signo), &mctx, info); + if (mono_do_crash_chaining) { + mono_chain_signal (MONO_SIG_HANDLER_PARAMS); + return; + } } #endif @@ -3372,7 +3383,7 @@ MONO_SIG_HANDLER_FUNC (, mono_sigsegv_signal_handler) } else { // FIXME: This shouldn't run on the altstack if (mono_dump_start ()) - mono_handle_native_crash (mono_get_signame (SIGSEGV), &mctx, info, ctx); + mono_handle_native_crash (mono_get_signame (SIGSEGV), &mctx, info); } #endif } @@ -3383,14 +3394,23 @@ MONO_SIG_HANDLER_FUNC (, mono_sigsegv_signal_handler) return; if (mono_dump_start ()) - mono_handle_native_crash (mono_get_signame (SIGSEGV), &mctx, (MONO_SIG_HANDLER_INFO_TYPE*)info, ctx); + mono_handle_native_crash (mono_get_signame (SIGSEGV), &mctx, (MONO_SIG_HANDLER_INFO_TYPE*)info); + + if (mono_do_crash_chaining) { + mono_chain_signal (MONO_SIG_HANDLER_PARAMS); + return; + } } if (mono_is_addr_implicit_null_check (fault_addr)) { mono_arch_handle_exception (ctx, NULL); } else { if (mono_dump_start ()) - mono_handle_native_crash (mono_get_signame (SIGSEGV), &mctx, (MONO_SIG_HANDLER_INFO_TYPE*)info, ctx); + mono_handle_native_crash (mono_get_signame (SIGSEGV), &mctx, (MONO_SIG_HANDLER_INFO_TYPE*)info); + if (mono_do_crash_chaining) { + mono_chain_signal (MONO_SIG_HANDLER_PARAMS); + return; + } } #endif } diff --git a/mono/mini/mini-runtime.h b/mono/mini/mini-runtime.h index 9c429cbc446..e2fd25ae4d7 100644 --- a/mono/mini/mini-runtime.h +++ b/mono/mini/mini-runtime.h @@ -556,7 +556,7 @@ void mono_dump_native_crash_info (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info); void -mono_post_native_crash_handler (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, gboolean crash_chaining, void *context); +mono_post_native_crash_handler (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, gboolean crash_chaining); gboolean mono_is_addr_implicit_null_check (void *addr); diff --git a/mono/mini/mini-windows.c b/mono/mini/mini-windows.c index f78caaeee12..673aeefeeed 100644 --- a/mono/mini/mini-windows.c +++ b/mono/mini/mini-windows.c @@ -281,7 +281,7 @@ mono_dump_native_crash_info (const char *signal, MonoContext *mctx, MONO_SIG_HAN } void -mono_post_native_crash_handler (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, gboolean crash_chaining, void *context) +mono_post_native_crash_handler (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, gboolean crash_chaining) { if (!crash_chaining) abort (); diff --git a/mono/mini/mini.h b/mono/mini/mini.h index bf70f4e805a..ca2d416e733 100644 --- a/mono/mini/mini.h +++ b/mono/mini/mini.h @@ -2503,7 +2503,7 @@ typedef gboolean (*MonoJitStackWalk) (StackFrameInfo *frame, MonoCont void mono_exceptions_init (void); gboolean mono_handle_exception (MonoContext *ctx, gpointer obj); -void mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *siginfo, void *context); +void mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *siginfo); MONO_API void mono_print_thread_dump (void *sigctx); MONO_API void mono_print_thread_dump_from_ctx (MonoContext *ctx); void mono_walk_stack_with_ctx (MonoJitStackWalk func, MonoContext *start_ctx, MonoUnwindOptions unwind_options, void *user_data); -- cgit v1.2.3