Skip to content

Commit 9fe31e6

Browse files
beepster4096WaffleLapkin
beepster4096
authored and
WaffleLapkin
committed
fix tail call drop unwinding
1 parent 377c9a5 commit 9fe31e6

7 files changed

+479
-29
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
117117

118118
debug!("expr_into_dest: fn_span={:?}", fn_span);
119119

120-
unpack!(block = this.break_for_tail_call(block));
120+
unpack!(block = this.break_for_tail_call(block, &args, source_info));
121121

122122
this.cfg.terminate(
123123
block,

‎compiler/rustc_mir_build/src/build/scope.rs

+67-19
Original file line numberDiff line numberDiff line change
@@ -717,31 +717,79 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
717717
///
718718
/// Unlike other kinds of early exits, tail calls do not go through the drop tree.
719719
/// Instead, all scheduled drops are immediately added to the CFG.
720-
pub(crate) fn break_for_tail_call(&mut self, mut block: BasicBlock) -> BlockAnd<()> {
720+
pub(crate) fn break_for_tail_call(
721+
&mut self,
722+
mut block: BasicBlock,
723+
args: &[Spanned<Operand<'tcx>>],
724+
source_info: SourceInfo,
725+
) -> BlockAnd<()> {
726+
let arg_drops: Vec<_> = args
727+
.iter()
728+
.rev()
729+
.filter_map(|arg| match &arg.node {
730+
Operand::Copy(_) => bug!("copy op in tail call args"),
731+
Operand::Move(place) => {
732+
let local =
733+
place.as_local().unwrap_or_else(|| bug!("projection in tail call args"));
734+
735+
Some(DropData { source_info, local, kind: DropKind::Value })
736+
}
737+
Operand::Constant(_) => None,
738+
})
739+
.collect();
740+
741+
let mut unwind_to = self.diverge_cleanup_target(
742+
self.scopes.scopes.iter().rev().nth(1).unwrap().region_scope,
743+
DUMMY_SP,
744+
);
745+
let unwind_drops = &mut self.scopes.unwind_drops;
746+
721747
// the innermost scope contains only the destructors for the tail call arguments
722748
// we only want to drop these in case of a panic, so we skip it
723749
for scope in self.scopes.scopes[1..].iter().rev().skip(1) {
724-
for drop in scope.drops.iter().rev() {
725-
match drop.kind {
750+
// FIXME(explicit_tail_calls) code duplication with `build_scope_drops`
751+
for drop_data in scope.drops.iter().rev() {
752+
let source_info = drop_data.source_info;
753+
let local = drop_data.local;
754+
755+
match drop_data.kind {
726756
DropKind::Value => {
727-
let target = self.cfg.start_new_block();
728-
let terminator = TerminatorKind::Drop {
729-
target,
730-
// The caller will handle this if needed.
731-
// FIXME(explicit_tail_calls): add new reason
732-
unwind: UnwindAction::Terminate(UnwindTerminateReason::Abi),
733-
place: drop.local.into(),
734-
replace: false,
735-
};
736-
self.cfg.terminate(block, drop.source_info, terminator);
737-
block = target;
757+
// `unwind_to` should drop the value that we're about to
758+
// schedule. If dropping this value panics, then we continue
759+
// with the *next* value on the unwind path.
760+
debug_assert_eq!(unwind_drops.drops[unwind_to].0.local, drop_data.local);
761+
debug_assert_eq!(unwind_drops.drops[unwind_to].0.kind, drop_data.kind);
762+
unwind_to = unwind_drops.drops[unwind_to].1;
763+
764+
let mut unwind_entry_point = unwind_to;
765+
766+
// the tail call arguments must be dropped if any of these drops panic
767+
for drop in arg_drops.iter().copied() {
768+
unwind_entry_point = unwind_drops.add_drop(drop, unwind_entry_point);
769+
}
770+
771+
unwind_drops.add_entry(block, unwind_entry_point);
772+
773+
let next = self.cfg.start_new_block();
774+
self.cfg.terminate(
775+
block,
776+
source_info,
777+
TerminatorKind::Drop {
778+
place: local.into(),
779+
target: next,
780+
unwind: UnwindAction::Continue,
781+
replace: false,
782+
},
783+
);
784+
block = next;
738785
}
739786
DropKind::Storage => {
740-
let stmt = Statement {
741-
source_info: drop.source_info,
742-
kind: StatementKind::StorageDead(drop.local),
743-
};
744-
self.cfg.push(block, stmt);
787+
// Only temps and vars need their storage dead.
788+
assert!(local.index() > self.arg_count);
789+
self.cfg.push(
790+
block,
791+
Statement { source_info, kind: StatementKind::StorageDead(local) },
792+
);
745793
}
746794
}
747795
}

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

+5-6
Original file line numberDiff line numberDiff line change
@@ -54,22 +54,21 @@
5454
bb4: {
5555
StorageDead(_7);
5656
StorageDead(_6);
57-
- drop(_5) -> [return: bb5, unwind terminate];
58-
+ drop(_5) -> [return: bb5, unwind: bb13];
57+
drop(_5) -> [return: bb5, unwind: bb10];
5958
}
6059

6160
bb5: {
6261
StorageDead(_5);
63-
- drop(_4) -> [return: bb6, unwind terminate];
62+
- drop(_4) -> [return: bb6, unwind: bb11];
6463
+ goto -> bb6;
6564
}
6665

6766
bb6: {
6867
+ _8 = const false;
6968
StorageDead(_4);
7069
StorageDead(_3);
71-
- drop(_2) -> [return: bb7, unwind terminate];
72-
+ drop(_2) -> [return: bb7, unwind: bb13];
70+
- drop(_2) -> [return: bb7, unwind continue];
71+
+ drop(_2) -> [return: bb7, unwind: bb12];
7372
}
7473

7574
bb7: {
@@ -100,7 +99,7 @@
10099
+ }
101100
+
102101
+ bb13 (cleanup): {
103-
+ abort;
102+
+ unreachable;
104103
+ }
105104
+
106105
+ bb14 (cleanup): {

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,18 @@ fn f() -> () {
5353
bb4: {
5454
StorageDead(_7);
5555
StorageDead(_6);
56-
drop(_5) -> [return: bb5, unwind terminate];
56+
drop(_5) -> [return: bb5, unwind: bb15];
5757
}
5858

5959
bb5: {
6060
StorageDead(_5);
61-
drop(_4) -> [return: bb6, unwind terminate];
61+
drop(_4) -> [return: bb6, unwind: bb16];
6262
}
6363

6464
bb6: {
6565
StorageDead(_4);
6666
StorageDead(_3);
67-
drop(_2) -> [return: bb7, unwind terminate];
67+
drop(_2) -> [return: bb7, unwind: bb17];
6868
}
6969

7070
bb7: {

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

+186
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
- // MIR for `f_with_arg` before ElaborateDrops
2+
+ // MIR for `f_with_arg` after ElaborateDrops
3+
4+
fn f_with_arg(_1: String, _2: String) -> () {
5+
debug _arg1 => _1;
6+
debug _arg2 => _2;
7+
let mut _0: ();
8+
let mut _3: !;
9+
let _4: std::string::String;
10+
let _8: ();
11+
let mut _9: std::string::String;
12+
let mut _10: std::string::String;
13+
let mut _11: std::string::String;
14+
+ let mut _12: bool;
15+
scope 1 {
16+
debug _a => _4;
17+
let _5: i32;
18+
scope 2 {
19+
debug _b => _5;
20+
let _6: std::string::String;
21+
scope 3 {
22+
debug _c => _6;
23+
let _7: std::string::String;
24+
scope 4 {
25+
debug _d => _7;
26+
}
27+
}
28+
}
29+
}
30+
31+
bb0: {
32+
+ _12 = const false;
33+
StorageLive(_4);
34+
_4 = String::new() -> [return: bb1, unwind: bb27];
35+
}
36+
37+
bb1: {
38+
StorageLive(_5);
39+
_5 = const 12_i32;
40+
StorageLive(_6);
41+
_6 = String::new() -> [return: bb2, unwind: bb26];
42+
}
43+
44+
bb2: {
45+
+ _12 = const true;
46+
StorageLive(_7);
47+
_7 = String::new() -> [return: bb3, unwind: bb25];
48+
}
49+
50+
bb3: {
51+
StorageLive(_8);
52+
StorageLive(_9);
53+
+ _12 = const false;
54+
_9 = move _6;
55+
_8 = std::mem::drop::<String>(move _9) -> [return: bb4, unwind: bb23];
56+
}
57+
58+
bb4: {
59+
StorageDead(_9);
60+
StorageDead(_8);
61+
StorageLive(_10);
62+
_10 = String::new() -> [return: bb5, unwind: bb24];
63+
}
64+
65+
bb5: {
66+
StorageLive(_11);
67+
_11 = String::new() -> [return: bb6, unwind: bb22];
68+
}
69+
70+
bb6: {
71+
drop(_7) -> [return: bb7, unwind: bb20];
72+
}
73+
74+
bb7: {
75+
StorageDead(_7);
76+
- drop(_6) -> [return: bb8, unwind: bb18];
77+
+ goto -> bb8;
78+
}
79+
80+
bb8: {
81+
+ _12 = const false;
82+
StorageDead(_6);
83+
StorageDead(_5);
84+
drop(_4) -> [return: bb9, unwind: bb16];
85+
}
86+
87+
bb9: {
88+
StorageDead(_4);
89+
drop(_2) -> [return: bb10, unwind: bb14];
90+
}
91+
92+
bb10: {
93+
drop(_1) -> [return: bb11, unwind: bb12];
94+
}
95+
96+
bb11: {
97+
tailcall g_with_arg(move _10, move _11);
98+
}
99+
100+
bb12 (cleanup): {
101+
drop(_10) -> [return: bb13, unwind terminate];
102+
}
103+
104+
bb13 (cleanup): {
105+
drop(_11) -> [return: bb29, unwind terminate];
106+
}
107+
108+
bb14 (cleanup): {
109+
drop(_10) -> [return: bb15, unwind terminate];
110+
}
111+
112+
bb15 (cleanup): {
113+
drop(_11) -> [return: bb28, unwind terminate];
114+
}
115+
116+
bb16 (cleanup): {
117+
drop(_10) -> [return: bb17, unwind terminate];
118+
}
119+
120+
bb17 (cleanup): {
121+
drop(_11) -> [return: bb27, unwind terminate];
122+
}
123+
124+
bb18 (cleanup): {
125+
drop(_10) -> [return: bb19, unwind terminate];
126+
}
127+
128+
bb19 (cleanup): {
129+
drop(_11) -> [return: bb26, unwind terminate];
130+
}
131+
132+
bb20 (cleanup): {
133+
drop(_10) -> [return: bb21, unwind terminate];
134+
}
135+
136+
bb21 (cleanup): {
137+
drop(_11) -> [return: bb25, unwind terminate];
138+
}
139+
140+
bb22 (cleanup): {
141+
drop(_10) -> [return: bb24, unwind terminate];
142+
}
143+
144+
bb23 (cleanup): {
145+
- drop(_9) -> [return: bb24, unwind terminate];
146+
+ goto -> bb24;
147+
}
148+
149+
bb24 (cleanup): {
150+
drop(_7) -> [return: bb25, unwind terminate];
151+
}
152+
153+
bb25 (cleanup): {
154+
- drop(_6) -> [return: bb26, unwind terminate];
155+
+ goto -> bb32;
156+
}
157+
158+
bb26 (cleanup): {
159+
drop(_4) -> [return: bb27, unwind terminate];
160+
}
161+
162+
bb27 (cleanup): {
163+
drop(_2) -> [return: bb28, unwind terminate];
164+
}
165+
166+
bb28 (cleanup): {
167+
drop(_1) -> [return: bb29, unwind terminate];
168+
}
169+
170+
bb29 (cleanup): {
171+
resume;
172+
+ }
173+
+
174+
+ bb30 (cleanup): {
175+
+ unreachable;
176+
+ }
177+
+
178+
+ bb31 (cleanup): {
179+
+ drop(_6) -> [return: bb26, unwind terminate];
180+
+ }
181+
+
182+
+ bb32 (cleanup): {
183+
+ switchInt(_12) -> [0: bb26, otherwise: bb31];
184+
}
185+
}
186+

0 commit comments

Comments
 (0)