[crashing] Improve crash chaining (#20018)

With this change, `mono_post_native_crash_handler` chains the crashing signal directly, instead of that having to happen after `mono_handle_native_crash` returns (which created a fragile repetitive code pattern). Finally, this way `mono_post_native_crash_handler` can also re-raise the signal right after chaining it. Since we remove our crashing signal handlers in `mono_handle_native_crash`, this should allow the OS to handle the signal as the final action.

Fixes https://github.com/mono/mono/issues/19860

9 files changed, 38 insertions, 46 deletions

diff --git a/mono/mini/exceptions-amd64.c b/mono/mini/exceptions-amd64.c
index cd0397ef39e..fa144e5d579 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);
+		mono_handle_native_crash (mono_get_signame (SIGSEGV), NULL, 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);
+			mono_handle_native_crash (mono_get_signame (SIGSEGV), ctx, NULL, 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 867dd1eae0f..0769a255ad9 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);
+			mono_handle_native_crash (mono_get_signame (SIGSEGV), (MonoContext*)sigctx, siginfo, sigctx);
 	/* 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 22e54bc9258..adaa6680f9b 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);
+		mono_handle_native_crash (mono_get_signame (SIGSEGV), NULL, 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);
+			mono_handle_native_crash (mono_get_signame (SIGSEGV), &mctx, siginfo, sigctx);
 		else
 			abort ();
 	}
diff --git a/mono/mini/mini-exceptions.c b/mono/mini/mini-exceptions.c
index fd304a6e7c8..21423ebf200 100644
--- a/mono/mini/mini-exceptions.c
+++ b/mono/mini/mini-exceptions.c
@@ -3375,7 +3375,7 @@ static gboolean handle_crash_loop = FALSE;
  *   printing diagnostic information and aborting.
  */
 void
-mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info)
+mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, void *context)
 {
 	MonoJitTlsData *jit_tls = mono_tls_get_jit_tls ();
 
@@ -3388,17 +3388,20 @@ mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLE
 	sigemptyset (&sa.sa_mask);
 	sa.sa_flags = 0;
 
-	/* Remove our SIGABRT handler */
-	g_assert (sigaction (SIGABRT, &sa, NULL) != -1);
+	/* 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);
+	}
 
-	/* On some systems we get a SIGILL when calling abort (), because it might
-	 * fail to raise SIGABRT */
+	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);
 	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
@@ -3441,13 +3444,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);
+	mono_post_native_crash_handler (signal, mctx, info, mono_do_crash_chaining, context);
 }
 
 #else
 
 void
-mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info)
+mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, void *context)
 {
 	g_assert_not_reached ();
 }
diff --git a/mono/mini/mini-posix.c b/mono/mini/mini-posix.c
index 7f9537423d1..89c890a01c0 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);
+			mono_handle_native_crash (mono_get_signame (info->si_signo), &mctx, info, ctx);
 		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);
+		mono_handle_native_crash (mono_get_signame (info->si_signo), &mctx, NULL, ctx);
 	} 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)
+mono_post_native_crash_handler (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, gboolean crash_chaining, void *context)
 {
 	if (!crash_chaining) {
 		/*Android abort is a fluke, it doesn't abort, it triggers another segv. */
@@ -1168,6 +1168,11 @@ 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 afb7f875e09..f027be730c1 100644
--- a/mono/mini/mini-runtime.c
+++ b/mono/mini/mini-runtime.c
@@ -3228,11 +3228,8 @@ 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);
-		if (mono_do_crash_chaining) {
-			mono_chain_signal (MONO_SIG_HANDLER_PARAMS);
-			goto exit;
-		}
+			mono_handle_native_crash (mono_get_signame (SIGFPE), &mctx, info, ctx);
+		goto exit;
 	}
 
 	mono_arch_handle_exception (ctx, exc);
@@ -3253,14 +3250,10 @@ 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);
+		mono_handle_native_crash (mono_get_signame (info->si_signo), &mctx, info, ctx);
 #else
-		mono_handle_native_crash (mono_get_signame (SIGTERM), &mctx, info);
+		mono_handle_native_crash (mono_get_signame (SIGTERM), &mctx, info, ctx);
 #endif
-	if (mono_do_crash_chaining) {
-		mono_chain_signal (MONO_SIG_HANDLER_PARAMS);
-		return;
-	}
 }
 
 #if defined(MONO_ARCH_USE_SIGACTION) || defined(HOST_WIN32)
@@ -3340,11 +3333,7 @@ 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);
-		if (mono_do_crash_chaining) {
-			mono_chain_signal (MONO_SIG_HANDLER_PARAMS);
-			return;
-		}
+			mono_handle_native_crash (mono_get_signame (signo), &mctx, info, ctx);
 	}
 #endif
 
@@ -3383,7 +3372,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);
+				mono_handle_native_crash (mono_get_signame (SIGSEGV), &mctx, info, ctx);
 		}
 #endif
 	}
@@ -3394,19 +3383,14 @@ 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);
-
-		if (mono_do_crash_chaining) {
-			mono_chain_signal (MONO_SIG_HANDLER_PARAMS);
-			return;
-		}
+			mono_handle_native_crash (mono_get_signame (SIGSEGV), &mctx, (MONO_SIG_HANDLER_INFO_TYPE*)info, ctx);
 	}
 
 	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);
+			mono_handle_native_crash (mono_get_signame (SIGSEGV), &mctx, (MONO_SIG_HANDLER_INFO_TYPE*)info, ctx);
 	}
 #endif
 }
diff --git a/mono/mini/mini-runtime.h b/mono/mini/mini-runtime.h
index e2fd25ae4d7..9c429cbc446 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);
+mono_post_native_crash_handler (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, gboolean crash_chaining, void *context);
 
 gboolean
 mono_is_addr_implicit_null_check (void *addr);
diff --git a/mono/mini/mini-windows.c b/mono/mini/mini-windows.c
index 673aeefeeed..f78caaeee12 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)
+mono_post_native_crash_handler (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *info, gboolean crash_chaining, void *context)
 {
 	if (!crash_chaining)
 		abort ();
diff --git a/mono/mini/mini.h b/mono/mini/mini.h
index ca2d416e733..bf70f4e805a 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     mono_handle_native_crash               (const char *signal, MonoContext *mctx, MONO_SIG_HANDLER_INFO_TYPE *siginfo, void *context);
 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);