From f79affd8c3b08c70f5a248ecb48280e0310aafab Mon Sep 17 00:00:00 2001 From: DrMeepster <19316085+DrMeepster@users.noreply.github.com> Date: Sat, 1 Jul 2023 01:53:00 -0700 Subject: [PATCH] fix tail call drop unwinding --- .../rustc_mir_build/src/build/expr/stmt.rs | 2 +- compiler/rustc_mir_build/src/build/scope.rs | 85 ++++++-- .../tail_call_drops.f.ElaborateDrops.diff | 11 +- .../mir-opt/tail_call_drops.f.built.after.mir | 6 +- ..._call_drops.f_with_arg.ElaborateDrops.diff | 186 ++++++++++++++++ ...tail_call_drops.f_with_arg.built.after.mir | 202 ++++++++++++++++++ tests/mir-opt/tail_call_drops.rs | 15 ++ 7 files changed, 479 insertions(+), 28 deletions(-) create mode 100644 tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.diff create mode 100644 tests/mir-opt/tail_call_drops.f_with_arg.built.after.mir diff --git a/compiler/rustc_mir_build/src/build/expr/stmt.rs b/compiler/rustc_mir_build/src/build/expr/stmt.rs index e57e628e8296a..bfc076d75aefb 100644 --- a/compiler/rustc_mir_build/src/build/expr/stmt.rs +++ b/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, diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 9ab78416acb53..7b37619290bed 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/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) }, + ); } } } diff --git a/tests/mir-opt/tail_call_drops.f.ElaborateDrops.diff b/tests/mir-opt/tail_call_drops.f.ElaborateDrops.diff index b9b2b88b808d2..9943c47fc1e5d 100644 --- a/tests/mir-opt/tail_call_drops.f.ElaborateDrops.diff +++ b/tests/mir-opt/tail_call_drops.f.ElaborateDrops.diff @@ -54,13 +54,12 @@ 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; } @@ -68,8 +67,8 @@ + _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): { diff --git a/tests/mir-opt/tail_call_drops.f.built.after.mir b/tests/mir-opt/tail_call_drops.f.built.after.mir index a1f612eac8991..c7e42daa1c2dc 100644 --- a/tests/mir-opt/tail_call_drops.f.built.after.mir +++ b/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: { diff --git a/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.diff b/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.diff new file mode 100644 index 0000000000000..98d6634253a53 --- /dev/null +++ b/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::(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]; + } + } + diff --git a/tests/mir-opt/tail_call_drops.f_with_arg.built.after.mir b/tests/mir-opt/tail_call_drops.f_with_arg.built.after.mir new file mode 100644 index 0000000000000..015bb442e2a63 --- /dev/null +++ b/tests/mir-opt/tail_call_drops.f_with_arg.built.after.mir @@ -0,0 +1,202 @@ +// MIR for `f_with_arg` after built + +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; + 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: { + StorageLive(_4); + _4 = String::new() -> [return: bb1, unwind: bb34]; + } + + bb1: { + FakeRead(ForLet(None), _4); + StorageLive(_5); + _5 = const 12_i32; + FakeRead(ForLet(None), _5); + StorageLive(_6); + _6 = String::new() -> [return: bb2, unwind: bb33]; + } + + bb2: { + FakeRead(ForLet(None), _6); + StorageLive(_7); + _7 = String::new() -> [return: bb3, unwind: bb32]; + } + + bb3: { + FakeRead(ForLet(None), _7); + StorageLive(_8); + StorageLive(_9); + _9 = move _6; + _8 = std::mem::drop::(move _9) -> [return: bb4, unwind: bb30]; + } + + bb4: { + StorageDead(_9); + StorageDead(_8); + StorageLive(_10); + _10 = String::new() -> [return: bb5, unwind: bb31]; + } + + bb5: { + StorageLive(_11); + _11 = String::new() -> [return: bb6, unwind: bb29]; + } + + bb6: { + drop(_7) -> [return: bb7, unwind: bb27]; + } + + bb7: { + StorageDead(_7); + drop(_6) -> [return: bb8, unwind: bb25]; + } + + bb8: { + StorageDead(_6); + StorageDead(_5); + drop(_4) -> [return: bb9, unwind: bb23]; + } + + bb9: { + StorageDead(_4); + drop(_2) -> [return: bb10, unwind: bb21]; + } + + bb10: { + drop(_1) -> [return: bb11, unwind: bb19]; + } + + bb11: { + tailcall g_with_arg(move _10, move _11); + } + + bb12: { + StorageDead(_11); + StorageDead(_10); + drop(_7) -> [return: bb13, unwind: bb32]; + } + + bb13: { + StorageDead(_7); + drop(_6) -> [return: bb14, unwind: bb33]; + } + + bb14: { + StorageDead(_6); + StorageDead(_5); + drop(_4) -> [return: bb15, unwind: bb34]; + } + + bb15: { + StorageDead(_4); + unreachable; + } + + bb16: { + drop(_2) -> [return: bb17, unwind: bb35]; + } + + bb17: { + drop(_1) -> [return: bb18, unwind: bb36]; + } + + bb18: { + return; + } + + bb19 (cleanup): { + drop(_10) -> [return: bb20, unwind terminate]; + } + + bb20 (cleanup): { + drop(_11) -> [return: bb36, unwind terminate]; + } + + bb21 (cleanup): { + drop(_10) -> [return: bb22, unwind terminate]; + } + + bb22 (cleanup): { + drop(_11) -> [return: bb35, unwind terminate]; + } + + bb23 (cleanup): { + drop(_10) -> [return: bb24, unwind terminate]; + } + + bb24 (cleanup): { + drop(_11) -> [return: bb34, unwind terminate]; + } + + bb25 (cleanup): { + drop(_10) -> [return: bb26, unwind terminate]; + } + + bb26 (cleanup): { + drop(_11) -> [return: bb33, unwind terminate]; + } + + bb27 (cleanup): { + drop(_10) -> [return: bb28, unwind terminate]; + } + + bb28 (cleanup): { + drop(_11) -> [return: bb32, unwind terminate]; + } + + bb29 (cleanup): { + drop(_10) -> [return: bb31, unwind terminate]; + } + + bb30 (cleanup): { + drop(_9) -> [return: bb31, unwind terminate]; + } + + bb31 (cleanup): { + drop(_7) -> [return: bb32, unwind terminate]; + } + + bb32 (cleanup): { + drop(_6) -> [return: bb33, unwind terminate]; + } + + bb33 (cleanup): { + drop(_4) -> [return: bb34, unwind terminate]; + } + + bb34 (cleanup): { + drop(_2) -> [return: bb35, unwind terminate]; + } + + bb35 (cleanup): { + drop(_1) -> [return: bb36, unwind terminate]; + } + + bb36 (cleanup): { + resume; + } +} diff --git a/tests/mir-opt/tail_call_drops.rs b/tests/mir-opt/tail_call_drops.rs index a7c5c65a16733..d5110322e6226 100644 --- a/tests/mir-opt/tail_call_drops.rs +++ b/tests/mir-opt/tail_call_drops.rs @@ -22,4 +22,19 @@ fn f() { fn g() {} +// EMIT_MIR tail_call_drops.f_with_arg.built.after.mir +// EMIT_MIR tail_call_drops.f_with_arg.ElaborateDrops.diff +fn f_with_arg(_arg1: String, _arg2: String) { + let _a = String::new(); + let _b = 12; + let _c = String::new(); + let _d = String::new(); + + drop(_c); + + become g_with_arg(String::new(), String::new()); +} + +fn g_with_arg(_arg1: String, _arg2: String) {} + fn main() {}