fix tail call drop unwinding

Author: beepster4096

compiler/rustc_mir_build/src/build/expr/stmt.rs

@@ -122,7 +122,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
                     debug!("expr_into_dest: fn_span={:?}", fn_span);
 
-                    unpack!(block = this.break_for_tail_call(block));
+                    unpack!(block = this.break_for_tail_call(block, &args, source_info));
 
                     this.cfg.terminate(
                         block,

compiler/rustc_mir_build/src/build/scope.rs

@@ -725,30 +725,79 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     ///
     /// Unlike other kinds of early exits, tail calls do not go through the drop tree.
     /// Instead, all scheduled drops are immediately added to the CFG.
-    pub(crate) fn break_for_tail_call(&mut self, mut block: BasicBlock) -> BlockAnd<()> {
+    pub(crate) fn break_for_tail_call(
+        &mut self,
+        mut block: BasicBlock,
+        args: &[Operand<'tcx>],
+        source_info: SourceInfo,
+    ) -> BlockAnd<()> {
+        let arg_drops: Vec<_> = args
+            .iter()
+            .rev()
+            .filter_map(|arg| match arg {
+                Operand::Copy(_) => bug!("copy op in tail call args"),
+                Operand::Move(place) => {
+                    let local =
+                        place.as_local().unwrap_or_else(|| bug!("projection in tail call args"));
+
+                    Some(DropData { source_info, local, kind: DropKind::Value })
+                }
+                Operand::Constant(_) => None,
+            })
+            .collect();
+
+        let mut unwind_to = self.diverge_cleanup_target(
+            self.scopes.scopes.iter().rev().nth(1).unwrap().region_scope,
+            DUMMY_SP,
+        );
+        let unwind_drops = &mut self.scopes.unwind_drops;
+
         // the innermost scope contains only the destructors for the tail call arguments
         // we only want to drop these in case of a panic, so we skip it
         for scope in self.scopes.scopes[1..].iter().rev().skip(1) {
-            for drop in scope.drops.iter().rev() {
-                match drop.kind {
+            // FIXME(explicit_tail_calls) code duplication with `build_scope_drops`
+            for drop_data in scope.drops.iter().rev() {
+                let source_info = drop_data.source_info;
+                let local = drop_data.local;
+
+                match drop_data.kind {
                     DropKind::Value => {
-                        let target = self.cfg.start_new_block();
-                        let terminator = TerminatorKind::Drop {
-                            target,
-                            // The caller will handle this if needed.
-                            unwind: UnwindAction::Terminate,
-                            place: drop.local.into(),
-                            replace: false,
-                        };
-                        self.cfg.terminate(block, drop.source_info, terminator);
-                        block = target;
+                        // `unwind_to` should drop the value that we're about to
+                        // schedule. If dropping this value panics, then we continue
+                        // with the *next* value on the unwind path.
+                        debug_assert_eq!(unwind_drops.drops[unwind_to].0.local, drop_data.local);
+                        debug_assert_eq!(unwind_drops.drops[unwind_to].0.kind, drop_data.kind);
+                        unwind_to = unwind_drops.drops[unwind_to].1;
+
+                        let mut unwind_entry_point = unwind_to;
+
+                        // the tail call arguments must be dropped if any of these drops panic
+                        for drop in arg_drops.iter().copied() {
+                            unwind_entry_point = unwind_drops.add_drop(drop, unwind_entry_point);
+                        }
+
+                        unwind_drops.add_entry(block, unwind_entry_point);
+
+                        let next = self.cfg.start_new_block();
+                        self.cfg.terminate(
+                            block,
+                            source_info,
+                            TerminatorKind::Drop {
+                                place: local.into(),
+                                target: next,
+                                unwind: UnwindAction::Continue,
+                                replace: false,
+                            },
+                        );
+                        block = next;
                     }
                     DropKind::Storage => {
-                        let stmt = Statement {
-                            source_info: drop.source_info,
-                            kind: StatementKind::StorageDead(drop.local),
-                        };
-                        self.cfg.push(block, stmt);
+                        // Only temps and vars need their storage dead.
+                        assert!(local.index() > self.arg_count);
+                        self.cfg.push(
+                            block,
+                            Statement { source_info, kind: StatementKind::StorageDead(local) },
+                        );
                     }
                 }
             }

tests/mir-opt/tail_call_drops.f.ElaborateDrops.diff

@@ -54,22 +54,21 @@
       bb4: {
           StorageDead(_7);
           StorageDead(_6);
--         drop(_5) -> [return: bb5, unwind terminate];
-+         drop(_5) -> [return: bb5, unwind: bb13];
+          drop(_5) -> [return: bb5, unwind: bb10];
       }
 
       bb5: {
           StorageDead(_5);
--         drop(_4) -> [return: bb6, unwind terminate];
+-         drop(_4) -> [return: bb6, unwind: bb11];
 +         goto -> bb6;
       }
 
       bb6: {
 +         _8 = const false;
           StorageDead(_4);
           StorageDead(_3);
--         drop(_2) -> [return: bb7, unwind terminate];
-+         drop(_2) -> [return: bb7, unwind: bb13];
+-         drop(_2) -> [return: bb7, unwind continue];
++         drop(_2) -> [return: bb7, unwind: bb12];
       }
 
       bb7: {
@@ -100,7 +99,7 @@
 +     }
 + 
 +     bb13 (cleanup): {
-+         abort;
++         unreachable;
 +     }
 + 
 +     bb14 (cleanup): {

tests/mir-opt/tail_call_drops.f.built.after.mir

@@ -53,18 +53,18 @@ fn f() -> () {
     bb4: {
         StorageDead(_7);
         StorageDead(_6);
-        drop(_5) -> [return: bb5, unwind terminate];
+        drop(_5) -> [return: bb5, unwind: bb15];
     }
 
     bb5: {
         StorageDead(_5);
-        drop(_4) -> [return: bb6, unwind terminate];
+        drop(_4) -> [return: bb6, unwind: bb16];
     }
 
     bb6: {
         StorageDead(_4);
         StorageDead(_3);
-        drop(_2) -> [return: bb7, unwind terminate];
+        drop(_2) -> [return: bb7, unwind: bb17];
     }
 
     bb7: {

tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.diff

@@ -0,0 +1,186 @@
+- // MIR for `f_with_arg` before ElaborateDrops
++ // MIR for `f_with_arg` after ElaborateDrops
+  
+  fn f_with_arg(_1: String, _2: String) -> () {
+      debug _arg1 => _1;
+      debug _arg2 => _2;
+      let mut _0: ();
+      let mut _3: !;
+      let _4: std::string::String;
+      let _8: ();
+      let mut _9: std::string::String;
+      let mut _10: std::string::String;
+      let mut _11: std::string::String;
++     let mut _12: bool;
+      scope 1 {
+          debug _a => _4;
+          let _5: i32;
+          scope 2 {
+              debug _b => _5;
+              let _6: std::string::String;
+              scope 3 {
+                  debug _c => _6;
+                  let _7: std::string::String;
+                  scope 4 {
+                      debug _d => _7;
+                  }
+              }
+          }
+      }
+  
+      bb0: {
++         _12 = const false;
+          StorageLive(_4);
+          _4 = String::new() -> [return: bb1, unwind: bb27];
+      }
+  
+      bb1: {
+          StorageLive(_5);
+          _5 = const 12_i32;
+          StorageLive(_6);
+          _6 = String::new() -> [return: bb2, unwind: bb26];
+      }
+  
+      bb2: {
++         _12 = const true;
+          StorageLive(_7);
+          _7 = String::new() -> [return: bb3, unwind: bb25];
+      }
+  
+      bb3: {
+          StorageLive(_8);
+          StorageLive(_9);
++         _12 = const false;
+          _9 = move _6;
+          _8 = std::mem::drop::<String>(move _9) -> [return: bb4, unwind: bb23];
+      }
+  
+      bb4: {
+          StorageDead(_9);
+          StorageDead(_8);
+          StorageLive(_10);
+          _10 = String::new() -> [return: bb5, unwind: bb24];
+      }
+  
+      bb5: {
+          StorageLive(_11);
+          _11 = String::new() -> [return: bb6, unwind: bb22];
+      }
+  
+      bb6: {
+          drop(_7) -> [return: bb7, unwind: bb20];
+      }
+  
+      bb7: {
+          StorageDead(_7);
+-         drop(_6) -> [return: bb8, unwind: bb18];
++         goto -> bb8;
+      }
+  
+      bb8: {
++         _12 = const false;
+          StorageDead(_6);
+          StorageDead(_5);
+          drop(_4) -> [return: bb9, unwind: bb16];
+      }
+  
+      bb9: {
+          StorageDead(_4);
+          drop(_2) -> [return: bb10, unwind: bb14];
+      }
+  
+      bb10: {
+          drop(_1) -> [return: bb11, unwind: bb12];
+      }
+  
+      bb11: {
+          tailcall g_with_arg(move _10, move _11);
+      }
+  
+      bb12 (cleanup): {
+          drop(_10) -> [return: bb13, unwind terminate];
+      }
+  
+      bb13 (cleanup): {
+          drop(_11) -> [return: bb29, unwind terminate];
+      }
+  
+      bb14 (cleanup): {
+          drop(_10) -> [return: bb15, unwind terminate];
+      }
+  
+      bb15 (cleanup): {
+          drop(_11) -> [return: bb28, unwind terminate];
+      }
+  
+      bb16 (cleanup): {
+          drop(_10) -> [return: bb17, unwind terminate];
+      }
+  
+      bb17 (cleanup): {
+          drop(_11) -> [return: bb27, unwind terminate];
+      }
+  
+      bb18 (cleanup): {
+          drop(_10) -> [return: bb19, unwind terminate];
+      }
+  
+      bb19 (cleanup): {
+          drop(_11) -> [return: bb26, unwind terminate];
+      }
+  
+      bb20 (cleanup): {
+          drop(_10) -> [return: bb21, unwind terminate];
+      }
+  
+      bb21 (cleanup): {
+          drop(_11) -> [return: bb25, unwind terminate];
+      }
+  
+      bb22 (cleanup): {
+          drop(_10) -> [return: bb24, unwind terminate];
+      }
+  
+      bb23 (cleanup): {
+-         drop(_9) -> [return: bb24, unwind terminate];
++         goto -> bb24;
+      }
+  
+      bb24 (cleanup): {
+          drop(_7) -> [return: bb25, unwind terminate];
+      }
+  
+      bb25 (cleanup): {
+-         drop(_6) -> [return: bb26, unwind terminate];
++         goto -> bb32;
+      }
+  
+      bb26 (cleanup): {
+          drop(_4) -> [return: bb27, unwind terminate];
+      }
+  
+      bb27 (cleanup): {
+          drop(_2) -> [return: bb28, unwind terminate];
+      }
+  
+      bb28 (cleanup): {
+          drop(_1) -> [return: bb29, unwind terminate];
+      }
+  
+      bb29 (cleanup): {
+          resume;
++     }
++ 
++     bb30 (cleanup): {
++         unreachable;
++     }
++ 
++     bb31 (cleanup): {
++         drop(_6) -> [return: bb26, unwind terminate];
++     }
++ 
++     bb32 (cleanup): {
++         switchInt(_12) -> [0: bb26, otherwise: bb31];
+      }
+  }
+