Skip to content

Commit

Permalink
Rollup merge of rust-lang#102721 - nbdd0121:panic, r=Amanieu
Browse files Browse the repository at this point in the history
Prevent foreign Rust exceptions from being caught

Fix rust-lang#102715

Use the address of a static variable (which is guaranteed to be unique per copy of std) to tell apart if a Rust exception comes from local or foreign Rust code, and abort for the latter.
  • Loading branch information
matthiaskrgr authored Oct 11, 2022
2 parents 4ccd2f4 + c33cef6 commit 591eab0
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 18 deletions.
37 changes: 27 additions & 10 deletions library/panic_unwind/src/emcc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ static EXCEPTION_TYPE_INFO: TypeInfo = TypeInfo {
name: b"rust_panic\0".as_ptr(),
};

// NOTE(nbdd0121): The `canary` field will be part of stable ABI after `c_unwind` stabilization.
#[repr(C)]
struct Exception {
// See `gcc.rs` on why this is present. We already have a static here so just use it.
canary: *const TypeInfo,

// This is necessary because C++ code can capture our exception with
// std::exception_ptr and rethrow it multiple times, possibly even in
// another thread.
Expand All @@ -70,16 +75,21 @@ pub unsafe fn cleanup(ptr: *mut u8) -> Box<dyn Any + Send> {
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 {
if !catch_data.is_rust_panic {
super::__rust_foreign_exception();
};
}

let canary = ptr::addr_of!((*adjusted_ptr).canary).read();
if !ptr::eq(canary, &EXCEPTION_TYPE_INFO) {
super::__rust_foreign_exception();
}

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();
}
let out = (*adjusted_ptr).data.take().unwrap();
__cxa_end_catch();
out
}
Expand All @@ -90,7 +100,14 @@ 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 { caught: AtomicBool::new(false), data: Some(data) });
ptr::write(
exception,
Exception {
canary: &EXCEPTION_TYPE_INFO,
caught: AtomicBool::new(false),
data: Some(data),
},
);
__cxa_throw(exception as *mut _, &EXCEPTION_TYPE_INFO, exception_cleanup);
}

Expand Down
30 changes: 27 additions & 3 deletions library/panic_unwind/src/gcc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,23 @@

use alloc::boxed::Box;
use core::any::Any;
use core::ptr;

use unwind as uw;

// In case where multiple copies of std exist in a single process,
// we use address of this static variable to distinguish an exception raised by
// this copy and some other copy (which needs to be treated as foreign exception).
static CANARY: u8 = 0;

// NOTE(nbdd0121)
// Once `c_unwind` feature is stabilized, there will be ABI stability requirement
// on this struct. The first two field must be `_Unwind_Exception` and `canary`,
// as it may be accessed by a different version of the std with a different compiler.
#[repr(C)]
struct Exception {
_uwe: uw::_Unwind_Exception,
canary: *const u8,
cause: Box<dyn Any + Send>,
}

Expand All @@ -54,6 +65,7 @@ pub unsafe fn panic(data: Box<dyn Any + Send>) -> u32 {
exception_cleanup,
private: [0; uw::unwinder_private_data_size],
},
canary: &CANARY,
cause: data,
});
let exception_param = Box::into_raw(exception) as *mut uw::_Unwind_Exception;
Expand All @@ -75,10 +87,22 @@ pub unsafe fn cleanup(ptr: *mut u8) -> Box<dyn Any + Send> {
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
}

let exception = exception.cast::<Exception>();
// Just access the canary field, avoid accessing the entire `Exception` as
// it can be a foreign Rust exception.
let canary = ptr::addr_of!((*exception).canary).read();
if !ptr::eq(canary, &CANARY) {
// A foreign Rust exception, treat it slightly differently from other
// foreign exceptions, because call into `_Unwind_DeleteException` will
// call into `__rust_drop_panic` which produces a confusing
// "Rust panic must be rethrown" message.
super::__rust_foreign_exception();
}

let exception = Box::from_raw(exception as *mut Exception);
exception.cause
}

// Rust's exception class identifier. This is used by personality routines to
Expand Down
20 changes: 15 additions & 5 deletions library/panic_unwind/src/seh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,15 @@
use alloc::boxed::Box;
use core::any::Any;
use core::mem::{self, ManuallyDrop};
use core::ptr;
use libc::{c_int, c_uint, c_void};

// NOTE(nbdd0121): The `canary` field will be part of stable ABI after `c_unwind` stabilization.
#[repr(C)]
struct Exception {
// See `gcc.rs` on why this is present. We already have a static here so just use it.
canary: *const _TypeDescriptor,

// This needs to be an Option because we catch the exception by reference
// and its destructor is executed by the C++ runtime. When we take the Box
// out of the exception, we need to leave the exception in a valid state
Expand Down Expand Up @@ -235,7 +241,7 @@ static mut TYPE_DESCRIPTOR: _TypeDescriptor = _TypeDescriptor {
macro_rules! define_cleanup {
($abi:tt $abi2:tt) => {
unsafe extern $abi fn exception_cleanup(e: *mut Exception) {
if let Exception { data: Some(b) } = e.read() {
if let Exception { data: Some(b), .. } = e.read() {
drop(b);
super::__rust_drop_panic();
}
Expand Down Expand Up @@ -265,7 +271,7 @@ pub unsafe fn panic(data: Box<dyn Any + Send>) -> u32 {
// The ManuallyDrop is needed here since we don't want Exception to be
// dropped when unwinding. Instead it will be dropped by exception_cleanup
// which is invoked by the C++ runtime.
let mut exception = ManuallyDrop::new(Exception { data: Some(data) });
let mut exception = ManuallyDrop::new(Exception { canary: &TYPE_DESCRIPTOR, data: Some(data) });
let throw_ptr = &mut exception as *mut _ as *mut _;

// This... may seems surprising, and justifiably so. On 32-bit MSVC the
Expand Down Expand Up @@ -321,8 +327,12 @@ pub unsafe fn cleanup(payload: *mut u8) -> Box<dyn Any + Send> {
// __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()
}
let exception = payload as *mut Exception;
let canary = ptr::addr_of!((*exception).canary).read();
if !ptr::eq(canary, &TYPE_DESCRIPTOR) {
// A foreign Rust exception.
super::__rust_foreign_exception();
}
(*exception).data.take().unwrap()
}
6 changes: 6 additions & 0 deletions src/test/run-make-fulldeps/foreign-rust-exceptions/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
include ../tools.mk

all:
$(RUSTC) bar.rs --crate-type=cdylib
$(RUSTC) foo.rs
$(call RUN,foo) 2>&1 | $(CGREP) "Rust cannot catch foreign exceptions"
7 changes: 7 additions & 0 deletions src/test/run-make-fulldeps/foreign-rust-exceptions/bar.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#![crate_type = "cdylib"]
#![feature(c_unwind)]

#[no_mangle]
extern "C-unwind" fn panic() {
panic!();
}
12 changes: 12 additions & 0 deletions src/test/run-make-fulldeps/foreign-rust-exceptions/foo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#![feature(c_unwind)]

#[link(name = "bar")]
extern "C-unwind" {
fn panic();
}

fn main() {
let _ = std::panic::catch_unwind(|| {
unsafe { panic() };
});
}

0 comments on commit 591eab0

Please sign in to comment.