Rollup merge of rust-lang#48572 - alexcrichton:noexcept-msvc2, r=eddyb

rustc: Tweak funclet cleanups of ffi functions

This commit is targeted at addressing rust-lang#48251 by specifically fixing a case where
a longjmp over Rust frames on MSVC runs cleanups, accidentally running the
"abort the program" cleanup as well. Added in rust-lang#46833 `extern` ABI functions in
Rust will abort the process if Rust panics, and currently this is modeled as a
normal cleanup like all other destructors.

Unfortunately it turns out that `longjmp` on MSVC is implemented with SEH, the
same mechanism used to implement panics in Rust. This means that `longjmp` over
Rust frames will run Rust cleanups (even though we don't necessarily want it
to). Notably this means that if you `longjmp` over a Rust stack frame then that
probably means you'll abort the program because one of the cleanups will abort
the process.

After some discussion on IRC it turns out that `longjmp` doesn't run cleanups
for *caught* exceptions, it only runs cleanups for cleanup pads. Using this
information this commit tweaks the codegen for an `extern` function to
a catch-all clause for exceptions instead of a cleanup block. This catch-all is
equivalent to the C++ code:

    try {
        foo();
    } catch (...) {
        bar();
    }

and in fact our codegen here is designed to match exactly what clang emits for
that C++ code!

With this tweak a longjmp over Rust code will no longer abort the process. A
longjmp will continue to "accidentally" run Rust cleanups (destructors) on MSVC.
Other non-MSVC platforms will not rust destructors with a longjmp, so we'll
probably still recommend "don't have destructors on the stack", but in any case
this is a more surgical fix than rust-lang#48567 and should help us stick to standard
personality functions a bit longer.

Author: Manishearth

src/librustc_trans/mir/mod.rs

@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use common::{C_i32, C_null};
 use libc::c_uint;
 use llvm::{self, ValueRef, BasicBlockRef};
 use llvm::debuginfo::DIScope;
@@ -23,6 +24,7 @@ use common::{CodegenCx, Funclet};
 use debuginfo::{self, declare_local, VariableAccess, VariableKind, FunctionDebugContext};
 use monomorphize::Instance;
 use abi::{ArgAttribute, FnType, PassMode};
+use type_::Type;
 
 use syntax_pos::{DUMMY_SP, NO_EXPANSION, BytePos, Span};
 use syntax::symbol::keywords;
@@ -222,7 +224,7 @@ pub fn trans_mir<'a, 'tcx: 'a>(
 
     // Compute debuginfo scopes from MIR scopes.
     let scopes = debuginfo::create_mir_scopes(cx, mir, &debug_context);
-    let (landing_pads, funclets) = create_funclets(&bx, &cleanup_kinds, &block_bxs);
+    let (landing_pads, funclets) = create_funclets(mir, &bx, &cleanup_kinds, &block_bxs);
 
     let mut fx = FunctionCx {
         mir,
@@ -333,6 +335,7 @@ pub fn trans_mir<'a, 'tcx: 'a>(
 }
 
 fn create_funclets<'a, 'tcx>(
+    mir: &'a Mir<'tcx>,
     bx: &Builder<'a, 'tcx>,
     cleanup_kinds: &IndexVec<mir::BasicBlock, CleanupKind>,
     block_bxs: &IndexVec<mir::BasicBlock, BasicBlockRef>)
@@ -341,14 +344,59 @@ fn create_funclets<'a, 'tcx>(
 {
     block_bxs.iter_enumerated().zip(cleanup_kinds).map(|((bb, &llbb), cleanup_kind)| {
         match *cleanup_kind {
-            CleanupKind::Funclet if base::wants_msvc_seh(bx.sess()) => {
+            CleanupKind::Funclet if base::wants_msvc_seh(bx.sess()) => {}
+            _ => return (None, None)
+        }
+
+        let cleanup;
+        let ret_llbb;
+        match mir[bb].terminator.as_ref().map(|t| &t.kind) {
+            // This is a basic block that we're aborting the program for,
+            // notably in an `extern` function. These basic blocks are inserted
+            // so that we assert that `extern` functions do indeed not panic,
+            // and if they do we abort the process.
+            //
+            // On MSVC these are tricky though (where we're doing funclets). If
+            // we were to do a cleanuppad (like below) the normal functions like
+            // `longjmp` would trigger the abort logic, terminating the
+            // program. Instead we insert the equivalent of `catch(...)` for C++
+            // which magically doesn't trigger when `longjmp` files over this
+            // frame.
+            //
+            // Lots more discussion can be found on #48251 but this codegen is
+            // modeled after clang's for:
+            //
+            //      try {
+            //          foo();
+            //      } catch (...) {
+            //          bar();
+            //      }
+            Some(&mir::TerminatorKind::Abort) => {
+                let cs_bx = bx.build_sibling_block(&format!("cs_funclet{:?}", bb));
+                let cp_bx = bx.build_sibling_block(&format!("cp_funclet{:?}", bb));
+                ret_llbb = cs_bx.llbb();
+
+                let cs = cs_bx.catch_switch(None, None, 1);
+                cs_bx.add_handler(cs, cp_bx.llbb());
+
+                // The "null" here is actually a RTTI type descriptor for the
+                // C++ personality function, but `catch (...)` has no type so
+                // it's null. The 64 here is actually a bitfield which
+                // represents that this is a catch-all block.
+                let null = C_null(Type::i8p(bx.cx));
+                let sixty_four = C_i32(bx.cx, 64);
+                cleanup = cp_bx.catch_pad(cs, &[null, sixty_four, null]);
+                cp_bx.br(llbb);
+            }
+            _ => {
                 let cleanup_bx = bx.build_sibling_block(&format!("funclet_{:?}", bb));
-                let cleanup = cleanup_bx.cleanup_pad(None, &[]);
+                ret_llbb = cleanup_bx.llbb();
+                cleanup = cleanup_bx.cleanup_pad(None, &[]);
                 cleanup_bx.br(llbb);
-                (Some(cleanup_bx.llbb()), Some(Funclet::new(cleanup)))
             }
-            _ => (None, None)
-        }
+        };
+
+        (Some(ret_llbb), Some(Funclet::new(cleanup)))
     }).unzip()
 }
 

src/test/run-make/longjmp-across-rust/Makefile

@@ -0,0 +1,5 @@
+-include ../tools.mk
+
+all: $(call NATIVE_STATICLIB,foo)
+	$(RUSTC) main.rs
+	$(call RUN,main)

src/test/run-make/longjmp-across-rust/foo.c

@@ -0,0 +1,28 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#include <assert.h>
+#include <setjmp.h>
+
+static jmp_buf ENV;
+
+extern void test_middle();
+
+void test_start(void(*f)()) {
+  if (setjmp(ENV) != 0)
+    return;
+  f();
+  assert(0);
+}
+
+void test_end() {
+  longjmp(ENV, 1);
+  assert(0);
+}

src/test/run-make/longjmp-across-rust/main.rs

@@ -0,0 +1,40 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[link(name = "foo", kind = "static")]
+extern {
+    fn test_start(f: extern fn());
+    fn test_end();
+}
+
+fn main() {
+    unsafe {
+        test_start(test_middle);
+    }
+}
+
+struct A;
+
+impl Drop for A {
+    fn drop(&mut self) {
+    }
+}
+
+extern fn test_middle() {
+    let _a = A;
+    foo();
+}
+
+fn foo() {
+    let _a = A;
+    unsafe {
+        test_end();
+    }
+}