Skip to content

Commit 6faf40b

Browse files
matthiaskrgrRenjiSann
matthiaskrgr
authored and
RenjiSann
committed
Rollup merge of rust-lang#122168 - compiler-errors:inline-coroutine-body-validation, r=cjgillot
Fix validation on substituted callee bodies in MIR inliner When inlining a coroutine, we will substitute the MIR body with the args of the call. There is code in the MIR validator that attempts to prevent query cycles, and will use the coroutine body directly when it detects that's the body that's being validated. That means that when inlining a coroutine body that has been substituted, it may no longer be parameterized over the original args of the coroutine, which will lead to substitution ICEs. Fixes rust-lang#119064
2 parents 7ae67c9 + 78ebb93 commit 6faf40b

File tree

6 files changed

+669
-10
lines changed

6 files changed

+669
-10
lines changed

‎compiler/rustc_const_eval/src/transform/validate.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ impl<'tcx> MirPass<'tcx> for Validator {
8585
cfg_checker.check_cleanup_control_flow();
8686

8787
// Also run the TypeChecker.
88-
for (location, msg) in validate_types(tcx, self.mir_phase, param_env, body) {
88+
for (location, msg) in validate_types(tcx, self.mir_phase, param_env, body, body) {
8989
cfg_checker.fail(location, msg);
9090
}
9191

@@ -540,19 +540,25 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
540540

541541
/// A faster version of the validation pass that only checks those things which may break when
542542
/// instantiating any generic parameters.
543+
///
544+
/// `caller_body` is used to detect cycles in MIR inlining and MIR validation before
545+
/// `optimized_mir` is available.
543546
pub fn validate_types<'tcx>(
544547
tcx: TyCtxt<'tcx>,
545548
mir_phase: MirPhase,
546549
param_env: ty::ParamEnv<'tcx>,
547550
body: &Body<'tcx>,
551+
caller_body: &Body<'tcx>,
548552
) -> Vec<(Location, String)> {
549-
let mut type_checker = TypeChecker { body, tcx, param_env, mir_phase, failures: Vec::new() };
553+
let mut type_checker =
554+
TypeChecker { body, caller_body, tcx, param_env, mir_phase, failures: Vec::new() };
550555
type_checker.visit_body(body);
551556
type_checker.failures
552557
}
553558

554559
struct TypeChecker<'a, 'tcx> {
555560
body: &'a Body<'tcx>,
561+
caller_body: &'a Body<'tcx>,
556562
tcx: TyCtxt<'tcx>,
557563
param_env: ParamEnv<'tcx>,
558564
mir_phase: MirPhase,
@@ -704,8 +710,13 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
704710
}
705711
&ty::Coroutine(def_id, args) => {
706712
let f_ty = if let Some(var) = parent_ty.variant_index {
707-
let gen_body = if def_id == self.body.source.def_id() {
708-
self.body
713+
// If we're currently validating an inlined copy of this body,
714+
// then it will no longer be parameterized over the original
715+
// args of the coroutine. Otherwise, we prefer to use this body
716+
// since we may be in the process of computing this MIR in the
717+
// first place.
718+
let gen_body = if def_id == self.caller_body.source.def_id() {
719+
self.caller_body
709720
} else {
710721
self.tcx.optimized_mir(def_id)
711722
};

‎compiler/rustc_mir_transform/src/coroutine/by_move_body.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,9 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody {
6464
let mut by_move_body = body.clone();
6565
MakeByMoveBody { tcx, by_ref_fields, by_move_coroutine_ty }.visit_body(&mut by_move_body);
6666
dump_mir(tcx, false, "coroutine_by_move", &0, &by_move_body, |_, _| Ok(()));
67-
by_move_body.source = mir::MirSource {
68-
instance: InstanceDef::CoroutineKindShim {
69-
coroutine_def_id: coroutine_def_id.to_def_id(),
70-
},
71-
promoted: None,
72-
};
67+
by_move_body.source = mir::MirSource::from_instance(InstanceDef::CoroutineKindShim {
68+
coroutine_def_id: coroutine_def_id.to_def_id(),
69+
});
7370
body.coroutine.as_mut().unwrap().by_move_body = Some(by_move_body);
7471
}
7572
}

‎compiler/rustc_mir_transform/src/inline.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ impl<'tcx> Inliner<'tcx> {
213213
MirPhase::Runtime(RuntimePhase::Optimized),
214214
self.param_env,
215215
&callee_body,
216+
&caller_body,
216217
)
217218
.is_empty()
218219
{

‎tests/mir-opt/inline_coroutine_body.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
2+
// skip-filecheck
3+
//@ unit-test: Inline
4+
//@ edition: 2021
5+
//@ compile-flags: -Zinline-mir-hint-threshold=10000 -Zinline-mir-threshold=10000 --crate-type=lib
6+
7+
pub async fn run(permit: ActionPermit<'_, ()>, ctx: &mut core::task::Context<'_>) {
8+
run2(permit, ctx);
9+
}
10+
11+
// EMIT_MIR inline_coroutine_body.run2-{closure#0}.Inline.diff
12+
fn run2<T>(permit: ActionPermit<'_, T>, ctx: &mut core::task::Context) {
13+
_ = || {
14+
let mut fut = ActionPermit::perform(permit);
15+
let fut = unsafe { core::pin::Pin::new_unchecked(&mut fut) };
16+
_ = core::future::Future::poll(fut, ctx);
17+
};
18+
}
19+
20+
pub struct ActionPermit<'a, T> {
21+
_guard: core::cell::Ref<'a, T>,
22+
}
23+
24+
impl<'a, T> ActionPermit<'a, T> {
25+
async fn perform(self) {
26+
core::future::ready(()).await
27+
}
28+
}

‎tests/mir-opt/inline_coroutine_body.run2-{closure#0}.Inline.panic-abort.diff

Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
- // MIR for `run2::{closure#0}` before Inline
2+
+ // MIR for `run2::{closure#0}` after Inline
3+
4+
fn run2::{closure#0}(_1: {closure@$DIR/inline_coroutine_body.rs:13:9: 13:11}) -> () {
5+
debug permit => (_1.0: ActionPermit<'_, T>);
6+
debug ctx => (*(_1.1: &mut std::task::Context<'_>));
7+
let mut _0: ();
8+
let mut _2: {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6};
9+
let mut _3: ActionPermit<'_, T>;
10+
let mut _5: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6};
11+
let _6: ();
12+
let mut _7: std::task::Poll<()>;
13+
let mut _8: std::pin::Pin<&mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6}>;
14+
let mut _9: &mut std::task::Context<'_>;
15+
let mut _10: &mut std::task::Context<'_>;
16+
scope 1 {
17+
debug fut => _2;
18+
let _4: std::pin::Pin<&mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6}>;
19+
scope 2 {
20+
debug fut => _4;
21+
scope 4 {
22+
}
23+
+ scope 7 (inlined ActionPermit::<'_, T>::perform::{closure#0}) {
24+
+ debug _task_context => _31;
25+
+ debug self => ((*(_8.0: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6})).0: ActionPermit<'_, T>);
26+
+ let _11: ActionPermit<'_, T>;
27+
+ let mut _12: std::future::Ready<()>;
28+
+ let mut _13: std::future::Ready<()>;
29+
+ let mut _14: ();
30+
+ let mut _16: ();
31+
+ let _17: ();
32+
+ let mut _18: std::task::Poll<()>;
33+
+ let mut _19: std::pin::Pin<&mut std::future::Ready<()>>;
34+
+ let mut _20: &mut std::future::Ready<()>;
35+
+ let mut _21: &mut std::future::Ready<()>;
36+
+ let mut _22: &mut std::task::Context<'_>;
37+
+ let mut _23: &mut std::task::Context<'_>;
38+
+ let mut _24: &mut std::task::Context<'_>;
39+
+ let mut _25: isize;
40+
+ let mut _27: !;
41+
+ let mut _28: &mut std::task::Context<'_>;
42+
+ let mut _29: ();
43+
+ let mut _30: ();
44+
+ let mut _31: &mut std::task::Context<'_>;
45+
+ let mut _32: u32;
46+
+ let mut _33: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6};
47+
+ let mut _34: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6};
48+
+ let mut _35: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6};
49+
+ let mut _36: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6};
50+
+ let mut _37: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6};
51+
+ let mut _38: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6};
52+
+ let mut _39: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6};
53+
+ let mut _40: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6};
54+
+ scope 8 {
55+
+ debug self => (((*(_8.0: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6})) as variant#3).0: ActionPermit<'_, T>);
56+
+ let mut _15: std::future::Ready<()>;
57+
+ scope 9 {
58+
+ debug __awaitee => (((*(_8.0: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6})) as variant#3).1: std::future::Ready<()>);
59+
+ let _26: ();
60+
+ scope 10 {
61+
+ }
62+
+ scope 11 {
63+
+ debug result => _26;
64+
+ }
65+
+ }
66+
+ scope 12 (inlined ready::<()>) {
67+
+ debug t => _14;
68+
+ let mut _41: std::option::Option<()>;
69+
+ }
70+
+ }
71+
+ }
72+
}
73+
scope 3 {
74+
+ scope 6 (inlined Pin::<&mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6}>::new_unchecked) {
75+
+ debug pointer => _5;
76+
+ }
77+
}
78+
}
79+
+ scope 5 (inlined ActionPermit::<'_, T>::perform) {
80+
+ debug self => _3;
81+
+ }
82+
83+
bb0: {
84+
StorageLive(_2);
85+
StorageLive(_3);
86+
_3 = move (_1.0: ActionPermit<'_, T>);
87+
- _2 = ActionPermit::<'_, T>::perform(move _3) -> [return: bb1, unwind unreachable];
88+
- }
89+
-
90+
- bb1: {
91+
+ _2 = {coroutine@$DIR/inline_coroutine_body.rs:25:28: 27:6 (#0)} { self: move _3 };
92+
StorageDead(_3);
93+
StorageLive(_4);
94+
StorageLive(_5);
95+
_5 = &mut _2;
96+
- _4 = Pin::<&mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6}>::new_unchecked(move _5) -> [return: bb2, unwind unreachable];
97+
- }
98+
-
99+
- bb2: {
100+
+ _4 = Pin::<&mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6}> { __pointer: _5 };
101+
StorageDead(_5);
102+
StorageLive(_6);
103+
StorageLive(_7);
104+
StorageLive(_8);
105+
_8 = move _4;
106+
StorageLive(_9);
107+
_10 = deref_copy (_1.1: &mut std::task::Context<'_>);
108+
_9 = &mut (*_10);
109+
- _7 = <{async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6} as Future>::poll(move _8, move _9) -> [return: bb3, unwind unreachable];
110+
+ StorageLive(_11);
111+
+ StorageLive(_15);
112+
+ StorageLive(_16);
113+
+ StorageLive(_25);
114+
+ StorageLive(_27);
115+
+ StorageLive(_30);
116+
+ StorageLive(_31);
117+
+ StorageLive(_32);
118+
+ StorageLive(_33);
119+
+ StorageLive(_34);
120+
+ StorageLive(_35);
121+
+ StorageLive(_36);
122+
+ StorageLive(_37);
123+
+ StorageLive(_38);
124+
+ StorageLive(_39);
125+
+ StorageLive(_40);
126+
+ _33 = deref_copy (_8.0: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6});
127+
+ _32 = discriminant((*_33));
128+
+ switchInt(move _32) -> [0: bb3, 1: bb13, 3: bb12, otherwise: bb8];
129+
}
130+
131+
- bb3: {
132+
+ bb1: {
133+
+ StorageDead(_2);
134+
+ return;
135+
+ }
136+
+
137+
+ bb2: {
138+
+ StorageDead(_40);
139+
+ StorageDead(_39);
140+
+ StorageDead(_38);
141+
+ StorageDead(_37);
142+
+ StorageDead(_36);
143+
+ StorageDead(_35);
144+
+ StorageDead(_34);
145+
+ StorageDead(_33);
146+
+ StorageDead(_32);
147+
+ StorageDead(_31);
148+
+ StorageDead(_30);
149+
+ StorageDead(_27);
150+
+ StorageDead(_25);
151+
+ StorageDead(_16);
152+
+ StorageDead(_15);
153+
+ StorageDead(_11);
154+
StorageDead(_9);
155+
StorageDead(_8);
156+
StorageDead(_7);
157+
_6 = const ();
158+
StorageDead(_6);
159+
_0 = const ();
160+
StorageDead(_4);
161+
- drop(_2) -> [return: bb4, unwind unreachable];
162+
+ drop(_2) -> [return: bb1, unwind unreachable];
163+
}
164+
165+
+ bb3: {
166+
+ _31 = move _9;
167+
+ _34 = deref_copy (_8.0: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6});
168+
+ _35 = deref_copy (_8.0: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6});
169+
+ (((*_34) as variant#3).0: ActionPermit<'_, T>) = move ((*_35).0: ActionPermit<'_, T>);
170+
+ StorageLive(_12);
171+
+ StorageLive(_13);
172+
+ StorageLive(_14);
173+
+ _14 = ();
174+
+ StorageLive(_41);
175+
+ _41 = Option::<()>::Some(_14);
176+
+ _13 = std::future::Ready::<()>(move _41);
177+
+ StorageDead(_41);
178+
+ StorageDead(_14);
179+
+ _12 = <std::future::Ready<()> as IntoFuture>::into_future(move _13) -> [return: bb4, unwind unreachable];
180+
+ }
181+
+
182+
bb4: {
183+
- StorageDead(_2);
184+
- return;
185+
+ StorageDead(_13);
186+
+ _36 = deref_copy (_8.0: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6});
187+
+ (((*_36) as variant#3).1: std::future::Ready<()>) = move _12;
188+
+ goto -> bb5;
189+
+ }
190+
+
191+
+ bb5: {
192+
+ StorageLive(_17);
193+
+ StorageLive(_18);
194+
+ StorageLive(_19);
195+
+ StorageLive(_20);
196+
+ StorageLive(_21);
197+
+ _37 = deref_copy (_8.0: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6});
198+
+ _21 = &mut (((*_37) as variant#3).1: std::future::Ready<()>);
199+
+ _20 = &mut (*_21);
200+
+ _19 = Pin::<&mut std::future::Ready<()>>::new_unchecked(move _20) -> [return: bb6, unwind unreachable];
201+
+ }
202+
+
203+
+ bb6: {
204+
+ StorageDead(_20);
205+
+ StorageLive(_22);
206+
+ StorageLive(_23);
207+
+ StorageLive(_24);
208+
+ _24 = _31;
209+
+ _23 = move _24;
210+
+ _22 = &mut (*_23);
211+
+ StorageDead(_24);
212+
+ _18 = <std::future::Ready<()> as Future>::poll(move _19, move _22) -> [return: bb7, unwind unreachable];
213+
+ }
214+
+
215+
+ bb7: {
216+
+ StorageDead(_22);
217+
+ StorageDead(_19);
218+
+ _25 = discriminant(_18);
219+
+ switchInt(move _25) -> [0: bb10, 1: bb9, otherwise: bb8];
220+
+ }
221+
+
222+
+ bb8: {
223+
+ unreachable;
224+
+ }
225+
+
226+
+ bb9: {
227+
+ _17 = const ();
228+
+ StorageDead(_23);
229+
+ StorageDead(_21);
230+
+ StorageDead(_18);
231+
+ StorageDead(_17);
232+
+ StorageLive(_28);
233+
+ StorageLive(_29);
234+
+ _29 = ();
235+
+ _7 = Poll::<()>::Pending;
236+
+ StorageDead(_12);
237+
+ StorageDead(_28);
238+
+ StorageDead(_29);
239+
+ _38 = deref_copy (_8.0: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6});
240+
+ discriminant((*_38)) = 3;
241+
+ goto -> bb2;
242+
+ }
243+
+
244+
+ bb10: {
245+
+ StorageLive(_26);
246+
+ _26 = ((_18 as Ready).0: ());
247+
+ _30 = _26;
248+
+ StorageDead(_26);
249+
+ StorageDead(_23);
250+
+ StorageDead(_21);
251+
+ StorageDead(_18);
252+
+ StorageDead(_17);
253+
+ StorageDead(_12);
254+
+ _39 = deref_copy (_8.0: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6});
255+
+ drop((((*_39) as variant#3).0: ActionPermit<'_, T>)) -> [return: bb11, unwind unreachable];
256+
+ }
257+
+
258+
+ bb11: {
259+
+ _7 = Poll::<()>::Ready(move _30);
260+
+ _40 = deref_copy (_8.0: &mut {async fn body@$DIR/inline_coroutine_body.rs:25:28: 27:6});
261+
+ discriminant((*_40)) = 1;
262+
+ goto -> bb2;
263+
+ }
264+
+
265+
+ bb12: {
266+
+ StorageLive(_12);
267+
+ StorageLive(_28);
268+
+ StorageLive(_29);
269+
+ _28 = move _9;
270+
+ StorageDead(_29);
271+
+ _31 = move _28;
272+
+ StorageDead(_28);
273+
+ _16 = const ();
274+
+ goto -> bb5;
275+
+ }
276+
+
277+
+ bb13: {
278+
+ assert(const false, "`async fn` resumed after completion") -> [success: bb13, unwind unreachable];
279+
}
280+
}
281+

0 commit comments

Comments
 (0)