Rollup merge of #85377 - ijackson:abort-docs, r=m-ou-se

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

Fixes #40230

3 files changed, 50 insertions, 9 deletions

diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs
index b4311bbe5f4..c5a4bbd3208 100644
--- a/library/core/src/intrinsics.rs
+++ b/library/core/src/intrinsics.rs
@@ -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,
diff --git a/library/std/src/process.rs b/library/std/src/process.rs
index b46d3dfc1e7..11a0432ce27 100644
--- a/library/std/src/process.rs
+++ b/library/std/src/process.rs
@@ -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
@@ -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
diff --git a/library/std/src/sys/unix/mod.rs b/library/std/src/sys/unix/mod.rs
index f3535b27128..2da71b2a448 100644
--- a/library/std/src/sys/unix/mod.rs
+++ b/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() }
 }