Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aborts: Clarify documentation and comments #85377

Merged
merged 7 commits into from
Jul 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -717,8 +717,14 @@ extern "rust-intrinsic" {
/// Therefore, implementations must not require the user to uphold
/// any safety invariants.
///
/// A more user-friendly and stable version of this operation is
/// [`std::process::abort`](../../std/process/fn.abort.html).
/// [`std::process::abort`](../../std/process/fn.abort.html) is to be preferred if possible,
/// as its behavior is more user-friendly and more stable.
///
/// The current implementation of `intrinsics::abort` is to invoke an invalid instruction,
/// on most platforms.
/// On Unix, the
/// process will probably terminate with a signal like `SIGABRT`, `SIGILL`, `SIGTRAP`, `SIGSEGV` or
/// `SIGBUS`. The precise behaviour is not guaranteed and not stable.
pub fn abort() -> !;

/// Informs the optimizer that this point in the code is not reachable,
Expand Down
7 changes: 7 additions & 0 deletions library/std/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1898,6 +1898,9 @@ pub fn exit(code: i32) -> ! {
/// process, no destructors on the current stack or any other thread's stack
/// will be run.
///
/// Rust IO buffers (eg, from `BufWriter`) will not be flushed.
/// Likewise, C stdio buffers will (on most platforms) not be flushed.
///
/// This is in contrast to the default behaviour of [`panic!`] which unwinds
/// the current thread's stack and calls all destructors.
/// When `panic="abort"` is set, either as an argument to `rustc` or in a
Expand All @@ -1908,6 +1911,10 @@ pub fn exit(code: i32) -> ! {
/// this function at a known point where there are no more destructors left
/// to run.
///
/// The process's termination will be similar to that from the C `abort()`
/// function. On Unix, the process will terminate with signal `SIGABRT`, which
/// typically means that the shell prints "Aborted".
///
/// # Examples
///
/// ```no_run
Expand Down
42 changes: 35 additions & 7 deletions library/std/src/sys/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,41 @@ pub fn cvt_nz(error: libc::c_int) -> crate::io::Result<()> {
if error == 0 { Ok(()) } else { Err(crate::io::Error::from_raw_os_error(error)) }
}

// 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 buffered
// 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.
// libc::abort() will run the SIGABRT handler. That's fine because anyone who
m-ou-se marked this conversation as resolved.
Show resolved Hide resolved
// installs a SIGABRT handler already has to expect it to run in Very Bad
// situations (eg, malloc crashing).
//
// Current glibc's abort() function unblocks SIGABRT, raises SIGABRT, clears the
// SIGABRT handler and raises it again, and then starts to get creative.
//
// See the public documentation for `intrinsics::abort()` and `process::abort()`
// for further discussion.
//
// There is confusion about whether libc::abort() flushes stdio streams.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps mention it is implementation-defined as a first point (C18 7.22.4.1p1).

// libc::abort() is required by ISO C 99 (7.14.1.1p5) to be async-signal-safe,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to point to C18 (it is still 7.14.1.1p5).

// so flushing streams is at least extremely hard, if not entirely impossible.
//
// However, some versions of POSIX (eg IEEE Std 1003.1-2001) required abort to
// do so. In 1003.1-2004 this was fixed.
//
// glibc's implementation did the flush, unsafely, before glibc commit
// 91e7cf982d01 `abort: Do not flush stdio streams [BZ #15436]' by Florian
// Weimer. According to glibc's NEWS:
//
// The abort function terminates the process immediately, without flushing
// stdio streams. Previous glibc versions used to flush streams, resulting
// in deadlocks and further data corruption. This change also affects
// process aborts as the result of assertion failures.
//
// This is an accurate description of the problem. The only solution for
// program with nontrivial use of C stdio is a fixed libc - one which does not
// try to flush in abort - since even libc-internal errors, and assertion
// failures generated from C, will go via abort().
//
// On systems with old, buggy, libcs, the impact can be severe for a
// multithreaded C program. It is much less severe for Rust, because Rust
// stdlib doesn't use libc stdio buffering. In a typical Rust program, which
// does not use C stdio, even a buggy libc::abort() is, in fact, safe.
ijackson marked this conversation as resolved.
Show resolved Hide resolved
pub fn abort_internal() -> ! {
unsafe { libc::abort() }
}
Expand Down