Skip to content

Commit

Permalink
Rollup merge of rust-lang#48572 - alexcrichton:noexcept-msvc2, r=eddyb
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Manishearth committed Mar 1, 2018
2 parents 75b8c10 + 804666f commit 8025e15
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 6 deletions.
60 changes: 54 additions & 6 deletions src/librustc_trans/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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>)
Expand All @@ -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()
}

Expand Down
5 changes: 5 additions & 0 deletions src/test/run-make/longjmp-across-rust/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-include ../tools.mk

all: $(call NATIVE_STATICLIB,foo)
$(RUSTC) main.rs
$(call RUN,main)
28 changes: 28 additions & 0 deletions src/test/run-make/longjmp-across-rust/foo.c
Original file line number Diff line number Diff line change
@@ -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);
}
40 changes: 40 additions & 0 deletions src/test/run-make/longjmp-across-rust/main.rs
Original file line number Diff line number Diff line change
@@ -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();
}
}

0 comments on commit 8025e15

Please sign in to comment.