From c9fbe018f18d1cff8a4b30261ea4b1e981ef435b Mon Sep 17 00:00:00 2001
From: Martin Robinson <mrobinson@igalia.com>
Date: Tue, 6 Aug 2024 19:43:06 +0200
Subject: [PATCH] testing: Trigger a crash more reliably when panicking and
 hard fail is active (#32947)

Before when handling panics and hard-fail was activated, Servo would
just exit with an error return code. This isn't interpreted as a crash
by the WPT test runner. This change raises a SEGV signal instead, which
means that panics should more reliably be treated as crashes.

This doesn't seem to change any test results, at least any non-flaky
test results. It is necessary for the test in #32782 to work though.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
---
 ports/servoshell/crash_handler.rs | 42 ++++++++++++++++++-------------
 ports/servoshell/panic_hook.rs    |  6 ++++-
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/ports/servoshell/crash_handler.rs b/ports/servoshell/crash_handler.rs
index 3dd6aa86604..0644ee8a601 100644
--- a/ports/servoshell/crash_handler.rs
+++ b/ports/servoshell/crash_handler.rs
@@ -11,8 +11,6 @@ pub fn install() {
     use std::sync::atomic;
     use std::thread;
 
-    use sig::ffi::Sig;
-
     use crate::backtrace;
 
     extern "C" fn handler(sig: i32) {
@@ -42,21 +40,31 @@ pub fn install() {
         // know to be “async-signal-safe”, which includes sigaction(), raise(),
         // and _exit(), but generally doesn’t include anything that allocates.
         // https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03_03
-        unsafe {
-            // Reset the signal to the default action, and reraise the signal.
-            // Unlike libc::_exit(sig), which terminates the process normally,
-            // this terminates abnormally just like an uncaught signal, allowing
-            // mach (or your shell) to distinguish it from an ordinary exit, and
-            // allows your kernel to make a core dump if configured to do so.
-            let mut action: libc::sigaction = std::mem::zeroed();
-            action.sa_sigaction = libc::SIG_DFL;
-            libc::sigaction(sig, &action, std::ptr::null_mut());
-            libc::raise(sig);
-        }
+        raise_signal_or_exit_with_error(sig);
     }
 
-    signal!(Sig::SEGV, handler); // handle segfaults
-    signal!(Sig::ILL, handler); // handle stack overflow and unsupported CPUs
-    signal!(Sig::IOT, handler); // handle double panics
-    signal!(Sig::BUS, handler); // handle invalid memory access
+    signal!(libc::SIGSEGV, handler); // handle segfaults
+    signal!(libc::SIGILL, handler); // handle stack overflow and unsupported CPUs
+    signal!(libc::SIGIOT, handler); // handle double panics
+    signal!(libc::SIGBUS, handler); // handle invalid memory access
+}
+
+#[cfg(not(any(target_os = "macos", target_os = "linux")))]
+pub(crate) fn raise_signal_or_exit_with_error(_signal: i32) {
+    std::process::exit(1);
+}
+
+#[cfg(any(target_os = "macos", target_os = "linux"))]
+pub(crate) fn raise_signal_or_exit_with_error(signal: i32) {
+    unsafe {
+        // Reset the signal to the default action, and reraise the signal.
+        // Unlike libc::_exit(sig), which terminates the process normally,
+        // this terminates abnormally just like an uncaught signal, allowing
+        // mach (or your shell) to distinguish it from an ordinary exit, and
+        // allows your kernel to make a core dump if configured to do so.
+        let mut action: libc::sigaction = std::mem::zeroed();
+        action.sa_sigaction = libc::SIG_DFL;
+        libc::sigaction(signal, &action, std::ptr::null_mut());
+        libc::raise(signal);
+    }
 }
diff --git a/ports/servoshell/panic_hook.rs b/ports/servoshell/panic_hook.rs
index c71f27d2dfd..ed376308b4f 100644
--- a/ports/servoshell/panic_hook.rs
+++ b/ports/servoshell/panic_hook.rs
@@ -9,6 +9,8 @@ use std::{env, thread};
 use log::{error, warn};
 use servo::config::opts;
 
+use crate::crash_handler::raise_signal_or_exit_with_error;
+
 pub(crate) fn panic_hook(info: &PanicInfo) {
     warn!("Panic hook called.");
     let msg = match info.payload().downcast_ref::<&'static str>() {
@@ -40,7 +42,9 @@ pub(crate) fn panic_hook(info: &PanicInfo) {
     drop(stderr);
 
     if opts::get().hard_fail && !opts::get().multiprocess {
-        std::process::exit(1);
+        // When we are exiting due to a hard-failure mode, we trigger a segfault so that crash
+        // tests detect that we crashed. If we exit normally it just looks like a non-crash exit.
+        raise_signal_or_exit_with_error(libc::SIGSEGV);
     }
 
     error!("{}", msg);