From 804666f4ad5a66e6a1bae3b2e326efa030135a05 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 26 Feb 2018 18:59:47 -0800 Subject: [PATCH] rustc: Tweak funclet cleanups of ffi functions This commit is targeted at addressing #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 #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 #48567 and should help us stick to standard personality functions a bit longer. --- src/librustc_trans/mir/mod.rs | 60 +++++++++++++++++-- .../run-make/longjmp-across-rust/Makefile | 5 ++ src/test/run-make/longjmp-across-rust/foo.c | 28 +++++++++ src/test/run-make/longjmp-across-rust/main.rs | 40 +++++++++++++ 4 files changed, 127 insertions(+), 6 deletions(-) create mode 100644 src/test/run-make/longjmp-across-rust/Makefile create mode 100644 src/test/run-make/longjmp-across-rust/foo.c create mode 100644 src/test/run-make/longjmp-across-rust/main.rs diff --git a/src/librustc_trans/mir/mod.rs b/src/librustc_trans/mir/mod.rs index da01592d9118a..99e3a59e2c4f4 100644 --- a/src/librustc_trans/mir/mod.rs +++ b/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, block_bxs: &IndexVec) @@ -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() } diff --git a/src/test/run-make/longjmp-across-rust/Makefile b/src/test/run-make/longjmp-across-rust/Makefile new file mode 100644 index 0000000000000..9d71ed8fcf3ab --- /dev/null +++ b/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) diff --git a/src/test/run-make/longjmp-across-rust/foo.c b/src/test/run-make/longjmp-across-rust/foo.c new file mode 100644 index 0000000000000..eb9939576741b --- /dev/null +++ b/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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#include +#include + +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); +} diff --git a/src/test/run-make/longjmp-across-rust/main.rs b/src/test/run-make/longjmp-across-rust/main.rs new file mode 100644 index 0000000000000..c420473a560eb --- /dev/null +++ b/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 or the MIT license +// , 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(); + } +}