Skip to content

Commit

Permalink
Rollup merge of #85377 - ijackson:abort-docs, r=m-ou-se
Browse files Browse the repository at this point in the history
aborts: Clarify documentation and comments

In the docs for intrinsics::abort():

 * Strengthen the recommendation by to use process::abort instead.
 * Document the fact that it sometimes (ab)uses an LLVM debug trap and what the likely consequences are.
 * State that the precise behaviour is unstable.

In the docs for process::abort():

 * Promise that we have the same behaviour as C `abort()`.
 * Document the likely consequences, including, specifically, the consequences on Unix.

In the internal comment for unix::abort_internal:

 * Refer to the public docs for the public API functions.
 * Correct and expand the description of libc::abort.  Specifically:
 * Do not claim that abort() unregisters signal handlers.  It doesn't; it honours the SIGABRT handler.
 * Discuss, extensively, the issue with abort() flushing stdio buffers.
 * Describe the glibc behaviour in some detail.

Co-authored-by: Mark Wooding <[email protected]>
Signed-off-by: Ian Jackson <[email protected]>

Fixes #40230
  • Loading branch information
JohnTitor authored Jul 5, 2021
2 parents 1fcd9ab + 08d912f commit add24d2
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 9 deletions.
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
// 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.
// libc::abort() is required by ISO C 99 (7.14.1.1p5) to be async-signal-safe,
// 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.
pub fn abort_internal() -> ! {
unsafe { libc::abort() }
}
Expand Down

0 comments on commit add24d2

Please sign in to comment.