fix translation of MSVC funclets that loop to their own start

arielb1 · arielb1 · commit ee982d4355a1 · 2017-05-28T12:00:03.000+03:00
diff --git a/src/librustc_data_structures/indexed_vec.rs b/src/librustc_data_structures/indexed_vec.rs
@@ -212,6 +212,13 @@ impl<I: Idx, T> IndexMut<I> for IndexVec<I, T> {
     }
 }
 
+impl<I: Idx, T> Default for IndexVec<I, T> {
+    #[inline]
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl<I: Idx, T> Extend<T> for IndexVec<I, T> {
     #[inline]
     fn extend<J: IntoIterator<Item = T>>(&mut self, iter: J) {
diff --git a/src/librustc_trans/common.rs b/src/librustc_trans/common.rs
@@ -191,15 +191,6 @@ impl Funclet {
     }
 }
 
-impl Clone for Funclet {
-    fn clone(&self) -> Funclet {
-        Funclet {
-            cleanuppad: self.cleanuppad,
-            operand: OperandBundleDef::new("funclet", &[self.cleanuppad]),
-        }
-    }
-}
-
 pub fn val_ty(v: ValueRef) -> Type {
     unsafe {
         Type::from_ref(llvm::LLVMTypeOf(v))
diff --git a/src/librustc_trans/mir/analyze.rs b/src/librustc_trans/mir/analyze.rs
@@ -197,6 +197,16 @@ pub enum CleanupKind {
     Internal { funclet: mir::BasicBlock }
 }
 
+impl CleanupKind {
+    pub fn funclet_bb(self, for_bb: mir::BasicBlock) -> Option<mir::BasicBlock> {
+        match self {
+            CleanupKind::NotCleanup => None,
+            CleanupKind::Funclet => Some(for_bb),
+            CleanupKind::Internal { funclet } => Some(funclet),
+        }
+    }
+}
+
 pub fn cleanup_kinds<'a, 'tcx>(mir: &mir::Mir<'tcx>) -> IndexVec<mir::BasicBlock, CleanupKind> {
     fn discover_masters<'tcx>(result: &mut IndexVec<mir::BasicBlock, CleanupKind>,
                               mir: &mir::Mir<'tcx>) {
diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs
@@ -19,104 +19,96 @@ use adt;
 use base::{self, Lifetime};
 use callee;
 use builder::Builder;
-use common::{self, Funclet};
-use common::{C_bool, C_str_slice, C_struct, C_u32, C_undef};
+use common::{self, C_bool, C_str_slice, C_struct, C_u32, C_undef};
 use consts;
 use machine::llalign_of_min;
 use meth;
 use monomorphize;
 use type_of;
 use type_::Type;
 
-use rustc_data_structures::indexed_vec::IndexVec;
 use syntax::symbol::Symbol;
 
 use std::cmp;
 
 use super::{MirContext, LocalRef};
-use super::analyze::CleanupKind;
 use super::constant::Const;
 use super::lvalue::{Alignment, LvalueRef};
 use super::operand::OperandRef;
 use super::operand::OperandValue::{Pair, Ref, Immediate};
 
 impl<'a, 'tcx> MirContext<'a, 'tcx> {
-    pub fn trans_block(&mut self, bb: mir::BasicBlock,
-        funclets: &IndexVec<mir::BasicBlock, Option<Funclet>>) {
+    pub fn trans_block(&mut self, bb: mir::BasicBlock) {
         let mut bcx = self.get_builder(bb);
         let data = &self.mir[bb];
 
         debug!("trans_block({:?}={:?})", bb, data);
 
-        let funclet = match self.cleanup_kinds[bb] {
-            CleanupKind::Internal { funclet } => funclets[funclet].as_ref(),
-            _ => funclets[bb].as_ref(),
-        };
-
         for statement in &data.statements {
             bcx = self.trans_statement(bcx, statement);
         }
 
-        self.trans_terminator(bcx, bb, data.terminator(), funclet);
+        self.trans_terminator(bcx, bb, data.terminator());
     }
 
     fn trans_terminator(&mut self,
                         mut bcx: Builder<'a, 'tcx>,
                         bb: mir::BasicBlock,
-                        terminator: &mir::Terminator<'tcx>,
-                        funclet: Option<&Funclet>)
+                        terminator: &mir::Terminator<'tcx>)
     {
         debug!("trans_terminator: {:?}", terminator);
 
         // Create the cleanup bundle, if needed.
         let tcx = bcx.tcx();
+        let span = terminator.source_info.span;
+        let funclet_bb = self.cleanup_kinds[bb].funclet_bb(bb);
+        let funclet = funclet_bb.and_then(|funclet_bb| self.funclets[funclet_bb].as_ref());
+
         let cleanup_pad = funclet.map(|lp| lp.cleanuppad());
         let cleanup_bundle = funclet.map(|l| l.bundle());
 
-        let funclet_br = |this: &Self, bcx: Builder, bb: mir::BasicBlock| {
-            let lltarget = this.blocks[bb];
-            if let Some(cp) = cleanup_pad {
-                match this.cleanup_kinds[bb] {
-                    CleanupKind::Funclet => {
-                        // micro-optimization: generate a `ret` rather than a jump
-                        // to a return block
-                        bcx.cleanup_ret(cp, Some(lltarget));
-                    }
-                    CleanupKind::Internal { .. } => bcx.br(lltarget),
-                    CleanupKind::NotCleanup => bug!("jump from cleanup bb to bb {:?}", bb)
+        let lltarget = |this: &mut Self, target: mir::BasicBlock| {
+            let lltarget = this.blocks[target];
+            let target_funclet = this.cleanup_kinds[target].funclet_bb(target);
+            match (funclet_bb, target_funclet) {
+                (None, None) => (lltarget, false),
+                (Some(f), Some(t_f))
+                    if f == t_f || !base::wants_msvc_seh(tcx.sess)
+                    => (lltarget, false),
+                (None, Some(_)) => {
+                    // jump *into* cleanup - need a landing pad if GNU
+                    (this.landing_pad_to(target), false)
+                }
+                (Some(_), None) => span_bug!(span, "{:?} - jump out of cleanup?", terminator),
+                (Some(_), Some(_)) => {
+                    (this.landing_pad_to(target), true)
                 }
-            } else {
-                bcx.br(lltarget);
             }
         };
 
         let llblock = |this: &mut Self, target: mir::BasicBlock| {
-            let lltarget = this.blocks[target];
-
-            if let Some(cp) = cleanup_pad {
-                match this.cleanup_kinds[target] {
-                    CleanupKind::Funclet => {
-                        // MSVC cross-funclet jump - need a trampoline
+            let (lltarget, is_cleanupret) = lltarget(this, target);
+            if is_cleanupret {
+                // MSVC cross-funclet jump - need a trampoline
+
+                debug!("llblock: creating cleanup trampoline for {:?}", target);
+                let name = &format!("{:?}_cleanup_trampoline_{:?}", bb, target);
+                let trampoline = this.new_block(name);
+                trampoline.cleanup_ret(cleanup_pad.unwrap(), Some(lltarget));
+                trampoline.llbb()
+            } else {
+                lltarget
+            }
+        };
 
-                        debug!("llblock: creating cleanup trampoline for {:?}", target);
-                        let name = &format!("{:?}_cleanup_trampoline_{:?}", bb, target);
-                        let trampoline = this.new_block(name);
-                        trampoline.cleanup_ret(cp, Some(lltarget));
-                        trampoline.llbb()
-                    }
-                    CleanupKind::Internal { .. } => lltarget,
-                    CleanupKind::NotCleanup =>
-                        bug!("jump from cleanup bb {:?} to bb {:?}", bb, target)
-                }
+        let funclet_br = |this: &mut Self, bcx: Builder, target: mir::BasicBlock| {
+            let (lltarget, is_cleanupret) = lltarget(this, target);
+            if is_cleanupret {
+                // micro-optimization: generate a `ret` rather than a jump
+                // to a trampoline.
+                bcx.cleanup_ret(cleanup_pad.unwrap(), Some(lltarget));
             } else {
-                if let (CleanupKind::NotCleanup, CleanupKind::Funclet) =
-                    (this.cleanup_kinds[bb], this.cleanup_kinds[target])
-                {
-                    // jump *into* cleanup - need a landing pad if GNU
-                    this.landing_pad_to(target)
-                } else {
-                    lltarget
-                }
+                bcx.br(lltarget);
             }
         };
 
@@ -168,7 +160,6 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
             }
         };
 
-        let span = terminator.source_info.span;
         self.set_debug_loc(&bcx, terminator.source_info);
         match terminator.kind {
             mir::TerminatorKind::Resume => {
@@ -752,7 +743,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
 
     fn landing_pad_uncached(&mut self, target_bb: BasicBlockRef) -> BasicBlockRef {
         if base::wants_msvc_seh(self.ccx.sess()) {
-            return target_bb;
+            span_bug!(self.mir.span, "landing pad was not inserted?")
         }
 
         let bcx = self.new_block("cleanup");
diff --git a/src/librustc_trans/mir/mod.rs b/src/librustc_trans/mir/mod.rs
@@ -69,6 +69,10 @@ pub struct MirContext<'a, 'tcx:'a> {
     /// The funclet status of each basic block
     cleanup_kinds: IndexVec<mir::BasicBlock, analyze::CleanupKind>,
 
+    /// When targeting MSVC, this stores the cleanup info for each funclet
+    /// BB. This is initialized as we compute the funclets' head block in RPO.
+    funclets: &'a IndexVec<mir::BasicBlock, Option<Funclet>>,
+
     /// This stores the landing-pad block for a given BB, computed lazily on GNU
     /// and eagerly on MSVC.
     landing_pads: IndexVec<mir::BasicBlock, Option<BasicBlockRef>>,
@@ -202,8 +206,11 @@ pub fn trans_mir<'a, 'tcx: 'a>(
         debuginfo::create_function_debug_context(ccx, instance, sig, llfn, mir);
     let bcx = Builder::new_block(ccx, llfn, "start");
 
-    let cleanup_kinds = analyze::cleanup_kinds(&mir);
+    if mir.basic_blocks().iter().any(|bb| bb.is_cleanup) {
+        bcx.set_personality_fn(ccx.eh_personality());
+    }
 
+    let cleanup_kinds = analyze::cleanup_kinds(&mir);
     // Allocate a `Block` for every basic block, except
     // the start block, if nothing loops back to it.
     let reentrant_start_block = !mir.predecessors_for(mir::START_BLOCK).is_empty();
@@ -218,6 +225,7 @@ pub fn trans_mir<'a, 'tcx: 'a>(
 
     // Compute debuginfo scopes from MIR scopes.
     let scopes = debuginfo::create_mir_scopes(ccx, mir, &debug_context);
+    let (landing_pads, funclets) = create_funclets(&bcx, &cleanup_kinds, &block_bcxs);
 
     let mut mircx = MirContext {
         mir: mir,
@@ -228,7 +236,8 @@ pub fn trans_mir<'a, 'tcx: 'a>(
         blocks: block_bcxs,
         unreachable_block: None,
         cleanup_kinds: cleanup_kinds,
-        landing_pads: IndexVec::from_elem(None, mir.basic_blocks()),
+        landing_pads: landing_pads,
+        funclets: &funclets,
         scopes: scopes,
         locals: IndexVec::new(),
         debug_context: debug_context,
@@ -306,28 +315,13 @@ pub fn trans_mir<'a, 'tcx: 'a>(
     // emitting should be enabled.
     debuginfo::start_emitting_source_locations(&mircx.debug_context);
 
-    let funclets: IndexVec<mir::BasicBlock, Option<Funclet>> =
-    mircx.cleanup_kinds.iter_enumerated().map(|(bb, cleanup_kind)| {
-        if let CleanupKind::Funclet = *cleanup_kind {
-            let bcx = mircx.get_builder(bb);
-            unsafe {
-                llvm::LLVMSetPersonalityFn(mircx.llfn, mircx.ccx.eh_personality());
-            }
-            if base::wants_msvc_seh(ccx.sess()) {
-                return Some(Funclet::new(bcx.cleanup_pad(None, &[])));
-            }
-        }
-
-        None
-    }).collect();
-
     let rpo = traversal::reverse_postorder(&mir);
     let mut visited = BitVector::new(mir.basic_blocks().len());
 
     // Translate the body of each block using reverse postorder
     for (bb, _) in rpo {
         visited.insert(bb.index());
-        mircx.trans_block(bb, &funclets);
+        mircx.trans_block(bb);
     }
 
     // Remove blocks that haven't been visited, or have no
@@ -343,6 +337,26 @@ pub fn trans_mir<'a, 'tcx: 'a>(
     }
 }
 
+fn create_funclets<'a, 'tcx>(
+    bcx: &Builder<'a, 'tcx>,
+    cleanup_kinds: &IndexVec<mir::BasicBlock, CleanupKind>,
+    block_bcxs: &IndexVec<mir::BasicBlock, BasicBlockRef>)
+    -> (IndexVec<mir::BasicBlock, Option<BasicBlockRef>>,
+        IndexVec<mir::BasicBlock, Option<Funclet>>)
+{
+    block_bcxs.iter_enumerated().zip(cleanup_kinds).map(|((bb, &llbb), cleanup_kind)| {
+        match *cleanup_kind {
+            CleanupKind::Funclet if base::wants_msvc_seh(bcx.sess()) => {
+                let cleanup_bcx = bcx.build_sibling_block(&format!("funclet_{:?}", bb));
+                let cleanup = cleanup_bcx.cleanup_pad(None, &[]);
+                cleanup_bcx.br(llbb);
+                (Some(cleanup_bcx.llbb()), Some(Funclet::new(cleanup)))
+            }
+            _ => (None, None)
+        }
+    }).unzip()
+}
+
 /// Produce, for each argument, a `ValueRef` pointing at the
 /// argument's value. As arguments are lvalues, these are always
 /// indirect.

Original file line number	Diff line number	Diff line change
`@@ -212,6 +212,13 @@ impl<I: Idx, T> IndexMut<I> for IndexVec<I, T> {`
`212`	`212`	`}`
`213`	`213`	`}`
`214`	`214`
	`215`	`+impl<I: Idx, T> Default for IndexVec<I, T> {`
	`216`	`+ #[inline]`
	`217`	`+ fn default() -> Self {`
	`218`	`+ Self::new()`
	`219`	`+ }`
	`220`	`+}`
	`221`	`+`
`215`	`222`	`impl<I: Idx, T> Extend<T> for IndexVec<I, T> {`
`216`	`223`	`#[inline]`
`217`	`224`	`fn extend<J: IntoIterator<Item = T>>(&mut self, iter: J) {`