Auto merge of rust-lang#128321 - BatmanAoD:catch-unwind-doc-update, r=Mark-Simulacrum

bors · bors · commit b4307a531056 · 2024-09-29T05:54:47.000Z
Update `catch_unwind` doc comments for `c_unwind` Updates `catch_unwind` doc comments to indicate that catching a foreign exception _will no longer_ be UB. Instead, there are two possible behaviors, though it is not specified which one an implementation will choose. Nominated for t-lang to confirm that they are okay with making such a promise based on t-opsem FCP, or whether they would like to be included in the FCP. Related: rust-lang#74990, rust-lang#115285, rust-lang/reference#1226
diff --git a/core/src/intrinsics.rs b/core/src/intrinsics.rs
@@ -2659,12 +2659,17 @@ extern "rust-intrinsic" {
     ///
     /// `catch_fn` must not unwind.
     ///
-    /// The third argument is a function called if an unwind occurs (both Rust unwinds and foreign
-    /// unwinds). This function takes the data pointer and a pointer to the target-specific
-    /// exception object that was caught. For more information, see the compiler's source as well as
-    /// std's `catch_unwind` implementation.
+    /// The third argument is a function called if an unwind occurs (both Rust `panic` and foreign
+    /// unwinds). This function takes the data pointer and a pointer to the target- and
+    /// runtime-specific exception object that was caught.
     ///
-    /// The stable version of this intrinsic is `std::panic::catch_unwind`.
+    /// Note that in the case of a foreign unwinding operation, the exception object data may not be
+    /// safely usable from Rust, and should not be directly exposed via the standard library. To
+    /// prevent unsafe access, the library implementation may either abort the process or present an
+    /// opaque error type to the user.
+    ///
+    /// For more information, see the compiler's source, as well as the documentation for the stable
+    /// version of this intrinsic, `std::panic::catch_unwind`.
     #[rustc_nounwind]
     pub fn catch_unwind(try_fn: fn(*mut u8), data: *mut u8, catch_fn: fn(*mut u8, *mut u8)) -> i32;
 
diff --git a/std/src/panic.rs b/std/src/panic.rs
@@ -288,45 +288,55 @@ pub use core::panic::abort_unwind;
 
 /// Invokes a closure, capturing the cause of an unwinding panic if one occurs.
 ///
-/// This function will return `Ok` with the closure's result if the closure
-/// does not panic, and will return `Err(cause)` if the closure panics. The
-/// `cause` returned is the object with which panic was originally invoked.
+/// This function will return `Ok` with the closure's result if the closure does
+/// not panic, and will return `Err(cause)` if the closure panics. The `cause`
+/// returned is the object with which panic was originally invoked.
 ///
-/// It is currently undefined behavior to unwind from Rust code into foreign
-/// code, so this function is particularly useful when Rust is called from
-/// another language (normally C). This can run arbitrary Rust code, capturing a
-/// panic and allowing a graceful handling of the error.
+/// Rust functions that are expected to be called from foreign code that does
+/// not support unwinding (such as C compiled with `-fno-exceptions`) should be
+/// defined using `extern "C"`, which ensures that if the Rust code panics, it
+/// is automatically caught and the process is aborted. If this is the desired
+/// behavior, it is not necessary to use `catch_unwind` explicitly. This
+/// function should instead be used when more graceful error-handling is needed.
 ///
 /// It is **not** recommended to use this function for a general try/catch
 /// mechanism. The [`Result`] type is more appropriate to use for functions that
 /// can fail on a regular basis. Additionally, this function is not guaranteed
 /// to catch all panics, see the "Notes" section below.
 ///
-/// The closure provided is required to adhere to the [`UnwindSafe`] trait to ensure
-/// that all captured variables are safe to cross this boundary. The purpose of
-/// this bound is to encode the concept of [exception safety][rfc] in the type
-/// system. Most usage of this function should not need to worry about this
-/// bound as programs are naturally unwind safe without `unsafe` code. If it
-/// becomes a problem the [`AssertUnwindSafe`] wrapper struct can be used to quickly
-/// assert that the usage here is indeed unwind safe.
+/// The closure provided is required to adhere to the [`UnwindSafe`] trait to
+/// ensure that all captured variables are safe to cross this boundary. The
+/// purpose of this bound is to encode the concept of [exception safety][rfc] in
+/// the type system. Most usage of this function should not need to worry about
+/// this bound as programs are naturally unwind safe without `unsafe` code. If
+/// it becomes a problem the [`AssertUnwindSafe`] wrapper struct can be used to
+/// quickly assert that the usage here is indeed unwind safe.
 ///
 /// [rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1236-stabilize-catch-panic.md
 ///
 /// # Notes
 ///
-/// Note that this function **might not catch all panics** in Rust. A panic in
-/// Rust is not always implemented via unwinding, but can be implemented by
-/// aborting the process as well. This function *only* catches unwinding panics,
-/// not those that abort the process.
+/// This function **might not catch all Rust panics**. A Rust panic is not
+/// always implemented via unwinding, but can be implemented by aborting the
+/// process as well. This function *only* catches unwinding panics, not those
+/// that abort the process.
 ///
-/// Note that if a custom panic hook has been set, it will be invoked before
-/// the panic is caught, before unwinding.
+/// If a custom panic hook has been set, it will be invoked before the panic is
+/// caught, before unwinding.
 ///
-/// Also note that unwinding into Rust code with a foreign exception (e.g.
-/// an exception thrown from C++ code) is undefined behavior.
+/// Although unwinding into Rust code with a foreign exception (e.g. an
+/// exception thrown from C++ code, or a `panic!` in Rust code compiled or
+/// linked with a different runtime) via an appropriate ABI (e.g. `"C-unwind"`)
+/// is permitted, catching such an exception using this function will have one
+/// of two behaviors, and it is unspecified which will occur:
 ///
-/// Finally, be **careful in how you drop the result of this function**.
-/// If it is `Err`, it contains the panic payload, and dropping that may in turn panic!
+/// * The process aborts, after executing all destructors of `f` and the
+///   functions it called.
+/// * The function returns a `Result::Err` containing an opaque type.
+///
+/// Finally, be **careful in how you drop the result of this function**. If it
+/// is `Err`, it contains the panic payload, and dropping that may in turn
+/// panic!
 ///
 /// # Examples
 ///
diff --git a/std/src/thread/mod.rs b/std/src/thread/mod.rs
@@ -664,6 +664,19 @@ impl Builder {
 /// println!("{result}");
 /// ```
 ///
+/// # Notes
+///
+/// This function has the same minimal guarantee regarding "foreign" unwinding operations (e.g.
+/// an exception thrown from C++ code, or a `panic!` in Rust code compiled or linked with a
+/// different runtime) as [`catch_unwind`]; namely, if the thread created with `thread::spawn`
+/// unwinds all the way to the root with such an exception, one of two behaviors are possible,
+/// and it is unspecified which will occur:
+///
+/// * The process aborts.
+/// * The process does not abort, and [`join`] will return a `Result::Err`
+///   containing an opaque type.
+///
+/// [`catch_unwind`]: ../../std/panic/fn.catch_unwind.html
 /// [`channels`]: crate::sync::mpsc
 /// [`join`]: JoinHandle::join
 /// [`Err`]: crate::result::Result::Err
@@ -1784,7 +1797,7 @@ impl<T> JoinHandle<T> {
     /// operations that happen after `join` returns.
     ///
     /// If the associated thread panics, [`Err`] is returned with the parameter given
-    /// to [`panic!`].
+    /// to [`panic!`] (though see the Notes below).
     ///
     /// [`Err`]: crate::result::Result::Err
     /// [atomic memory orderings]: crate::sync::atomic
@@ -1806,6 +1819,18 @@ impl<T> JoinHandle<T> {
     /// }).unwrap();
     /// join_handle.join().expect("Couldn't join on the associated thread");
     /// ```
+    ///
+    /// # Notes
+    ///
+    /// If a "foreign" unwinding operation (e.g. an exception thrown from C++
+    /// code, or a `panic!` in Rust code compiled or linked with a different
+    /// runtime) unwinds all the way to the thread root, the process may be
+    /// aborted; see the Notes on [`thread::spawn`]. If the process is not
+    /// aborted, this function will return a `Result::Err` containing an opaque
+    /// type.
+    ///
+    /// [`catch_unwind`]: ../../std/panic/fn.catch_unwind.html
+    /// [`thread::spawn`]: spawn
     #[stable(feature = "rust1", since = "1.0.0")]
     pub fn join(self) -> Result<T> {
         self.0.join()
