From 2faff44dc8efaf23428f5f03bf6a9794f3e8fc6a Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 8 Apr 2026 18:27:21 +0200 Subject: [PATCH] [mono] Restore signal handlers during crash chaining Expose static remove_signal_handler as mono_runtime_posix_restore_handler and use it in mono_handle_native_crash to restore pre-Mono signal handlers instead of resetting to SIG_DFL. The saved handlers are kept in the hash table so that mono_chain_signal can still chain to them if needed. When signal chaining is disabled, there are no saved handlers, so it falls back to SIG_DFL, same as before. Includes an Android functional test (CrashChaining) that installs pre-Mono SIGSEGV and SIGABRT handlers and verifies they are preserved during crash chaining. --- src/mono/mono/mini/mini-exceptions.c | 14 +-- src/mono/mono/mini/mini-posix.c | 7 +- src/mono/mono/mini/mini-runtime.h | 3 + .../android/build/AndroidBuild.targets | 1 + .../AndroidAppBuilder/AndroidAppBuilder.cs | 6 ++ src/tasks/AndroidAppBuilder/ApkBuilder.cs | 8 ++ .../Templates/CMakeLists-android.txt | 1 + ....Device_Emulator.CrashChaining.Test.csproj | 18 ++++ .../Device_Emulator/CrashChaining/Program.cs | 29 +++++ .../CrashChaining/test_crash_chaining.c | 100 ++++++++++++++++++ 10 files changed, 173 insertions(+), 14 deletions(-) create mode 100644 src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/Android.Device_Emulator.CrashChaining.Test.csproj create mode 100644 src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/Program.cs create mode 100644 src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/test_crash_chaining.c diff --git a/src/mono/mono/mini/mini-exceptions.c b/src/mono/mono/mini/mini-exceptions.c index 188fa9d340ede1..c795fd2adc5d55 100644 --- a/src/mono/mono/mini/mini-exceptions.c +++ b/src/mono/mono/mini/mini-exceptions.c @@ -2955,24 +2955,18 @@ mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLE MonoJitTlsData *jit_tls = mono_tls_get_jit_tls (); #ifdef MONO_ARCH_USE_SIGACTION - struct sigaction sa; - sa.sa_handler = SIG_DFL; - sigemptyset (&sa.sa_mask); - sa.sa_flags = 0; - /* Remove our SIGABRT handler */ - g_assert (sigaction (SIGABRT, &sa, NULL) != -1); + mono_runtime_posix_restore_handler (SIGABRT); /* On some systems we get a SIGILL when calling abort (), because it might * fail to raise SIGABRT */ - g_assert (sigaction (SIGILL, &sa, NULL) != -1); + mono_runtime_posix_restore_handler (SIGILL); /* Remove SIGCHLD, it uses the finalizer thread */ - g_assert (sigaction (SIGCHLD, &sa, NULL) != -1); + mono_runtime_posix_restore_handler (SIGCHLD); /* Remove SIGQUIT, we are already dumping threads */ - g_assert (sigaction (SIGQUIT, &sa, NULL) != -1); - + mono_runtime_posix_restore_handler (SIGQUIT); #endif if (mini_debug_options.suspend_on_native_crash) { diff --git a/src/mono/mono/mini/mini-posix.c b/src/mono/mono/mini/mini-posix.c index 4c36fd6b3ffeea..292047b29a4759 100644 --- a/src/mono/mono/mini/mini-posix.c +++ b/src/mono/mono/mini/mini-posix.c @@ -389,8 +389,8 @@ add_signal_handler (int signo, MonoSignalHandler handler, int flags) } } -static void -remove_signal_handler (int signo) +void +mono_runtime_posix_restore_handler (int signo) { struct sigaction sa; struct sigaction *saved_action = get_saved_signal_handler (signo); @@ -400,11 +400,10 @@ remove_signal_handler (int signo) sigemptyset (&sa.sa_mask); sa.sa_flags = 0; - sigaction (signo, &sa, NULL); + g_assert (sigaction (signo, &sa, NULL) != -1); } else { g_assert (sigaction (signo, saved_action, NULL) != -1); } - remove_saved_signal_handler(signo); } void diff --git a/src/mono/mono/mini/mini-runtime.h b/src/mono/mono/mini/mini-runtime.h index 45fbe080cefc1b..9ca4d1dbf59aa2 100644 --- a/src/mono/mono/mini/mini-runtime.h +++ b/src/mono/mono/mini/mini-runtime.h @@ -639,6 +639,9 @@ mono_runtime_setup_stat_profiler (void); void mono_runtime_posix_install_handlers (void); +void +mono_runtime_posix_restore_handler (int signo); + void mono_gdb_render_native_backtraces (pid_t crashed_pid); diff --git a/src/mono/msbuild/android/build/AndroidBuild.targets b/src/mono/msbuild/android/build/AndroidBuild.targets index 96e8607dd80dab..a6488235a8c670 100644 --- a/src/mono/msbuild/android/build/AndroidBuild.targets +++ b/src/mono/msbuild/android/build/AndroidBuild.targets @@ -254,6 +254,7 @@ DiagnosticPorts="$(DiagnosticPorts)" EnvironmentVariables="@(AndroidEnv)" ExtraLinkerArguments="@(ExtraAppLinkerArgs)" + ExtraNativeSources="@(ExtraAppNativeSources)" ForceAOT="$(RunAOTCompilation)" ForceFullAOT="$(ForceFullAOT)" ForceInterpreter="$(MonoForceInterpreter)" diff --git a/src/tasks/AndroidAppBuilder/AndroidAppBuilder.cs b/src/tasks/AndroidAppBuilder/AndroidAppBuilder.cs index 006b746ef0d010..25b6157eeeee50 100644 --- a/src/tasks/AndroidAppBuilder/AndroidAppBuilder.cs +++ b/src/tasks/AndroidAppBuilder/AndroidAppBuilder.cs @@ -38,6 +38,11 @@ public class AndroidAppBuilderTask : Task /// public ITaskItem[] ExtraLinkerArguments { get; set; } = Array.Empty(); + /// + /// Additional native source files to compile alongside monodroid.c + /// + public ITaskItem[] ExtraNativeSources { get; set; } = Array.Empty(); + /// /// Prefer FullAOT mode for Emulator over JIT /// @@ -140,6 +145,7 @@ public override bool Execute() apkBuilder.IsLibraryMode = IsLibraryMode; apkBuilder.NativeDependencies = NativeDependencies; apkBuilder.ExtraLinkerArguments = ExtraLinkerArguments; + apkBuilder.ExtraNativeSources = ExtraNativeSources; apkBuilder.RuntimeFlavor = RuntimeFlavor; (ApkBundlePath, ApkPackageId) = apkBuilder.BuildApk(RuntimeIdentifier, MainLibraryFileName, RuntimeHeaders); diff --git a/src/tasks/AndroidAppBuilder/ApkBuilder.cs b/src/tasks/AndroidAppBuilder/ApkBuilder.cs index df0a274e9d4895..4f905661b20b21 100644 --- a/src/tasks/AndroidAppBuilder/ApkBuilder.cs +++ b/src/tasks/AndroidAppBuilder/ApkBuilder.cs @@ -49,6 +49,7 @@ public partial class ApkBuilder public bool IsLibraryMode { get; set; } public ITaskItem[] Assemblies { get; set; } = Array.Empty(); public ITaskItem[] ExtraLinkerArguments { get; set; } = Array.Empty(); + public ITaskItem[] ExtraNativeSources { get; set; } = Array.Empty(); public string[] NativeDependencies { get; set; } = Array.Empty(); public string RuntimeFlavor { get; set; } = nameof(RuntimeFlavorEnum.Mono); @@ -357,10 +358,17 @@ public ApkBuilder(TaskLoggingHelper logger) "monodroid-coreclr.c" : (IsLibraryMode) ? "monodroid-librarymode.c" : "monodroid.c"; string runtimeInclude = string.Join(" ", runtimeHeaders.Select(h => $"\"{NormalizePathToUnix(h)}\"")); + var extraSources = new StringBuilder(); + foreach (ITaskItem item in ExtraNativeSources) + { + extraSources.AppendLine($" \"{NormalizePathToUnix(item.GetMetadata("FullPath"))}\""); + } + string cmakeLists = Utils.GetEmbeddedResource("CMakeLists-android.txt") .Replace("%RuntimeInclude%", runtimeInclude) .Replace("%NativeLibrariesToLink%", NormalizePathToUnix(nativeLibraries)) .Replace("%MONODROID_SOURCE%", monodroidSource) + .Replace("%ExtraSources%", extraSources.ToString().TrimEnd()) .Replace("%AotSources%", NormalizePathToUnix(aotSources)) .Replace("%AotModulesSource%", string.IsNullOrEmpty(aotSources) ? "" : "modules.c") .Replace("%APP_LINKER_ARGS%", extraLinkerArgs.ToString()); diff --git a/src/tasks/AndroidAppBuilder/Templates/CMakeLists-android.txt b/src/tasks/AndroidAppBuilder/Templates/CMakeLists-android.txt index 23331319df0bd9..15eab832eb9ec7 100644 --- a/src/tasks/AndroidAppBuilder/Templates/CMakeLists-android.txt +++ b/src/tasks/AndroidAppBuilder/Templates/CMakeLists-android.txt @@ -15,6 +15,7 @@ add_library( SHARED %MONODROID_SOURCE% %AotModulesSource% + %ExtraSources% ) %AotSources% diff --git a/src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/Android.Device_Emulator.CrashChaining.Test.csproj b/src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/Android.Device_Emulator.CrashChaining.Test.csproj new file mode 100644 index 00000000000000..91cd2706c80b34 --- /dev/null +++ b/src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/Android.Device_Emulator.CrashChaining.Test.csproj @@ -0,0 +1,18 @@ + + + Exe + false + true + $(NetCoreAppCurrent) + Android.Device_Emulator.CrashChaining.Test.dll + 42 + + + + + + + + + + diff --git a/src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/Program.cs b/src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/Program.cs new file mode 100644 index 00000000000000..bbe31e77e45834 --- /dev/null +++ b/src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/Program.cs @@ -0,0 +1,29 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +/// +/// Test that crash chaining preserves signal handlers during +/// mono_handle_native_crash. When crash_chaining is enabled, +/// mono_handle_native_crash should NOT reset SIGABRT etc. to SIG_DFL, +/// because that would let secondary signals (e.g. FORTIFY aborts on +/// other threads) kill the process before the crash can be chained. +/// +public static class Program +{ + // Returns 0 on success (SIGABRT handler preserved during crash chaining), + // non-zero on failure. + [DllImport("__Internal")] + private static extern int test_crash_chaining(); + + public static int Main() + { + int result = test_crash_chaining(); + Console.WriteLine(result == 0 + ? "PASS: crash chaining preserved signal handlers" + : $"FAIL: crash chaining test returned {result}"); + return result == 0 ? 42 : result; + } +} diff --git a/src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/test_crash_chaining.c b/src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/test_crash_chaining.c new file mode 100644 index 00000000000000..305c0320324c31 --- /dev/null +++ b/src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/test_crash_chaining.c @@ -0,0 +1,100 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include +#include +#include +#include + +#define LOG_ERROR(fmt, ...) __android_log_print(ANDROID_LOG_ERROR, "DOTNET", fmt, ##__VA_ARGS__) + +int test_crash_chaining(void); + +/* + * Test for crash chaining: verify that mono_handle_native_crash does not + * reset SIGABRT to SIG_DFL when crash_chaining is enabled. + * + * Strategy: + * 1. Install pre-Mono SIGSEGV and SIGABRT handlers before mono_jit_init. + * 2. Trigger a SIGSEGV from a non-JIT native function. + * 3. Mono chains to our SIGSEGV handler (because signal_chaining is enabled). + * 4. Our handler checks that SIGABRT was restored to the pre-Mono handler. + * + * Returns 0 on success, non-zero on failure. + */ +static volatile sig_atomic_t g_test_crash_chain_result = -1; +static sigjmp_buf g_test_jmpbuf; + +__attribute__((noinline)) +static void do_test_crash(void) +{ + volatile int *ptr = 0; + *ptr = 42; +} + +static void +test_sigabrt_handler(int signum) +{ + (void)signum; +} + +/** + * Pre-Mono SIGSEGV handler installed before mono_jit_init. Mono's + * signal chaining saves this handler and calls it via mono_chain_signal + * when a native crash occurs. + * + * After Mono's crash diagnostics (mono_handle_native_crash) run, this + * handler verifies that SIGABRT was restored to the pre-Mono handler. + * Without the fix, mono_handle_native_crash unconditionally resets + * SIGABRT to SIG_DFL. With the fix, SIGABRT is restored to the + * pre-Mono handler (test_sigabrt_handler). + */ +static void +test_pre_mono_sigsegv_handler(int signum, siginfo_t *info, void *context) +{ + (void)signum; + (void)info; + (void)context; + + // By the time Mono chains to us, mono_handle_native_crash has + // already run. Check if SIGABRT was restored to our pre-Mono handler. + struct sigaction current_abrt; + sigaction(SIGABRT, NULL, ¤t_abrt); + + g_test_crash_chain_result = (current_abrt.sa_handler == test_sigabrt_handler) ? 0 : 1; + + siglongjmp(g_test_jmpbuf, 1); +} + +__attribute__((constructor)) +static void install_pre_mono_handler(void) +{ + struct sigaction sa; + memset(&sa, 0, sizeof(sa)); + sa.sa_sigaction = test_pre_mono_sigsegv_handler; + sa.sa_flags = SA_SIGINFO; + sigemptyset(&sa.sa_mask); + sigaction(SIGSEGV, &sa, NULL); + + struct sigaction sa_abrt; + memset(&sa_abrt, 0, sizeof(sa_abrt)); + sa_abrt.sa_handler = test_sigabrt_handler; + sigemptyset(&sa_abrt.sa_mask); + sigaction(SIGABRT, &sa_abrt, NULL); +} + +int +test_crash_chaining(void) +{ + g_test_crash_chain_result = -1; + if (sigsetjmp(g_test_jmpbuf, 1) == 0) { + do_test_crash(); + } + + if (g_test_crash_chain_result == -1) { + LOG_ERROR("test_crash_chaining: handler was not called"); + return 3; + } + + return g_test_crash_chain_result; +}