From c15ad84519193c3b7fd5c364cd46598303df92e5 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sun, 29 Dec 2019 21:16:20 +0100 Subject: [PATCH] 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 e721162edc0..9451eefb9a5 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 e1907ec4e5f..417de8e23cc 100644 --- a/src/libpanic_unwind/seh.rs +++ b/src/libpanic_unwind/seh.rs @@ -77,8 +77,11 @@ // #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 @@ macro_rules! ptr { #[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 @@ pub struct _TypeDescriptor { static mut THROW_INFO: _ThrowInfo = _ThrowInfo { attributes: 0, - pnfnUnwind: ptr!(0), + pmfnUnwind: ptr!(0), pForwardCompat: ptr!(0), pCatchableTypeArray: ptr!(0), }; @@ -181,7 +184,7 @@ pub struct _TypeDescriptor { 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 @@ pub struct _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 8ea50a972ec..5adff0d1f92 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)); -- 2.44.0