fix translation of terminators in MSVC cleanup blocks

I have taken a fairly brute force approach here: for each successor, I
generate a basic block that contains a `cleanupret` to that block. It
is ugly but it works.

It might have been possible to play with phis, but there are rumours that
there must be a 1-1 mapping between cleanup pad entries and exits, which
is not the case in MIR.

Author: arielb1

src/librustc_trans/mir/block.rs

@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use llvm::{self, BasicBlockRef, ValueRef, OperandBundleDef};
+use llvm::{self, ValueRef, OperandBundleDef};
 use rustc::ty;
 use rustc::mir::repr as mir;
 use abi::{Abi, FnType, ArgType};
@@ -34,22 +34,38 @@ use super::operand::OperandValue::{self, FatPtr, Immediate, Ref};
 
 impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
     pub fn trans_block(&mut self, bb: mir::BasicBlock) {
-        debug!("trans_block({:?})", bb);
-
         let mut bcx = self.bcx(bb);
         let mir = self.mir.clone();
         let data = mir.basic_block_data(bb);
 
+        debug!("trans_block({:?}={:?})", bb, data);
+
         // 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);
+        let funclet_br = |this: &Self, bcx: BlockAndBuilder, bb: mir::BasicBlock| {
+            if let Some(cp) = cleanup_pad {
+                bcx.cleanup_ret(cp, Some(this.blocks[bb.index()].llbb));
+            } else {
+                bcx.br(this.blocks[bb.index()].llbb);
+            }
+        };
+        let llblock = |this: &mut Self, target: mir::BasicBlock| {
+            let lltarget = this.blocks[target.index()].llbb;
+
+            if let Some(cp) = cleanup_pad {
+                debug!("llblock: creating cleanup trampoline for {:?}", target);
+                let name = &format!("{:?}_cleanup_trampoline_{:?}", bb, target);
+                let block = this.fcx.new_block(name, None);
+                let bcx = block.build();
+                bcx.cleanup_ret(cp, Some(lltarget));
+                block.llbb
+            } else {
+                lltarget
+            }
         };
 
         for statement in &data.statements {
@@ -78,13 +94,14 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
             }
 
             mir::TerminatorKind::Goto { target } => {
-                funclet_br(bcx, self.llblock(target));
+                funclet_br(self, bcx, target);
             }
 
             mir::TerminatorKind::If { ref cond, targets: (true_bb, false_bb) } => {
                 let cond = self.trans_operand(&bcx, cond);
-                let lltrue = self.llblock(true_bb);
-                let llfalse = self.llblock(false_bb);
+
+                let lltrue = llblock(self, true_bb);
+                let llfalse = llblock(self, false_bb);
                 bcx.cond_br(cond.immediate(), lltrue, llfalse);
             }
 
@@ -106,18 +123,18 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
                     // code. This is especially helpful in cases like an if-let on a huge enum.
                     // Note: This optimization is only valid for exhaustive matches.
                     Some((&&bb, &c)) if c > targets.len() / 2 => {
-                        (Some(bb), self.blocks[bb.index()])
+                        (Some(bb), llblock(self, bb))
                     }
                     // We're generating an exhaustive switch, so the else branch
                     // can't be hit.  Branching to an unreachable instruction
                     // lets LLVM know this
-                    _ => (None, self.unreachable_block())
+                    _ => (None, self.unreachable_block().llbb)
                 };
-                let switch = bcx.switch(discr, default_blk.llbb, targets.len());
+                let switch = bcx.switch(discr, default_blk, targets.len());
                 assert_eq!(adt_def.variants.len(), targets.len());
                 for (adt_variant, &target) in adt_def.variants.iter().zip(targets) {
                     if default_bb != Some(target) {
-                        let llbb = self.llblock(target);
+                        let llbb = llblock(self, target);
                         let llval = bcx.with_block(|bcx| adt::trans_case(
                                 bcx, &repr, Disr::from(adt_variant.disr_val)));
                         build::AddCase(switch, llval, llbb)
@@ -129,10 +146,10 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
                 let (otherwise, targets) = targets.split_last().unwrap();
                 let discr = bcx.load(self.trans_lvalue(&bcx, discr).llval);
                 let discr = bcx.with_block(|bcx| base::to_immediate(bcx, discr, switch_ty));
-                let switch = bcx.switch(discr, self.llblock(*otherwise), values.len());
+                let switch = bcx.switch(discr, llblock(self, *otherwise), values.len());
                 for (value, target) in values.iter().zip(targets) {
                     let val = Const::from_constval(bcx.ccx(), value.clone(), switch_ty);
-                    let llbb = self.llblock(*target);
+                    let llbb = llblock(self, *target);
                     build::AddCase(switch, val.llval, llbb)
                 }
             }
@@ -148,7 +165,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) {
-                    funclet_br(bcx, self.llblock(target));
+                    funclet_br(self, bcx, target);
                     return;
                 }
                 let drop_fn = glue::get_drop_glue(bcx.ccx(), ty);
@@ -163,15 +180,12 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
                     let unwind = self.make_landing_pad(uwbcx);
                     bcx.invoke(drop_fn,
                                &[llvalue],
-                               self.llblock(target),
+                               self.blocks[target.index()].llbb,
                                unwind.llbb(),
                                cleanup_bundle.as_ref());
-                    self.bcx(target).at_start(|bcx| {
-                        debug_loc.apply_to_bcx(bcx);
-                    });
                 } else {
                     bcx.call(drop_fn, &[llvalue], cleanup_bundle.as_ref());
-                    funclet_br(bcx, self.llblock(target));
+                    funclet_br(self, bcx, target);
                 }
             }
 
@@ -213,7 +227,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
                     let llptr = self.trans_operand(&bcx, &args[0]).immediate();
                     let val = self.trans_operand(&bcx, &args[1]);
                     self.store_operand(&bcx, llptr, val);
-                    funclet_br(bcx, self.llblock(target));
+                    funclet_br(self, bcx, target);
                     return;
                 }
 
@@ -223,7 +237,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
                         this.trans_transmute(&bcx, &args[0], dest);
                     });
 
-                    funclet_br(bcx, self.llblock(target));
+                    funclet_br(self, bcx, target);
                     return;
                 }
 
@@ -328,7 +342,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
                         }
 
                         if let Some((_, target)) = *destination {
-                            funclet_br(bcx, self.llblock(target));
+                            funclet_br(self, bcx, target);
                         } else {
                             // trans_intrinsic_call already used Unreachable.
                             // bcx.unreachable();
@@ -376,9 +390,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
                             ty: sig.output.unwrap()
                         };
                         self.store_return(&bcx, ret_dest, fn_ty.ret, op);
-                        funclet_br(bcx, self.llblock(target));
+                        funclet_br(self, bcx, target);
                     } else {
-                        // no need to drop args, because the call never returns
                         bcx.unreachable();
                     }
                 }
@@ -581,10 +594,6 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
         self.blocks[bb.index()].build()
     }
 
-    pub fn llblock(&self, bb: mir::BasicBlock) -> BasicBlockRef {
-        self.blocks[bb.index()].llbb
-    }
-
     fn make_return_dest(&mut self, bcx: &BlockAndBuilder<'bcx, 'tcx>,
                         dest: &mir::Lvalue<'tcx>, fn_ret_ty: &ArgType,
                         llargs: &mut Vec<ValueRef>, is_intrinsic: bool) -> ReturnDest {

src/librustc_trans/mir/mod.rs

@@ -228,8 +228,7 @@ pub fn trans_mir<'blk, 'tcx: 'blk>(fcx: &'blk FunctionContext<'blk, 'tcx>) {
         let block = BasicBlock(block.llbb);
         // Unreachable block
         if !visited.contains(bb.index()) {
-            block.delete();
-        } else if block.pred_iter().count() == 0 {
+            debug!("trans_mir: block {:?} was not visited", bb);
             block.delete();
         }
     }

src/test/run-fail/issue-30380.rs

@@ -8,6 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+#![feature(rustc_attrs)]
+
 // check that panics in destructors during assignment do not leave
 // destroyed values lying around for other destructors to observe.
 
@@ -33,6 +35,7 @@ impl<'a> Drop for Observer<'a> {
     }
 }
 
+#[rustc_mir]
 fn foo(b: &mut Observer) {
     *b.0 = FilledOnDrop(1);
 }

src/test/run-pass/dynamic-drop.rs

@@ -8,12 +8,23 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::cell::RefCell;
+#![feature(rustc_attrs)]
+
+use std::cell::{Cell, RefCell};
+use std::panic;
+use std::usize;
+
+struct InjectedFailure;
 
 struct Allocator {
     data: RefCell<Vec<bool>>,
+    failing_op: usize,
+    cur_ops: Cell<usize>,
 }
 
+impl panic::UnwindSafe for Allocator {}
+impl panic::RefUnwindSafe for Allocator {}
+
 impl Drop for Allocator {
     fn drop(&mut self) {
         let data = self.data.borrow();
@@ -24,8 +35,20 @@ impl Drop for Allocator {
 }
 
 impl Allocator {
-    fn new() -> Self { Allocator { data: RefCell::new(vec![]) } }
+    fn new(failing_op: usize) -> Self {
+        Allocator {
+            failing_op: failing_op,
+            cur_ops: Cell::new(0),
+            data: RefCell::new(vec![])
+        }
+    }
     fn alloc(&self) -> Ptr {
+        self.cur_ops.set(self.cur_ops.get() + 1);
+
+        if self.cur_ops.get() == self.failing_op {
+            panic!(InjectedFailure);
+        }
+
         let mut data = self.data.borrow_mut();
         let addr = data.len();
         data.push(true);
@@ -42,25 +65,34 @@ impl<'a> Drop for Ptr<'a> {
             }
             ref mut d => *d = false
         }
+
+        self.1.cur_ops.set(self.1.cur_ops.get()+1);
+
+        if self.1.cur_ops.get() == self.1.failing_op {
+            panic!(InjectedFailure);
+        }
     }
 }
 
+#[rustc_mir]
 fn dynamic_init(a: &Allocator, c: bool) {
     let _x;
     if c {
         _x = Some(a.alloc());
     }
 }
 
-fn dynamic_drop(a: &Allocator, c: bool) -> Option<Ptr> {
+#[rustc_mir]
+fn dynamic_drop(a: &Allocator, c: bool) {
     let x = a.alloc();
     if c {
         Some(x)
     } else {
         None
-    }
+    };
 }
 
+#[rustc_mir]
 fn assignment2(a: &Allocator, c0: bool, c1: bool) {
     let mut _v = a.alloc();
     let mut _w = a.alloc();
@@ -73,6 +105,7 @@ fn assignment2(a: &Allocator, c0: bool, c1: bool) {
     }
 }
 
+#[rustc_mir]
 fn assignment1(a: &Allocator, c0: bool) {
     let mut _v = a.alloc();
     let mut _w = a.alloc();
@@ -82,19 +115,42 @@ fn assignment1(a: &Allocator, c0: bool) {
     _v = _w;
 }
 
+fn run_test<F>(mut f: F)
+    where F: FnMut(&Allocator)
+{
+    let first_alloc = Allocator::new(usize::MAX);
+    f(&first_alloc);
+
+    for failing_op in 1..first_alloc.cur_ops.get()+1 {
+        let alloc = Allocator::new(failing_op);
+        let alloc = &alloc;
+        let f = panic::AssertUnwindSafe(&mut f);
+        let result = panic::catch_unwind(move || {
+            f.0(alloc);
+        });
+        match result {
+            Ok(..) => panic!("test executed {} ops but now {}",
+                             first_alloc.cur_ops.get(), alloc.cur_ops.get()),
+            Err(e) => {
+                if e.downcast_ref::<InjectedFailure>().is_none() {
+                    panic::resume_unwind(e);
+                }
+            }
+        }
+    }
+}
 
 fn main() {
-    let a = Allocator::new();
-    dynamic_init(&a, false);
-    dynamic_init(&a, true);
-    dynamic_drop(&a, false);
-    dynamic_drop(&a, true);
-
-    assignment2(&a, false, false);
-    assignment2(&a, false, true);
-    assignment2(&a, true, false);
-    assignment2(&a, true, true);
-
-    assignment1(&a, false);
-    assignment1(&a, true);
+    run_test(|a| dynamic_init(a, false));
+    run_test(|a| dynamic_init(a, true));
+    run_test(|a| dynamic_drop(a, false));
+    run_test(|a| dynamic_drop(a, true));
+
+    run_test(|a| assignment2(a, false, false));
+    run_test(|a| assignment2(a, false, true));
+    run_test(|a| assignment2(a, true, false));
+    run_test(|a| assignment2(a, true, true));
+
+    run_test(|a| assignment1(a, false));
+    run_test(|a| assignment1(a, true));
 }