From 10720b418ecc72709adddba1b26e2cb293558e1b Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Thu, 26 Dec 2019 10:46:32 -0500 Subject: [PATCH 01/11] Fix a memory leak in emcc if a Rust panic is caught by C++ and discarded --- src/libpanic_unwind/emcc.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/libpanic_unwind/emcc.rs b/src/libpanic_unwind/emcc.rs index 9d3fe5254f8a9..61f33fd4d5d66 100644 --- a/src/libpanic_unwind/emcc.rs +++ b/src/libpanic_unwind/emcc.rs @@ -52,22 +52,32 @@ pub fn payload() -> *mut u8 { ptr::null_mut() } +struct Exception { + data: Option>, +} + pub unsafe fn cleanup(ptr: *mut u8) -> Box { assert!(!ptr.is_null()); - let adjusted_ptr = __cxa_begin_catch(ptr as *mut libc::c_void); - let ex = ptr::read(adjusted_ptr as *mut _); + let adjusted_ptr = __cxa_begin_catch(ptr as *mut libc::c_void) as *mut Exception; + let ex = (*adjusted_ptr).data.take(); __cxa_end_catch(); - ex + ex.unwrap() } pub unsafe fn panic(data: Box) -> u32 { let sz = mem::size_of_val(&data); - let exception = __cxa_allocate_exception(sz); + let exception = __cxa_allocate_exception(sz) as *mut Exception; if exception.is_null() { return uw::_URC_FATAL_PHASE1_ERROR as u32; } - ptr::write(exception as *mut _, data); - __cxa_throw(exception as *mut _, &EXCEPTION_TYPE_INFO, ptr::null_mut()); + ptr::write(exception, Exception { data: Some(data) }); + __cxa_throw(exception as *mut _, &EXCEPTION_TYPE_INFO, exception_cleanup); + + extern "C" fn exception_cleanup(ptr: *mut libc::c_void) { + unsafe { + ptr::drop_in_place(ptr as *mut Exception); + } + } } #[lang = "eh_personality"] @@ -89,7 +99,7 @@ extern "C" { fn __cxa_throw( thrown_exception: *mut libc::c_void, tinfo: *const TypeInfo, - dest: *mut libc::c_void, + dest: extern "C" fn(*mut libc::c_void), ) -> !; fn __gxx_personality_v0( version: c_int, From 46f52260d89517bcb1c49b189dfb54645776e8c3 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Thu, 26 Dec 2019 10:26:53 +0100 Subject: [PATCH 02/11] Simplify exception cleanup for libunwind-style unwinding --- src/libpanic_unwind/gcc.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libpanic_unwind/gcc.rs b/src/libpanic_unwind/gcc.rs index 6a48fa05f8d08..6bec2686533eb 100644 --- a/src/libpanic_unwind/gcc.rs +++ b/src/libpanic_unwind/gcc.rs @@ -57,7 +57,7 @@ use unwind as uw; #[repr(C)] struct Exception { _uwe: uw::_Unwind_Exception, - cause: Option>, + cause: Box, } pub unsafe fn panic(data: Box) -> u32 { @@ -67,7 +67,7 @@ pub unsafe fn panic(data: Box) -> u32 { exception_cleanup, private: [0; uw::unwinder_private_data_size], }, - cause: Some(data), + cause: data, }); let exception_param = Box::into_raw(exception) as *mut uw::_Unwind_Exception; return uw::_Unwind_RaiseException(exception_param) as u32; @@ -87,10 +87,8 @@ pub fn payload() -> *mut u8 { } pub unsafe fn cleanup(ptr: *mut u8) -> Box { - let my_ep = ptr as *mut Exception; - let cause = (*my_ep).cause.take(); - uw::_Unwind_DeleteException(ptr as *mut _); - cause.unwrap() + let exception = Box::from_raw(ptr as *mut Exception); + exception.cause } // Rust's exception class identifier. This is used by personality routines to From c15ad84519193c3b7fd5c364cd46598303df92e5 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sun, 29 Dec 2019 21:16:20 +0100 Subject: [PATCH 03/11] Fix a memory leak in SEH unwinding if a Rust panic is caught by C++ and discarded --- src/libpanic_unwind/lib.rs | 1 + src/libpanic_unwind/seh.rs | 61 +++++++++++++++++++++++--- src/librustc_codegen_llvm/intrinsic.rs | 23 +++++++--- 3 files changed, 73 insertions(+), 12 deletions(-) diff --git a/src/libpanic_unwind/lib.rs b/src/libpanic_unwind/lib.rs index e721162edc067..9451eefb9a5cc 100644 --- a/src/libpanic_unwind/lib.rs +++ b/src/libpanic_unwind/lib.rs @@ -26,6 +26,7 @@ #![feature(staged_api)] #![feature(std_internals)] #![feature(unwind_attributes)] +#![feature(abi_thiscall)] #![panic_runtime] #![feature(panic_runtime)] diff --git a/src/libpanic_unwind/seh.rs b/src/libpanic_unwind/seh.rs index e1907ec4e5f32..417de8e23cc9b 100644 --- a/src/libpanic_unwind/seh.rs +++ b/src/libpanic_unwind/seh.rs @@ -77,8 +77,11 @@ use libc::{c_int, c_uint, c_void}; // #include // // struct rust_panic { +// rust_panic(const rust_panic&); +// ~rust_panic(); +// // uint64_t x[2]; -// } +// }; // // void foo() { // rust_panic a = {0, 1}; @@ -128,7 +131,7 @@ mod imp { #[repr(C)] pub struct _ThrowInfo { pub attributes: c_uint, - pub pnfnUnwind: imp::ptr_t, + pub pmfnUnwind: imp::ptr_t, pub pForwardCompat: imp::ptr_t, pub pCatchableTypeArray: imp::ptr_t, } @@ -145,7 +148,7 @@ pub struct _CatchableType { pub pType: imp::ptr_t, pub thisDisplacement: _PMD, pub sizeOrOffset: c_int, - pub copy_function: imp::ptr_t, + pub copyFunction: imp::ptr_t, } #[repr(C)] @@ -168,7 +171,7 @@ const TYPE_NAME: [u8; 11] = *b"rust_panic\0"; static mut THROW_INFO: _ThrowInfo = _ThrowInfo { attributes: 0, - pnfnUnwind: ptr!(0), + pmfnUnwind: ptr!(0), pForwardCompat: ptr!(0), pCatchableTypeArray: ptr!(0), }; @@ -181,7 +184,7 @@ static mut CATCHABLE_TYPE: _CatchableType = _CatchableType { pType: ptr!(0), thisDisplacement: _PMD { mdisp: 0, pdisp: -1, vdisp: 0 }, sizeOrOffset: mem::size_of::<[u64; 2]>() as c_int, - copy_function: ptr!(0), + copyFunction: ptr!(0), }; extern "C" { @@ -208,6 +211,39 @@ static mut TYPE_DESCRIPTOR: _TypeDescriptor = _TypeDescriptor { name: TYPE_NAME, }; +// Destructor used if the C++ code decides to capture the exception and drop it +// without propagating it. The catch part of the try intrinsic will set the +// first word of the exception object to 0 so that it is skipped by the +// destructor. +// +// Note that x86 Windows uses the "thiscall" calling convention for C++ member +// functions instead of the default "C" calling convention. +cfg_if::cfg_if! { + if #[cfg(target_arch = "x86")] { + unsafe extern "thiscall" fn exception_cleanup(e: *mut [u64; 2]) { + if (*e)[0] != 0 { + cleanup(*e); + } + } + unsafe extern "thiscall" fn exception_copy(_dest: *mut [u64; 2], + _src: *mut [u64; 2]) + -> *mut [u64; 2] { + panic!("Rust panics cannot be copied"); + } + } else { + unsafe extern "C" fn exception_cleanup(e: *mut [u64; 2]) { + if (*e)[0] != 0 { + cleanup(*e); + } + } + unsafe extern "C" fn exception_copy(_dest: *mut [u64; 2], + _src: *mut [u64; 2]) + -> *mut [u64; 2] { + panic!("Rust panics cannot be copied"); + } + } +} + pub unsafe fn panic(data: Box) -> u32 { use core::intrinsics::atomic_store; @@ -220,8 +256,7 @@ pub unsafe fn panic(data: Box) -> u32 { // exception (constructed above). let ptrs = mem::transmute::<_, raw::TraitObject>(data); let mut ptrs = [ptrs.data as u64, ptrs.vtable as u64]; - let ptrs_ptr = ptrs.as_mut_ptr(); - let throw_ptr = ptrs_ptr as *mut _; + let throw_ptr = ptrs.as_mut_ptr() as *mut _; // This... may seems surprising, and justifiably so. On 32-bit MSVC the // pointers between these structure are just that, pointers. On 64-bit MSVC, @@ -243,6 +278,12 @@ pub unsafe fn panic(data: Box) -> u32 { // // In any case, we basically need to do something like this until we can // express more operations in statics (and we may never be able to). + if !cfg!(bootstrap) { + atomic_store( + &mut THROW_INFO.pmfnUnwind as *mut _ as *mut u32, + ptr!(exception_cleanup) as u32, + ); + } atomic_store( &mut THROW_INFO.pCatchableTypeArray as *mut _ as *mut u32, ptr!(&CATCHABLE_TYPE_ARRAY as *const _) as u32, @@ -255,6 +296,12 @@ pub unsafe fn panic(data: Box) -> u32 { &mut CATCHABLE_TYPE.pType as *mut _ as *mut u32, ptr!(&TYPE_DESCRIPTOR as *const _) as u32, ); + if !cfg!(bootstrap) { + atomic_store( + &mut CATCHABLE_TYPE.copyFunction as *mut _ as *mut u32, + ptr!(exception_copy) as u32, + ); + } extern "system" { #[unwind(allowed)] diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 8ea50a972ece6..5adff0d1f9233 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -922,6 +922,9 @@ fn codegen_msvc_try( // #include // // struct rust_panic { + // rust_panic(const rust_panic&); + // ~rust_panic(); + // // uint64_t x[2]; // } // @@ -929,17 +932,19 @@ fn codegen_msvc_try( // try { // foo(); // return 0; - // } catch(rust_panic a) { + // } catch(rust_panic& a) { // ret[0] = a.x[0]; // ret[1] = a.x[1]; + // a.x[0] = 0; // return 1; // } // } // // More information can be found in libstd's seh.rs implementation. let i64_2 = bx.type_array(bx.type_i64(), 2); - let i64_align = bx.tcx().data_layout.i64_align.abi; - let slot = bx.alloca(i64_2, i64_align); + let i64_2_ptr = bx.type_ptr_to(i64_2); + let ptr_align = bx.tcx().data_layout.pointer_align.abi; + let slot = bx.alloca(i64_2_ptr, ptr_align); bx.invoke(func, &[data], normal.llbb(), catchswitch.llbb(), None); normal.ret(bx.const_i32(0)); @@ -951,11 +956,19 @@ fn codegen_msvc_try( Some(did) => bx.get_static(did), None => bug!("eh_catch_typeinfo not defined, but needed for SEH unwinding"), }; - let funclet = catchpad.catch_pad(cs, &[tydesc, bx.const_i32(0), slot]); + let flags = bx.const_i32(8); // Catch by reference + let funclet = catchpad.catch_pad(cs, &[tydesc, flags, slot]); - let payload = catchpad.load(slot, i64_align); + let i64_align = bx.tcx().data_layout.i64_align.abi; + let payload_ptr = catchpad.load(slot, ptr_align); + let payload = catchpad.load(payload_ptr, i64_align); let local_ptr = catchpad.bitcast(local_ptr, bx.type_ptr_to(i64_2)); catchpad.store(payload, local_ptr, i64_align); + + // Clear the first word of the exception so avoid double-dropping it. + let payload_0_ptr = catchpad.inbounds_gep(payload_ptr, &[bx.const_i32(0), bx.const_i32(0)]); + catchpad.store(bx.const_u64(0), payload_0_ptr, i64_align); + catchpad.catch_ret(&funclet, caught.llbb()); caught.ret(bx.const_i32(1)); From 43611921120e2df3383f99b42bf060a6be28f23e Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sun, 29 Dec 2019 23:23:40 +0100 Subject: [PATCH 04/11] Add a test to check that swallowed Rust panics are dropped properly. --- .../foreign-exceptions/foo.cpp | 17 +++++++++++ .../foreign-exceptions/foo.rs | 30 ++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/test/run-make-fulldeps/foreign-exceptions/foo.cpp b/src/test/run-make-fulldeps/foreign-exceptions/foo.cpp index b0fd65f88e7de..ef5cd74c65283 100644 --- a/src/test/run-make-fulldeps/foreign-exceptions/foo.cpp +++ b/src/test/run-make-fulldeps/foreign-exceptions/foo.cpp @@ -57,4 +57,21 @@ extern "C" { throw; } } + + void swallow_exception(void (*cb)()) { + try { + // Do a rethrow to ensure that the exception is only dropped once. + // This is necessary since we don't support copying exceptions. + try { + cb(); + } catch (...) { + println("rethrowing Rust panic"); + throw; + }; + } catch (rust_panic e) { + assert(false && "shouldn't be able to catch a rust panic"); + } catch (...) { + println("swallowing foreign exception in catch (...)"); + } + } } diff --git a/src/test/run-make-fulldeps/foreign-exceptions/foo.rs b/src/test/run-make-fulldeps/foreign-exceptions/foo.rs index 399c78f8d2d02..d9818dffc502b 100644 --- a/src/test/run-make-fulldeps/foreign-exceptions/foo.rs +++ b/src/test/run-make-fulldeps/foreign-exceptions/foo.rs @@ -4,7 +4,6 @@ // For linking libstdc++ on MinGW #![cfg_attr(all(windows, target_env = "gnu"), feature(static_nobundle))] - #![feature(unwind_attributes)] use std::panic::{catch_unwind, AssertUnwindSafe}; @@ -20,6 +19,8 @@ impl<'a> Drop for DropCheck<'a> { extern "C" { fn throw_cxx_exception(); + fn swallow_exception(cb: extern "C" fn()); + #[unwind(allowed)] fn cxx_catch_callback(cb: extern "C" fn(), ok: *mut bool); } @@ -60,7 +61,34 @@ fn throw_rust_panic() { assert!(cxx_ok); } +fn check_exception_drop() { + static mut DROP_COUNT: usize = 0; + + struct CountDrop; + impl Drop for CountDrop { + fn drop(&mut self) { + println!("CountDrop::drop"); + unsafe { + DROP_COUNT += 1; + } + } + } + + + #[unwind(allowed)] + extern "C" fn callback() { + println!("throwing rust panic #2"); + panic!(CountDrop); + } + + unsafe { + swallow_exception(callback); + assert_eq!(DROP_COUNT, 1); + } +} + fn main() { unsafe { throw_cxx_exception() }; throw_rust_panic(); + check_exception_drop(); } From 838e3874fc11f24ebe8a7dff02a7f7614a5d2938 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 1 Jan 2020 22:14:37 +0100 Subject: [PATCH 05/11] Explain the panic! in exception_copy --- src/libpanic_unwind/seh.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libpanic_unwind/seh.rs b/src/libpanic_unwind/seh.rs index 417de8e23cc9b..ef27e38c2d1d6 100644 --- a/src/libpanic_unwind/seh.rs +++ b/src/libpanic_unwind/seh.rs @@ -218,6 +218,12 @@ static mut TYPE_DESCRIPTOR: _TypeDescriptor = _TypeDescriptor { // // Note that x86 Windows uses the "thiscall" calling convention for C++ member // functions instead of the default "C" calling convention. +// +// The exception_copy function is a bit special here: it is invoked by the MSVC +// runtime under a try/catch block and the panic that we generate here will be +// used as the result of the exception copy. This is used by the C++ runtime to +// support capturing exceptions with std::exception_ptr, which we can't support +// because Box isn't clonable. cfg_if::cfg_if! { if #[cfg(target_arch = "x86")] { unsafe extern "thiscall" fn exception_cleanup(e: *mut [u64; 2]) { @@ -225,6 +231,7 @@ cfg_if::cfg_if! { cleanup(*e); } } + #[unwind(allowed)] unsafe extern "thiscall" fn exception_copy(_dest: *mut [u64; 2], _src: *mut [u64; 2]) -> *mut [u64; 2] { @@ -236,6 +243,7 @@ cfg_if::cfg_if! { cleanup(*e); } } + #[unwind(allowed)] unsafe extern "C" fn exception_copy(_dest: *mut [u64; 2], _src: *mut [u64; 2]) -> *mut [u64; 2] { From ed217a53ff959414e9c723d8fecf80bc797a5a77 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 1 Jan 2020 22:24:39 +0100 Subject: [PATCH 06/11] Explain flag value of 8 for msvc_try --- src/librustc_codegen_llvm/intrinsic.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 5adff0d1f9233..27308cabd45e6 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -952,11 +952,15 @@ fn codegen_msvc_try( let cs = catchswitch.catch_switch(None, None, 1); catchswitch.add_handler(cs, catchpad.llbb()); + // The flag value of 8 indicates that we are catching the exception by + // reference instead of by value. + // + // Source: MicrosoftCXXABI::getAddrOfCXXCatchHandlerType in clang + let flags = bx.const_i32(8); let tydesc = match bx.tcx().lang_items().eh_catch_typeinfo() { Some(did) => bx.get_static(did), None => bug!("eh_catch_typeinfo not defined, but needed for SEH unwinding"), }; - let flags = bx.const_i32(8); // Catch by reference let funclet = catchpad.catch_pad(cs, &[tydesc, flags, slot]); let i64_align = bx.tcx().data_layout.i64_align.abi; From 757ed07f374edfe93be5c9084ac5c44ba738e1b2 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Tue, 7 Jan 2020 11:36:57 +0100 Subject: [PATCH 07/11] Apply review feedback --- src/libpanic_unwind/emcc.rs | 4 ++++ src/libpanic_unwind/seh.rs | 31 +++++++++++--------------- src/librustc_codegen_llvm/intrinsic.rs | 6 ++++- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/libpanic_unwind/emcc.rs b/src/libpanic_unwind/emcc.rs index 61f33fd4d5d66..fbadf4ac6a058 100644 --- a/src/libpanic_unwind/emcc.rs +++ b/src/libpanic_unwind/emcc.rs @@ -53,6 +53,10 @@ pub fn payload() -> *mut u8 { } struct Exception { + // 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 + // is still going to be called by __cxa_end_catch.. data: Option>, } diff --git a/src/libpanic_unwind/seh.rs b/src/libpanic_unwind/seh.rs index ef27e38c2d1d6..f1d0080472a01 100644 --- a/src/libpanic_unwind/seh.rs +++ b/src/libpanic_unwind/seh.rs @@ -224,33 +224,28 @@ static mut TYPE_DESCRIPTOR: _TypeDescriptor = _TypeDescriptor { // used as the result of the exception copy. This is used by the C++ runtime to // support capturing exceptions with std::exception_ptr, which we can't support // because Box isn't clonable. -cfg_if::cfg_if! { - if #[cfg(target_arch = "x86")] { - unsafe extern "thiscall" fn exception_cleanup(e: *mut [u64; 2]) { - if (*e)[0] != 0 { - cleanup(*e); - } - } - #[unwind(allowed)] - unsafe extern "thiscall" fn exception_copy(_dest: *mut [u64; 2], - _src: *mut [u64; 2]) - -> *mut [u64; 2] { - panic!("Rust panics cannot be copied"); - } - } else { - unsafe extern "C" fn exception_cleanup(e: *mut [u64; 2]) { +macro_rules! define_cleanup { + ($abi:tt) => { + unsafe extern $abi fn exception_cleanup(e: *mut [u64; 2]) { if (*e)[0] != 0 { cleanup(*e); } } #[unwind(allowed)] - unsafe extern "C" fn exception_copy(_dest: *mut [u64; 2], - _src: *mut [u64; 2]) - -> *mut [u64; 2] { + unsafe extern $abi fn exception_copy(_dest: *mut [u64; 2], + _src: *mut [u64; 2]) + -> *mut [u64; 2] { panic!("Rust panics cannot be copied"); } } } +cfg_if::cfg_if! { + if #[cfg(target_arch = "x86")] { + define_cleanup!("thiscall"); + } else { + define_cleanup!("C"); + } +} pub unsafe fn panic(data: Box) -> u32 { use core::intrinsics::atomic_store; diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 27308cabd45e6..031837c1efbe8 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -953,7 +953,9 @@ fn codegen_msvc_try( catchswitch.add_handler(cs, catchpad.llbb()); // The flag value of 8 indicates that we are catching the exception by - // reference instead of by value. + // reference instead of by value. We can't use catch by value because + // that requires copying the exception object, which we don't support + // since our exception object effectively contains a Box. // // Source: MicrosoftCXXABI::getAddrOfCXXCatchHandlerType in clang let flags = bx.const_i32(8); @@ -970,6 +972,8 @@ fn codegen_msvc_try( catchpad.store(payload, local_ptr, i64_align); // Clear the first word of the exception so avoid double-dropping it. + // This will be read by the destructor which is implicitly called at the + // end of the catch block by the runtime. let payload_0_ptr = catchpad.inbounds_gep(payload_ptr, &[bx.const_i32(0), bx.const_i32(0)]); catchpad.store(bx.const_u64(0), payload_0_ptr, i64_align); From 3a025760be8f4c56f0777fa34ba64a4f7bada8e7 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Fri, 10 Jan 2020 00:19:40 +0000 Subject: [PATCH 08/11] Abort if C++ tries to swallow a Rust panic --- src/libpanic_unwind/emcc.rs | 1 + src/libpanic_unwind/gcc.rs | 1 + src/libpanic_unwind/lib.rs | 6 ++++ src/libpanic_unwind/seh.rs | 1 + src/libstd/panicking.rs | 8 +++++ .../foreign-exceptions/foo.cpp | 17 ----------- .../foreign-exceptions/foo.rs | 29 ------------------- 7 files changed, 17 insertions(+), 46 deletions(-) diff --git a/src/libpanic_unwind/emcc.rs b/src/libpanic_unwind/emcc.rs index fbadf4ac6a058..268bafd240930 100644 --- a/src/libpanic_unwind/emcc.rs +++ b/src/libpanic_unwind/emcc.rs @@ -80,6 +80,7 @@ pub unsafe fn panic(data: Box) -> u32 { extern "C" fn exception_cleanup(ptr: *mut libc::c_void) { unsafe { ptr::drop_in_place(ptr as *mut Exception); + super::__rust_drop_panic(); } } } diff --git a/src/libpanic_unwind/gcc.rs b/src/libpanic_unwind/gcc.rs index 6bec2686533eb..6e04317d491fc 100644 --- a/src/libpanic_unwind/gcc.rs +++ b/src/libpanic_unwind/gcc.rs @@ -78,6 +78,7 @@ pub unsafe fn panic(data: Box) -> u32 { ) { unsafe { let _: Box = Box::from_raw(exception as *mut Exception); + super::__rust_drop_panic(); } } } diff --git a/src/libpanic_unwind/lib.rs b/src/libpanic_unwind/lib.rs index 9451eefb9a5cc..6383ae39fb6db 100644 --- a/src/libpanic_unwind/lib.rs +++ b/src/libpanic_unwind/lib.rs @@ -61,6 +61,12 @@ cfg_if::cfg_if! { } } +extern "C" { + /// Handler in libstd called when a panic object is dropped outside of + /// `catch_unwind`. + fn __rust_drop_panic() -> !; +} + mod dwarf; // Entry point for catching an exception, implemented using the `try` intrinsic diff --git a/src/libpanic_unwind/seh.rs b/src/libpanic_unwind/seh.rs index f1d0080472a01..d9dca2c0f4f47 100644 --- a/src/libpanic_unwind/seh.rs +++ b/src/libpanic_unwind/seh.rs @@ -229,6 +229,7 @@ macro_rules! define_cleanup { unsafe extern $abi fn exception_cleanup(e: *mut [u64; 2]) { if (*e)[0] != 0 { cleanup(*e); + super::__rust_drop_panic(); } } #[unwind(allowed)] diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 599ccc809be1f..43c2965f2315a 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -55,6 +55,14 @@ extern "C" { fn __rust_start_panic(payload: usize) -> u32; } +/// This function is called by the panic runtime if FFI code catches a Rust +/// panic but doesn't rethrow it. We don't support this case since it messes +/// with our panic count. +#[rustc_std_internal_symbol] +extern "C" fn __rust_drop_panic() -> ! { + rtabort!("Rust panics must be rethrown"); +} + #[derive(Copy, Clone)] enum Hook { Default, diff --git a/src/test/run-make-fulldeps/foreign-exceptions/foo.cpp b/src/test/run-make-fulldeps/foreign-exceptions/foo.cpp index ef5cd74c65283..b0fd65f88e7de 100644 --- a/src/test/run-make-fulldeps/foreign-exceptions/foo.cpp +++ b/src/test/run-make-fulldeps/foreign-exceptions/foo.cpp @@ -57,21 +57,4 @@ extern "C" { throw; } } - - void swallow_exception(void (*cb)()) { - try { - // Do a rethrow to ensure that the exception is only dropped once. - // This is necessary since we don't support copying exceptions. - try { - cb(); - } catch (...) { - println("rethrowing Rust panic"); - throw; - }; - } catch (rust_panic e) { - assert(false && "shouldn't be able to catch a rust panic"); - } catch (...) { - println("swallowing foreign exception in catch (...)"); - } - } } diff --git a/src/test/run-make-fulldeps/foreign-exceptions/foo.rs b/src/test/run-make-fulldeps/foreign-exceptions/foo.rs index d9818dffc502b..9c2045c8c89f7 100644 --- a/src/test/run-make-fulldeps/foreign-exceptions/foo.rs +++ b/src/test/run-make-fulldeps/foreign-exceptions/foo.rs @@ -19,8 +19,6 @@ impl<'a> Drop for DropCheck<'a> { extern "C" { fn throw_cxx_exception(); - fn swallow_exception(cb: extern "C" fn()); - #[unwind(allowed)] fn cxx_catch_callback(cb: extern "C" fn(), ok: *mut bool); } @@ -61,34 +59,7 @@ fn throw_rust_panic() { assert!(cxx_ok); } -fn check_exception_drop() { - static mut DROP_COUNT: usize = 0; - - struct CountDrop; - impl Drop for CountDrop { - fn drop(&mut self) { - println!("CountDrop::drop"); - unsafe { - DROP_COUNT += 1; - } - } - } - - - #[unwind(allowed)] - extern "C" fn callback() { - println!("throwing rust panic #2"); - panic!(CountDrop); - } - - unsafe { - swallow_exception(callback); - assert_eq!(DROP_COUNT, 1); - } -} - fn main() { unsafe { throw_cxx_exception() }; throw_rust_panic(); - check_exception_drop(); } From 8f60db8da8f7f159dbe93ce481a4608633c4c1bf Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sun, 12 Jan 2020 16:59:44 +0000 Subject: [PATCH 09/11] Don't include __rust_drop_panic when testing libstd --- src/libstd/panicking.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 43c2965f2315a..bfadeafb7c773 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -58,6 +58,7 @@ extern "C" { /// This function is called by the panic runtime if FFI code catches a Rust /// panic but doesn't rethrow it. We don't support this case since it messes /// with our panic count. +#[cfg(not(test))] #[rustc_std_internal_symbol] extern "C" fn __rust_drop_panic() -> ! { rtabort!("Rust panics must be rethrown"); From 4f163afed621adb51d0dfc7dd00c23cf38010d14 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Mon, 13 Jan 2020 00:39:41 +0000 Subject: [PATCH 10/11] Fix destructor return value in emcc.rs --- src/libpanic_unwind/emcc.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/libpanic_unwind/emcc.rs b/src/libpanic_unwind/emcc.rs index 268bafd240930..0f93140238bc4 100644 --- a/src/libpanic_unwind/emcc.rs +++ b/src/libpanic_unwind/emcc.rs @@ -76,12 +76,20 @@ pub unsafe fn panic(data: Box) -> u32 { } ptr::write(exception, Exception { data: Some(data) }); __cxa_throw(exception as *mut _, &EXCEPTION_TYPE_INFO, exception_cleanup); +} - extern "C" fn exception_cleanup(ptr: *mut libc::c_void) { - unsafe { - ptr::drop_in_place(ptr as *mut Exception); - super::__rust_drop_panic(); - } +// 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 { + unsafe { + ptr::drop_in_place(ptr as *mut Exception); + super::__rust_drop_panic(); } } @@ -104,7 +112,7 @@ extern "C" { fn __cxa_throw( thrown_exception: *mut libc::c_void, tinfo: *const TypeInfo, - dest: extern "C" fn(*mut libc::c_void), + dest: extern "C" fn(*mut libc::c_void) -> DestructorRet, ) -> !; fn __gxx_personality_v0( version: c_int, From 25519e5290b4e04ab7a6cb07bcda01a542a0b1f3 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Mon, 13 Jan 2020 08:04:48 +0000 Subject: [PATCH 11/11] Fix destructor in emcc.rs --- src/libpanic_unwind/emcc.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libpanic_unwind/emcc.rs b/src/libpanic_unwind/emcc.rs index 0f93140238bc4..9161d49959cf5 100644 --- a/src/libpanic_unwind/emcc.rs +++ b/src/libpanic_unwind/emcc.rs @@ -88,8 +88,12 @@ cfg_if::cfg_if! { } extern "C" fn exception_cleanup(ptr: *mut libc::c_void) -> DestructorRet { unsafe { - ptr::drop_in_place(ptr as *mut Exception); - super::__rust_drop_panic(); + 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 } }