Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustc: Tweak funclet cleanups of ffi functions #48572

Merged
merged 1 commit into from
Mar 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to understand how this test works. According to earlier comment, longjmp will still run destructors on MSVC. As such, A's destructor will run, causing a panic, but since we're in unwind mode, this will be a double unwind, causing an abort for that reason, or...?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes that was a mistake on my part, this test was originally included in a PR where it was fixed, but I changed strategies at the last minute.

let _a = A;
unsafe {
test_end();
}
}