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 (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 <mdw@distorted.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

Author: 2 people

library/core/src/intrinsics.rs

@@ -717,8 +717,12 @@ 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 behaviour is more user-friendly and more stable.
+    ///
+    /// The current implementation of `intrinsics::abort` (ab)uses a debug trap.  On Unix, the
+    /// process will probably die of 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,

library/std/src/process.rs

@@ -1908,6 +1908,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 die with signal `SIGABRT`, which
+/// typically means that the shell prints "Aborted".
+///
 /// # Examples
 ///
 /// ```no_run

library/std/src/sys/unix/mod.rs

@@ -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() }
 }