Skip to content

Commit f79affd

Browse files
beepster4096beepster4096
committed
fix tail call drop unwinding
1 parent 119cb0d commit f79affd

7 files changed

+479
-28
lines changed

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

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

123123
debug!("expr_into_dest: fn_span={:?}", fn_span);
124124

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

127127
this.cfg.terminate(
128128
block,

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

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

‎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)