Skip to content

Commit

Permalink
Auto merge of #31457 - lambda:rtabort-use-libc-abort, r=alexcrichton
Browse files Browse the repository at this point in the history
Use libc::abort, not intrinsics::abort, in rtabort!

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
  • Loading branch information
bors committed May 23, 2016
2 parents e24d621 + cfc3865 commit 6e45564
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 4 deletions.
26 changes: 24 additions & 2 deletions src/libstd/sys/common/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

use env;
use fmt;
use intrinsics;
use io::prelude::*;
use sync::atomic::{self, Ordering};
use sys::stdio::Stderr;
Expand All @@ -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
Expand Down
3 changes: 1 addition & 2 deletions src/test/run-pass/out-of-stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,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))]
Expand Down

0 comments on commit 6e45564

Please sign in to comment.