Guard against unwinding in cleanup code

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -135,21 +135,38 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
         // If there is a cleanup block and the function we're calling can unwind, then
         // do an invoke, otherwise do a call.
         let fn_ty = bx.fn_decl_backend_type(&fn_abi);
-        if let Some(cleanup) = cleanup.filter(|_| fn_abi.can_unwind) {
+
+        let unwind_block = if let Some(cleanup) = cleanup.filter(|_| fn_abi.can_unwind) {
+            Some(self.llblock(fx, cleanup))
+        } else if fx.mir[self.bb].is_cleanup
+            && fn_abi.can_unwind
+            && !base::wants_msvc_seh(fx.cx.tcx().sess)
+        {
+            // Exception must not propagate out of the execution of a cleanup (doing so
+            // can cause undefined behaviour). We insert a double unwind guard for
+            // functions that can potentially unwind to protect against this.
+            //
+            // This is not necessary for SEH which does not use successive unwinding
+            // like Itanium EH. EH frames in SEH are different from normal function
+            // frames and SEH will abort automatically if an exception tries to
+            // propagate out from cleanup.
+            Some(fx.double_unwind_guard())
+        } else {
+            None
+        };
+
+        if let Some(unwind_block) = unwind_block {
             let ret_llbb = if let Some((_, target)) = destination {
                 fx.llbb(target)
             } else {
                 fx.unreachable_block()
             };
-            let invokeret = bx.invoke(
-                fn_ty,
-                fn_ptr,
-                &llargs,
-                ret_llbb,
-                self.llblock(fx, cleanup),
-                self.funclet(fx),
-            );
+            let invokeret =
+                bx.invoke(fn_ty, fn_ptr, &llargs, ret_llbb, unwind_block, self.funclet(fx));
             bx.apply_attrs_callsite(&fn_abi, invokeret);
+            if fx.mir[self.bb].is_cleanup {
+                bx.apply_attrs_to_cleanup_callsite(invokeret);
+            }
 
             if let Some((ret_dest, target)) = destination {
                 let mut ret_bx = fx.build_block(target);
@@ -486,17 +503,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         let span = terminator.source_info.span;
         self.set_debug_loc(&mut bx, terminator.source_info);
 
-        // Get the location information.
-        let location = self.get_caller_location(&mut bx, terminator.source_info).immediate();
-
         // Obtain the panic entry point.
         let def_id = common::langcall(bx.tcx(), Some(span), "", LangItem::PanicNoUnwind);
         let instance = ty::Instance::mono(bx.tcx(), def_id);
         let fn_abi = bx.fn_abi_of_instance(instance, ty::List::empty());
         let llfn = bx.get_fn_addr(instance);
 
         // Codegen the actual panic invoke/call.
-        helper.do_call(self, &mut bx, fn_abi, llfn, &[location], None, None);
+        helper.do_call(self, &mut bx, fn_abi, llfn, &[], None, None);
     }
 
     /// Returns `true` if this is indeed a panic intrinsic and codegen is done.
@@ -1398,6 +1412,35 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         })
     }
 
+    fn double_unwind_guard(&mut self) -> Bx::BasicBlock {
+        self.double_unwind_guard.unwrap_or_else(|| {
+            assert!(!base::wants_msvc_seh(self.cx.sess()));
+
+            let mut bx = self.new_block("abort");
+            self.set_debug_loc(&mut bx, mir::SourceInfo::outermost(self.mir.span));
+
+            let llpersonality = self.cx.eh_personality();
+            let llretty = self.landing_pad_type();
+            bx.cleanup_landing_pad(llretty, llpersonality);
+
+            let def_id = common::langcall(bx.tcx(), None, "", LangItem::PanicNoUnwind);
+            let instance = ty::Instance::mono(bx.tcx(), def_id);
+            let fn_abi = bx.fn_abi_of_instance(instance, ty::List::empty());
+            let fn_ptr = bx.get_fn_addr(instance);
+            let fn_ty = bx.fn_decl_backend_type(&fn_abi);
+
+            let llret = bx.call(fn_ty, fn_ptr, &[], None);
+            bx.apply_attrs_callsite(&fn_abi, llret);
+            bx.apply_attrs_to_cleanup_callsite(llret);
+
+            bx.unreachable();
+            let llbb = bx.llbb();
+
+            self.double_unwind_guard = Some(llbb);
+            llbb
+        })
+    }
+
     // FIXME(eddyb) replace with `build_sibling_block`/`append_sibling_block`
     // (which requires having a `Bx` already, and not all callers do).
     fn new_block(&self, name: &str) -> Bx {

compiler/rustc_codegen_ssa/src/mir/mod.rs

@@ -62,6 +62,9 @@ pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> {
     /// Cached unreachable block
     unreachable_block: Option<Bx::BasicBlock>,
 
+    /// Cached double unwind guarding block
+    double_unwind_guard: Option<Bx::BasicBlock>,
+
     /// The location where each MIR arg/var/tmp/ret is stored. This is
     /// usually an `PlaceRef` representing an alloca, but not always:
     /// sometimes we can skip the alloca and just store the value
@@ -169,6 +172,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
         personality_slot: None,
         cached_llbbs,
         unreachable_block: None,
+        double_unwind_guard: None,
         cleanup_kinds,
         landing_pads: IndexVec::from_elem(None, mir.basic_blocks()),
         funclets: IndexVec::from_fn_n(|_| None, mir.basic_blocks().len()),

library/core/src/panicking.rs

@@ -87,8 +87,7 @@ fn panic_bounds_check(index: usize, len: usize) -> ! {
 
 #[cfg(not(bootstrap))]
 #[cold]
-#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
-#[track_caller]
+#[inline(never)]
 #[lang = "panic_no_unwind"] // needed by codegen for panic in nounwind function
 fn panic_no_unwind() -> ! {
     if cfg!(feature = "panic_immediate_abort") {

src/test/codegen/drop.rs

@@ -23,17 +23,7 @@ pub fn droppy() {
 // FIXME(eddyb) the `void @` forces a match on the instruction, instead of the
 // comment, that's `; call core::ptr::drop_in_place::<drop::SomeUniqueName>`
 // for the `v0` mangling, should switch to matching on that once `legacy` is gone.
-// CHECK-NOT: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK-NOT: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK-NOT: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK-NOT: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK-NOT: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
+// CHECK-COUNT-6: {{(call|invoke) void @.*}}drop_in_place{{.*}}SomeUniqueName
 // CHECK-NOT: {{(call|invoke) void @.*}}drop_in_place{{.*}}SomeUniqueName
 // The next line checks for the } that ends the function definition
 // CHECK-LABEL: {{^[}]}}

src/test/codegen/unwind-landingpad-cold.rs

@@ -6,7 +6,7 @@
 // get the `cold` attribute.
 
 // CHECK-LABEL: @check_cold
-// CHECK: call void {{.+}}drop_in_place{{.+}} [[ATTRIBUTES:#[0-9]+]]
+// CHECK: {{(call|invoke) void .+}}drop_in_place{{.+}} [[ATTRIBUTES:#[0-9]+]]
 // CHECK: attributes [[ATTRIBUTES]] = { cold }
 #[no_mangle]
 pub fn check_cold(f: fn(), x: Box<u32>) {

src/test/run-make-fulldeps/foreign-double-unwind/Makefile

@@ -0,0 +1,10 @@
+-include ../tools.mk
+
+all: foo
+	$(call RUN,foo) | $(CGREP) -v unreachable
+
+foo: foo.rs $(call NATIVE_STATICLIB,foo)
+	$(RUSTC) $< -lfoo $(EXTRARSCXXFLAGS)
+
+$(TMPDIR)/libfoo.o: foo.cpp
+	$(call COMPILE_OBJ_CXX,$@,$<)

src/test/run-make-fulldeps/foreign-double-unwind/foo.cpp

@@ -0,0 +1,33 @@
+#include <cstdio>
+#include <exception>
+
+void println(const char* s) {
+    puts(s);
+    fflush(stdout);
+}
+
+struct outer_exception {};
+struct inner_exception {};
+
+extern "C" {
+    void throw_cxx_exception() {
+        if (std::uncaught_exception()) {
+            println("throwing inner C++ exception");
+            throw inner_exception();
+        } else {
+            println("throwing outer C++ exception");
+            throw outer_exception();
+        }
+    }
+
+    void cxx_catch_callback(void (*cb)()) {
+        try {
+            cb();
+            println("unreachable: callback returns");
+        } catch (outer_exception) {
+            println("unreachable: caught outer exception in catch (...)");
+        } catch (inner_exception) {
+            println("unreachable: caught inner exception in catch (...)");
+        }
+    }
+}

src/test/run-make-fulldeps/foreign-double-unwind/foo.rs

@@ -0,0 +1,26 @@
+// Tests that C++ double unwinding through Rust code will be properly guarded
+// against instead of exhibiting undefined behaviour.
+
+#![feature(c_unwind)]
+
+extern "C-unwind" {
+    fn throw_cxx_exception();
+    fn cxx_catch_callback(cb: extern "C-unwind" fn());
+}
+
+struct ThrowOnDrop;
+
+impl Drop for ThrowOnDrop {
+    fn drop(&mut self) {
+        unsafe { throw_cxx_exception() };
+    }
+}
+
+extern "C-unwind" fn test_double_unwind() {
+    let _a = ThrowOnDrop;
+    let _b = ThrowOnDrop;
+}
+
+fn main() {
+    unsafe { cxx_catch_callback(test_double_unwind) };
+}