Use libc::abort, not intrinsics::abort, in rtabort!

lambda · lambda · commit cfc386583291 · 2016-05-23T00:22:41.000-04:00
intrinsics::abort compiles down to an illegal instruction, which on Unix-like platforms causes the process to be killed with SIGILL. A more appropriate way to kill the process would be SIGABRT; this indicates better that the runtime has explicitly aborted, rather than some kind of compiler bug or architecture mismatch that SIGILL might indicate. For rtassert!, replace this with libc::abort. libc::abort raises SIGABRT, but is defined to do so in such a way that it will terminate the process even if SIGABRT is currently masked or caught by a signal handler that returns. On non-Unix platforms, retain the existing behavior. On Windows we prefer to avoid depending on the C runtime, and we need a fallback for any other platforms that may be defined. An alternative on Windows would be to call TerminateProcess, but this seems less essential than switching to using SIGABRT on Unix-like platforms, where it is common for the process-killing signal to be printed out or logged. This is a [breaking-change] for any code that depends on the exact signal raised to abort a process via rtabort! cc #31273 cc #31333
diff --git a/src/libstd/sys/common/util.rs b/src/libstd/sys/common/util.rs
@@ -10,7 +10,6 @@
 
 use env;
 use fmt;
-use intrinsics;
 use io::prelude::*;
 use sync::atomic::{self, Ordering};
 use sys::stdio::Stderr;
@@ -34,9 +33,32 @@ pub fn dumb_print(args: fmt::Arguments) {
     let _ = Stderr::new().map(|mut stderr| stderr.write_fmt(args));
 }
 
+// On Unix-like platforms, libc::abort will unregister signal handlers
+// including the SIGABRT handler, preventing the abort from being blocked, and
+// fclose streams, with the side effect of flushing them so libc bufferred
+// output will be printed.  Additionally the shell will generally print a more
+// understandable error message like "Abort trap" rather than "Illegal
+// instruction" that intrinsics::abort would cause, as intrinsics::abort is
+// implemented as an illegal instruction.
+#[cfg(unix)]
+unsafe fn abort_internal() -> ! {
+    use libc;
+    libc::abort()
+}
+
+// On Windows, we want to avoid using libc, and there isn't a direct
+// equivalent of libc::abort.  The __failfast intrinsic may be a reasonable
+// substitute, but desireability of using it over the abort instrinsic is
+// debateable; see https://github.com/rust-lang/rust/pull/31519 for details.
+#[cfg(not(unix))]
+unsafe fn abort_internal() -> ! {
+    use intrinsics;
+    intrinsics::abort()
+}
+
 pub fn abort(args: fmt::Arguments) -> ! {
     dumb_print(format_args!("fatal runtime error: {}\n", args));
-    unsafe { intrinsics::abort(); }
+    unsafe { abort_internal(); }
 }
 
 #[allow(dead_code)] // stack overflow detection not enabled on all platforms
diff --git a/src/test/run-pass/out-of-stack.rs b/src/test/run-pass/out-of-stack.rs
@@ -46,8 +46,7 @@ fn check_status(status: std::process::ExitStatus)
     use std::os::unix::process::ExitStatusExt;
 
     assert!(!status.success());
-    assert!(status.signal() != Some(libc::SIGSEGV)
-            && status.signal() != Some(libc::SIGBUS));
+    assert_eq!(status.signal(), Some(libc::SIGABRT));
 }
 
 #[cfg(not(unix))]

Original file line number	Diff line number	Diff line change
`@@ -46,8 +46,7 @@ fn check_status(status: std::process::ExitStatus)`
`46`	`46`	`use std::os::unix::process::ExitStatusExt;`
`47`	`47`
`48`	`48`	`assert!(!status.success());`
`49`		`- assert!(status.signal() != Some(libc::SIGSEGV)`
`50`		`- && status.signal() != Some(libc::SIGBUS));`
	`49`	`+ assert_eq!(status.signal(), Some(libc::SIGABRT));`
`51`	`50`	`}`
`52`	`51`
`53`	`52`	`#[cfg(not(unix))]`