Auto merge of #70212 - Amanieu:catch_foreign, r=Mark-Simulacrum

Abort when foreign exceptions are caught by catch_unwind

Prior to this PR, foreign exceptions were not caught by catch_unwind, and instead passed through invisibly. This represented a painful soundness hole in some libraries ([take_mut](https://github.com/Sgeo/take_mut/blob/master/src/lib.rs#L37)), which relied on `catch_unwind` to handle all possible exit paths from a closure.

With this PR, foreign exceptions are now caught by `catch_unwind` and will trigger an abort since catching foreign exceptions is currently UB according to the latest proposals by the FFI unwind project group.

cc @rust-lang/wg-ffi-unwind

Author: bors

library/panic_abort/src/lib.rs

@@ -117,6 +117,17 @@ pub mod personalities {
         1 // `ExceptionContinueSearch`
     }
 
+    // Similar to above, this corresponds to the `eh_catch_typeinfo` lang item
+    // that's only used on Emscripten currently.
+    //
+    // Since panics don't generate exceptions and foreign exceptions are
+    // currently UB with -C panic=abort (although this may be subject to
+    // change), any catch_unwind calls will never use this typeinfo.
+    #[rustc_std_internal_symbol]
+    #[allow(non_upper_case_globals)]
+    #[cfg(target_os = "emscripten")]
+    static rust_eh_catch_typeinfo: [usize; 2] = [0; 2];
+
     // These two are called by our startup objects on i686-pc-windows-gnu, but
     // they don't need to do anything so the bodies are nops.
     #[rustc_std_internal_symbol]

library/panic_unwind/src/dwarf/eh.rs

@@ -51,11 +51,7 @@ pub enum EHAction {
 
 pub const USING_SJLJ_EXCEPTIONS: bool = cfg!(all(target_os = "ios", target_arch = "arm"));
 
-pub unsafe fn find_eh_action(
-    lsda: *const u8,
-    context: &EHContext<'_>,
-    foreign_exception: bool,
-) -> Result<EHAction, ()> {
+pub unsafe fn find_eh_action(lsda: *const u8, context: &EHContext<'_>) -> Result<EHAction, ()> {
     if lsda.is_null() {
         return Ok(EHAction::None);
     }
@@ -98,7 +94,7 @@ pub unsafe fn find_eh_action(
                     return Ok(EHAction::None);
                 } else {
                     let lpad = lpad_base + cs_lpad;
-                    return Ok(interpret_cs_action(cs_action, lpad, foreign_exception));
+                    return Ok(interpret_cs_action(cs_action, lpad));
                 }
             }
         }
@@ -123,21 +119,17 @@ pub unsafe fn find_eh_action(
                 // Can never have null landing pad for sjlj -- that would have
                 // been indicated by a -1 call site index.
                 let lpad = (cs_lpad + 1) as usize;
-                return Ok(interpret_cs_action(cs_action, lpad, foreign_exception));
+                return Ok(interpret_cs_action(cs_action, lpad));
             }
         }
     }
 }
 
-fn interpret_cs_action(cs_action: u64, lpad: usize, foreign_exception: bool) -> EHAction {
+fn interpret_cs_action(cs_action: u64, lpad: usize) -> EHAction {
     if cs_action == 0 {
         // If cs_action is 0 then this is a cleanup (Drop::drop). We run these
         // for both Rust panics and foreign exceptions.
         EHAction::Cleanup(lpad)
-    } else if foreign_exception {
-        // catch_unwind should not catch foreign exceptions, only Rust panics.
-        // Instead just continue unwinding.
-        EHAction::None
     } else {
         // Stop unwinding Rust panics at catch_unwind.
         EHAction::Catch(lpad)

library/panic_unwind/src/emcc.rs

@@ -8,8 +8,10 @@
 
 use alloc::boxed::Box;
 use core::any::Any;
+use core::intrinsics;
 use core::mem;
 use core::ptr;
+use core::sync::atomic::{AtomicBool, Ordering};
 use libc::{self, c_int};
 use unwind as uw;
 
@@ -47,6 +49,11 @@ static EXCEPTION_TYPE_INFO: TypeInfo = TypeInfo {
 };
 
 struct Exception {
+    // This is necessary because C++ code can capture our execption with
+    // std::exception_ptr and rethrow it multiple times, possibly even in
+    // another thread.
+    caught: AtomicBool,
+
     // This needs to be an Option because the object's lifetime follows C++
     // semantics: when catch_unwind moves the Box out of the exception it must
     // still leave the exception object in a valid state because its destructor
@@ -55,11 +62,27 @@ struct Exception {
 }
 
 pub unsafe fn cleanup(ptr: *mut u8) -> Box<dyn Any + Send> {
-    assert!(!ptr.is_null());
-    let adjusted_ptr = __cxa_begin_catch(ptr as *mut libc::c_void) as *mut Exception;
-    let ex = (*adjusted_ptr).data.take();
+    // intrinsics::try actually gives us a pointer to this structure.
+    #[repr(C)]
+    struct CatchData {
+        ptr: *mut u8,
+        is_rust_panic: bool,
+    }
+    let catch_data = &*(ptr as *mut CatchData);
+
+    let adjusted_ptr = __cxa_begin_catch(catch_data.ptr as *mut libc::c_void) as *mut Exception;
+    let out = if catch_data.is_rust_panic {
+        let was_caught = (*adjusted_ptr).caught.swap(true, Ordering::SeqCst);
+        if was_caught {
+            // Since cleanup() isn't allowed to panic, we just abort instead.
+            intrinsics::abort();
+        }
+        (*adjusted_ptr).data.take().unwrap()
+    } else {
+        super::__rust_foreign_exception();
+    };
     __cxa_end_catch();
-    ex.unwrap()
+    out
 }
 
 pub unsafe fn panic(data: Box<dyn Any + Send>) -> u32 {
@@ -68,25 +91,16 @@ pub unsafe fn panic(data: Box<dyn Any + Send>) -> u32 {
     if exception.is_null() {
         return uw::_URC_FATAL_PHASE1_ERROR as u32;
     }
-    ptr::write(exception, Exception { data: Some(data) });
+    ptr::write(exception, Exception { caught: AtomicBool::new(false), data: Some(data) });
     __cxa_throw(exception as *mut _, &EXCEPTION_TYPE_INFO, exception_cleanup);
 }
 
-// On WASM and ARM, the destructor returns the pointer to the object.
-cfg_if::cfg_if! {
-    if #[cfg(any(target_arch = "arm", target_arch = "wasm32"))] {
-        type DestructorRet = *mut libc::c_void;
-    } else {
-        type DestructorRet = ();
-    }
-}
-extern "C" fn exception_cleanup(ptr: *mut libc::c_void) -> DestructorRet {
+extern "C" fn exception_cleanup(ptr: *mut libc::c_void) -> *mut libc::c_void {
     unsafe {
         if let Some(b) = (ptr as *mut Exception).read().data {
             drop(b);
             super::__rust_drop_panic();
         }
-        #[cfg(any(target_arch = "arm", target_arch = "wasm32"))]
         ptr
     }
 }
@@ -109,7 +123,7 @@ extern "C" {
     fn __cxa_throw(
         thrown_exception: *mut libc::c_void,
         tinfo: *const TypeInfo,
-        dest: extern "C" fn(*mut libc::c_void) -> DestructorRet,
+        dest: extern "C" fn(*mut libc::c_void) -> *mut libc::c_void,
     ) -> !;
     fn __gxx_personality_v0(
         version: c_int,

library/panic_unwind/src/gcc.rs

@@ -73,8 +73,14 @@ pub unsafe fn panic(data: Box<dyn Any + Send>) -> u32 {
 }
 
 pub unsafe fn cleanup(ptr: *mut u8) -> Box<dyn Any + Send> {
-    let exception = Box::from_raw(ptr as *mut Exception);
-    exception.cause
+    let exception = ptr as *mut uw::_Unwind_Exception;
+    if (*exception).exception_class != rust_exception_class() {
+        uw::_Unwind_DeleteException(exception);
+        super::__rust_foreign_exception();
+    } else {
+        let exception = Box::from_raw(exception as *mut Exception);
+        exception.cause
+    }
 }
 
 // Rust's exception class identifier.  This is used by personality routines to
@@ -164,9 +170,7 @@ cfg_if::cfg_if! {
             // _Unwind_Context in our libunwind bindings and fetch the required data from there
             // directly, bypassing DWARF compatibility functions.
 
-            let exception_class = (*exception_object).exception_class;
-            let foreign_exception = exception_class != rust_exception_class();
-            let eh_action = match find_eh_action(context, foreign_exception) {
+            let eh_action = match find_eh_action(context) {
                 Ok(action) => action,
                 Err(_) => return uw::_URC_FAILURE,
             };
@@ -221,15 +225,14 @@ cfg_if::cfg_if! {
         // and indirectly on Windows x86_64 via SEH.
         unsafe extern "C" fn rust_eh_personality_impl(version: c_int,
                                                       actions: uw::_Unwind_Action,
-                                                      exception_class: uw::_Unwind_Exception_Class,
+                                                      _exception_class: uw::_Unwind_Exception_Class,
                                                       exception_object: *mut uw::_Unwind_Exception,
                                                       context: *mut uw::_Unwind_Context)
                                                       -> uw::_Unwind_Reason_Code {
             if version != 1 {
                 return uw::_URC_FATAL_PHASE1_ERROR;
             }
-            let foreign_exception = exception_class != rust_exception_class();
-            let eh_action = match find_eh_action(context, foreign_exception) {
+            let eh_action = match find_eh_action(context) {
                 Ok(action) => action,
                 Err(_) => return uw::_URC_FATAL_PHASE1_ERROR,
             };
@@ -293,10 +296,7 @@ cfg_if::cfg_if! {
     }
 }
 
-unsafe fn find_eh_action(
-    context: *mut uw::_Unwind_Context,
-    foreign_exception: bool,
-) -> Result<EHAction, ()> {
+unsafe fn find_eh_action(context: *mut uw::_Unwind_Context) -> Result<EHAction, ()> {
     let lsda = uw::_Unwind_GetLanguageSpecificData(context) as *const u8;
     let mut ip_before_instr: c_int = 0;
     let ip = uw::_Unwind_GetIPInfo(context, &mut ip_before_instr);
@@ -308,7 +308,7 @@ unsafe fn find_eh_action(
         get_text_start: &|| uw::_Unwind_GetTextRelBase(context),
         get_data_start: &|| uw::_Unwind_GetDataRelBase(context),
     };
-    eh::find_eh_action(lsda, &eh_context, foreign_exception)
+    eh::find_eh_action(lsda, &eh_context)
 }
 
 // Frame unwind info registration

library/panic_unwind/src/lib.rs

@@ -88,6 +88,9 @@ extern "C" {
     /// Handler in libstd called when a panic object is dropped outside of
     /// `catch_unwind`.
     fn __rust_drop_panic() -> !;
+
+    /// Handler in libstd called when a foreign exception is caught.
+    fn __rust_foreign_exception() -> !;
 }
 
 mod dwarf;

library/panic_unwind/src/seh.rs

@@ -309,15 +309,21 @@ pub unsafe fn panic(data: Box<dyn Any + Send>) -> u32 {
 
     extern "system" {
         #[unwind(allowed)]
-        pub fn _CxxThrowException(pExceptionObject: *mut c_void, pThrowInfo: *mut u8) -> !;
+        fn _CxxThrowException(pExceptionObject: *mut c_void, pThrowInfo: *mut u8) -> !;
     }
 
     _CxxThrowException(throw_ptr, &mut THROW_INFO as *mut _ as *mut _);
 }
 
 pub unsafe fn cleanup(payload: *mut u8) -> Box<dyn Any + Send> {
-    let exception = &mut *(payload as *mut Exception);
-    exception.data.take().unwrap()
+    // A NULL payload here means that we got here from the catch (...) of
+    // __rust_try. This happens when a non-Rust foreign exception is caught.
+    if payload.is_null() {
+        super::__rust_foreign_exception();
+    } else {
+        let exception = &mut *(payload as *mut Exception);
+        exception.data.take().unwrap()
+    }
 }
 
 // This is required by the compiler to exist (e.g., it's a lang item), but

library/std/src/panic.rs

@@ -359,6 +359,9 @@ impl<F: Future> Future for AssertUnwindSafe<F> {
 /// aborting the process as well. This function *only* catches unwinding panics,
 /// not those that abort the process.
 ///
+/// Also note that unwinding into Rust code with a foreign exception (e.g. a
+/// an exception thrown from C++ code) is undefined behavior.
+///
 /// # Examples
 ///
 /// ```

library/std/src/panicking.rs

@@ -60,6 +60,14 @@ extern "C" fn __rust_drop_panic() -> ! {
     rtabort!("Rust panics must be rethrown");
 }
 
+/// This function is called by the panic runtime if it catches an exception
+/// object which does not correspond to a Rust panic.
+#[cfg(not(test))]
+#[rustc_std_internal_symbol]
+extern "C" fn __rust_foreign_exception() -> ! {
+    rtabort!("Rust cannot catch foreign exceptions");
+}
+
 #[derive(Copy, Clone)]
 enum Hook {
     Default,

src/librustc_codegen_llvm/context.rs

@@ -82,6 +82,7 @@ pub struct CodegenCx<'ll, 'tcx> {
     pub dbg_cx: Option<debuginfo::CrateDebugContext<'ll, 'tcx>>,
 
     eh_personality: Cell<Option<&'ll Value>>,
+    eh_catch_typeinfo: Cell<Option<&'ll Value>>,
     pub rust_try_fn: Cell<Option<&'ll Value>>,
 
     intrinsics: RefCell<FxHashMap<&'static str, &'ll Value>>,
@@ -311,6 +312,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
             coverage_cx,
             dbg_cx,
             eh_personality: Cell::new(None),
+            eh_catch_typeinfo: Cell::new(None),
             rust_try_fn: Cell::new(None),
             intrinsics: Default::default(),
             local_gen_sym_counter: Cell::new(0),
@@ -819,6 +821,25 @@ impl CodegenCx<'b, 'tcx> {
         }
         None
     }
+
+    crate fn eh_catch_typeinfo(&self) -> &'b Value {
+        if let Some(eh_catch_typeinfo) = self.eh_catch_typeinfo.get() {
+            return eh_catch_typeinfo;
+        }
+        let tcx = self.tcx;
+        assert!(self.sess().target.target.options.is_like_emscripten);
+        let eh_catch_typeinfo = match tcx.lang_items().eh_catch_typeinfo() {
+            Some(def_id) => self.get_static(def_id),
+            _ => {
+                let ty = self
+                    .type_struct(&[self.type_ptr_to(self.type_isize()), self.type_i8p()], false);
+                self.declare_global("rust_eh_catch_typeinfo", ty)
+            }
+        };
+        let eh_catch_typeinfo = self.const_bitcast(eh_catch_typeinfo, self.type_i8p());
+        self.eh_catch_typeinfo.set(Some(eh_catch_typeinfo));
+        eh_catch_typeinfo
+    }
 }
 
 impl<'b, 'tcx> CodegenCx<'b, 'tcx> {