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

[MIR] MSVC SEH unwind handling #31600

Merged
merged 2 commits into from
Feb 18, 2016
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
3 changes: 2 additions & 1 deletion src/librustc_mir/mir_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ extern crate rustc_front;
use build;
use graphviz;
use pretty;
use transform::simplify_cfg;
use transform::{simplify_cfg, no_landing_pads};
use rustc::dep_graph::DepNode;
use rustc::mir::repr::Mir;
use hair::cx::Cx;
Expand Down Expand Up @@ -148,6 +148,7 @@ impl<'a, 'm, 'tcx> Visitor<'tcx> for InnerDump<'a,'m,'tcx> {

match build_mir(Cx::new(&infcx), implicit_arg_tys, id, span, decl, body) {
Ok(mut mir) => {
no_landing_pads::NoLandingPads.run_on_mir(&mut mir, self.tcx);
simplify_cfg::SimplifyCfg::new().run_on_mir(&mut mir, self.tcx);

let meta_item_list = self.attr
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/transform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@

pub mod simplify_cfg;
pub mod erase_regions;
pub mod no_landing_pads;
mod util;
49 changes: 49 additions & 0 deletions src/librustc_mir/transform/no_landing_pads.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2015 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.

//! This pass removes the unwind branch of all the terminators when the no-landing-pads option is
//! specified.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fairly nifty btw. =)


use rustc::middle::ty;
use rustc::mir::repr::*;
use rustc::mir::visit::MutVisitor;
use rustc::mir::transform::MirPass;

pub struct NoLandingPads;

impl<'tcx> MutVisitor<'tcx> for NoLandingPads {
fn visit_terminator(&mut self, bb: BasicBlock, terminator: &mut Terminator<'tcx>) {
match *terminator {
Terminator::Goto { .. } |
Terminator::Resume |
Terminator::Return |
Terminator::If { .. } |
Terminator::Switch { .. } |
Terminator::SwitchInt { .. } => {
/* nothing to do */
},
Terminator::Drop { ref mut unwind, .. } => {
unwind.take();
},
Terminator::Call { ref mut cleanup, .. } => {
cleanup.take();
},
}
self.super_terminator(bb, terminator);
}
}

impl MirPass for NoLandingPads {
fn run_on_mir<'tcx>(&mut self, mir: &mut Mir<'tcx>, tcx: &ty::ctxt<'tcx>) {
if tcx.sess.no_landing_pads() {
self.visit_mir(mir);
}
}
}
4 changes: 4 additions & 0 deletions src/librustc_trans/trans/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,10 @@ impl<'blk, 'tcx> BlockAndBuilder<'blk, 'tcx> {
{
self.bcx.monomorphize(value)
}

pub fn set_lpad(&self, lpad: Option<LandingPad>) {
self.bcx.lpad.set(lpad.map(|p| &*self.fcx().lpad_arena.alloc(p)))
}
}

impl<'blk, 'tcx> Deref for BlockAndBuilder<'blk, 'tcx> {
Expand Down
110 changes: 79 additions & 31 deletions src/librustc_trans/trans/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use llvm::{BasicBlockRef, ValueRef};
use llvm::{BasicBlockRef, ValueRef, OperandBundleDef};
use rustc::middle::ty;
use rustc::mir::repr as mir;
use syntax::abi::Abi;
Expand All @@ -34,15 +34,40 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
let mut bcx = self.bcx(bb);
let data = self.mir.basic_block_data(bb);

// MSVC SEH bits
let (cleanup_pad, cleanup_bundle) = if let Some((cp, cb)) = self.make_cleanup_pad(bb) {
(Some(cp), Some(cb))
} else {
(None, None)
};
let funclet_br = |bcx: BlockAndBuilder, llbb: BasicBlockRef| if let Some(cp) = cleanup_pad {
bcx.cleanup_ret(cp, Some(llbb));
} else {
bcx.br(llbb);
};

for statement in &data.statements {
bcx = self.trans_statement(bcx, statement);
}

debug!("trans_block: terminator: {:?}", data.terminator());

match *data.terminator() {
mir::Terminator::Resume => {
if let Some(cleanup_pad) = cleanup_pad {
bcx.cleanup_ret(cleanup_pad, None);
} else {
let ps = self.get_personality_slot(&bcx);
let lp = bcx.load(ps);
bcx.with_block(|bcx| {
base::call_lifetime_end(bcx, ps);
base::trans_unwind_resume(bcx, lp);
});
}
}

mir::Terminator::Goto { target } => {
bcx.br(self.llblock(target));
funclet_br(bcx, self.llblock(target));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I expected to have a distinct terminator in MIR for "cleanup-ret", rather than repurposing Goto -- but maybe that's silly, and Goto is perfectly fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that would be for a worse – at least all the passes would have to duplicate their logic for both regular Goto and the-special-case-cleanup-ret (which would most likely end up becoming an optional target in Resume terminator).

Copy link
Contributor

Choose a reason for hiding this comment

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

@nagisa

I think that would be for a worse.

Yes, ok. I was thinking the same. But it does mean there are extra restrictions to keep in mind about Goto terminators in unwind blocks. Should be OK.

Copy link
Member Author

Choose a reason for hiding this comment

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

On Feb 18, 2016 3:14 AM, "Niko Matsakis" notifications@github.com wrote:

In src/librustc_trans/trans/mir/block.rs:

         mir::Terminator::Goto { target } => {
  •            bcx.br(self.llblock(target));
    
  •            funclet_br(bcx, self.llblock(target));
    

@nagisa

I think that would be for a worse.

Yes, ok. I was thinking the same. But it does mean there are extra
restrictions to keep in mind about Goto terminators in unwind blocks.
Should be OK.

I feel like these restrictions could be enforced by a pass similar to
typeck. Actually we could just rename typeck to sanityck and throw various
validity checks into there.


Reply to this email directly or view it on GitHub.

Copy link
Contributor

Choose a reason for hiding this comment

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

On Thu, Feb 18, 2016 at 01:42:31AM -0800, Simonas Kazlauskas wrote:

I feel like these restrictions could be enforced by a pass similar to
typeck. Actually we could just rename typeck to sanityck and throw various
validity checks into there.

I would prefer to keep type checking distinct from sanity
checking
, but yes. (In particular, we will need to do actual
type-checking when we move to non-lexical lifetimes.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I hope I can get MIR typeck to a stage that you could drive region inference from it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@arielb1

I hope I can get MIR typeck to a stage that you could drive region inference from it.

That's my plan :)

}

mir::Terminator::If { ref cond, targets: (true_bb, false_bb) } => {
Expand Down Expand Up @@ -85,19 +110,10 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
}
}

mir::Terminator::Resume => {
let ps = self.get_personality_slot(&bcx);
let lp = bcx.load(ps);
bcx.with_block(|bcx| {
base::call_lifetime_end(bcx, ps);
base::trans_unwind_resume(bcx, lp);
});
}

mir::Terminator::Return => {
let return_ty = bcx.monomorphize(&self.mir.return_ty);
bcx.with_block(|bcx| {
base::build_return_block(bcx.fcx, bcx, return_ty, DebugLoc::None);
base::build_return_block(self.fcx, bcx, return_ty, DebugLoc::None);
})
}

Expand All @@ -106,7 +122,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
let ty = lvalue.ty.to_ty(bcx.tcx());
// Double check for necessity to drop
if !glue::type_needs_drop(bcx.tcx(), ty) {
bcx.br(self.llblock(target));
funclet_br(bcx, self.llblock(target));
return;
}
let drop_fn = glue::get_drop_glue(bcx.ccx(), ty);
Expand All @@ -123,11 +139,11 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
&[llvalue],
self.llblock(target),
unwind.llbb(),
None,
cleanup_bundle.as_ref(),
None);
} else {
bcx.call(drop_fn, &[llvalue], None, None);
bcx.br(self.llblock(target));
bcx.call(drop_fn, &[llvalue], cleanup_bundle.as_ref(), None);
funclet_br(bcx, self.llblock(target));
}
}

Expand Down Expand Up @@ -180,34 +196,33 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
}
}

let avoid_invoke = bcx.with_block(|bcx| base::avoid_invoke(bcx));
// Many different ways to call a function handled here
match (is_foreign, avoid_invoke, cleanup, destination) {
match (is_foreign, cleanup, destination) {
// The two cases below are the only ones to use LLVM’s `invoke`.
(false, false, &Some(cleanup), &None) => {
(false, &Some(cleanup), &None) => {
let cleanup = self.bcx(cleanup);
let landingpad = self.make_landing_pad(cleanup);
let unreachable_blk = self.unreachable_block();
bcx.invoke(callee.immediate(),
&llargs[..],
unreachable_blk.llbb,
landingpad.llbb(),
None,
cleanup_bundle.as_ref(),
Some(attrs));
},
(false, false, &Some(cleanup), &Some((_, success))) => {
(false, &Some(cleanup), &Some((_, success))) => {
let cleanup = self.bcx(cleanup);
let landingpad = self.make_landing_pad(cleanup);
let (target, postinvoke) = if must_copy_dest {
(bcx.fcx().new_block("", None).build(), Some(self.bcx(success)))
(self.fcx.new_block("", None).build(), Some(self.bcx(success)))
} else {
(self.bcx(success), None)
};
let invokeret = bcx.invoke(callee.immediate(),
&llargs[..],
target.llbb(),
landingpad.llbb(),
None,
cleanup_bundle.as_ref(),
Some(attrs));
if let Some(postinvoketarget) = postinvoke {
// We translate the copy into a temporary block. The temporary block is
Expand Down Expand Up @@ -242,14 +257,17 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
target.br(postinvoketarget.llbb());
}
},
(false, _, _, &None) => {
bcx.call(callee.immediate(), &llargs[..], None, Some(attrs));
(false, _, &None) => {
bcx.call(callee.immediate(),
&llargs[..],
cleanup_bundle.as_ref(),
Some(attrs));
bcx.unreachable();
}
(false, _, _, &Some((_, target))) => {
(false, _, &Some((_, target))) => {
let llret = bcx.call(callee.immediate(),
&llargs[..],
None,
cleanup_bundle.as_ref(),
Some(attrs));
if must_copy_dest {
let (ret_dest, ret_ty) = ret_dest_ty
Expand All @@ -258,10 +276,10 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
base::store_ty(bcx, llret, ret_dest.llval, ret_ty);
});
}
bcx.br(self.llblock(target));
funclet_br(bcx, self.llblock(target));
}
// Foreign functions
(true, _, _, destination) => {
(true, _, destination) => {
let (dest, _) = ret_dest_ty
.expect("return destination is not set");
bcx = bcx.map_block(|bcx| {
Expand All @@ -274,7 +292,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
debugloc)
});
if let Some((_, target)) = *destination {
bcx.br(self.llblock(target));
funclet_br(bcx, self.llblock(target));
}
},
}
Expand All @@ -297,11 +315,16 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
}
}

/// Create a landingpad wrapper around the given Block.
///
/// No-op in MSVC SEH scheme.
fn make_landing_pad(&mut self,
cleanup: BlockAndBuilder<'bcx, 'tcx>)
-> BlockAndBuilder<'bcx, 'tcx>
{
// FIXME(#30941) this doesn't handle msvc-style exceptions
if base::wants_msvc_seh(cleanup.sess()) {
return cleanup;
}
let bcx = self.fcx.new_block("cleanup", None).build();
let ccx = bcx.ccx();
let llpersonality = self.fcx.eh_personality();
Expand All @@ -314,6 +337,31 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
bcx
}

/// Create prologue cleanuppad instruction under MSVC SEH handling scheme.
///
/// Also handles setting some state for the original trans and creating an operand bundle for
/// function calls.
fn make_cleanup_pad(&mut self, bb: mir::BasicBlock) -> Option<(ValueRef, OperandBundleDef)> {
let bcx = self.bcx(bb);
let data = self.mir.basic_block_data(bb);
let use_funclets = base::wants_msvc_seh(bcx.sess()) && data.is_cleanup;
let cleanup_pad = if use_funclets {
bcx.set_personality_fn(self.fcx.eh_personality());
Some(bcx.cleanup_pad(None, &[]))
} else {
None
};
// Set the landingpad global-state for old translator, so it knows about the SEH used.
bcx.set_lpad(if let Some(cleanup_pad) = cleanup_pad {
Some(common::LandingPad::msvc(cleanup_pad))
} else if data.is_cleanup {
Some(common::LandingPad::gnu())
} else {
None
});
cleanup_pad.map(|f| (f, OperandBundleDef::new("funclet", &[f])))
}

fn unreachable_block(&mut self) -> Block<'bcx, 'tcx> {
self.unreachable_block.unwrap_or_else(|| {
let bl = self.fcx.new_block("unreachable", None);
Expand Down
1 change: 0 additions & 1 deletion src/test/run-fail/mir_drop_panics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
// except according to those terms.
#![feature(rustc_attrs)]

// ignore-msvc: FIXME(#30941)
// error-pattern:panic 1
// error-pattern:drop 2
use std::io::{self, Write};
Expand Down
1 change: 0 additions & 1 deletion src/test/run-fail/mir_trans_calls_converging_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#![feature(rustc_attrs)]

// ignore-msvc: FIXME(#30941)
// error-pattern:converging_fn called
// error-pattern:0 dropped
// error-pattern:exit
Expand Down
1 change: 0 additions & 1 deletion src/test/run-fail/mir_trans_calls_converging_drops_2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#![feature(rustc_attrs)]

// ignore-msvc: FIXME(#30941)
// error-pattern:complex called
// error-pattern:dropped
// error-pattern:exit
Expand Down
1 change: 0 additions & 1 deletion src/test/run-fail/mir_trans_calls_diverging_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#![feature(rustc_attrs)]

// ignore-msvc: FIXME(#30941)
// error-pattern:diverging_fn called
// error-pattern:0 dropped

Expand Down