Skip to content

Commit 1fc4515

Browse files
authored
Rollup merge of rust-lang#69814 - jonas-schievink:gen-ret-unw, r=Zoxc
Smaller and more correct generator codegen This removes unnecessary panicking branches in the resume function when the generator can not return or unwind, respectively. Closes rust-lang#66100 It also addresses the correctness concerns wrt poisoning on unwind. These are not currently a soundness issue because any operation *inside* a generator that could possibly unwind will result in a cleanup path for dropping it, ultimately reaching a `Resume` terminator, which we already handled correctly. Future MIR optimizations might optimize that out, though. r? @Zoxc
2 parents e42a844 + 38fa378 commit 1fc4515

File tree

2 files changed

+138
-7
lines changed

2 files changed

+138
-7
lines changed

src/librustc_mir/transform/generator.rs

+104-7
Original file line numberDiff line numberDiff line change
@@ -991,18 +991,101 @@ fn insert_panic_block<'tcx>(
991991
assert_block
992992
}
993993

994+
fn can_return<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> bool {
995+
// Returning from a function with an uninhabited return type is undefined behavior.
996+
if body.return_ty().conservative_is_privately_uninhabited(tcx) {
997+
return false;
998+
}
999+
1000+
// If there's a return terminator the function may return.
1001+
for block in body.basic_blocks() {
1002+
if let TerminatorKind::Return = block.terminator().kind {
1003+
return true;
1004+
}
1005+
}
1006+
1007+
// Otherwise the function can't return.
1008+
false
1009+
}
1010+
1011+
fn can_unwind<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> bool {
1012+
// Nothing can unwind when landing pads are off.
1013+
if tcx.sess.no_landing_pads() {
1014+
return false;
1015+
}
1016+
1017+
// Unwinds can only start at certain terminators.
1018+
for block in body.basic_blocks() {
1019+
match block.terminator().kind {
1020+
// These never unwind.
1021+
TerminatorKind::Goto { .. }
1022+
| TerminatorKind::SwitchInt { .. }
1023+
| TerminatorKind::Abort
1024+
| TerminatorKind::Return
1025+
| TerminatorKind::Unreachable
1026+
| TerminatorKind::GeneratorDrop
1027+
| TerminatorKind::FalseEdges { .. }
1028+
| TerminatorKind::FalseUnwind { .. } => {}
1029+
1030+
// Resume will *continue* unwinding, but if there's no other unwinding terminator it
1031+
// will never be reached.
1032+
TerminatorKind::Resume => {}
1033+
1034+
TerminatorKind::Yield { .. } => {
1035+
unreachable!("`can_unwind` called before generator transform")
1036+
}
1037+
1038+
// These may unwind.
1039+
TerminatorKind::Drop { .. }
1040+
| TerminatorKind::DropAndReplace { .. }
1041+
| TerminatorKind::Call { .. }
1042+
| TerminatorKind::Assert { .. } => return true,
1043+
}
1044+
}
1045+
1046+
// If we didn't find an unwinding terminator, the function cannot unwind.
1047+
false
1048+
}
1049+
9941050
fn create_generator_resume_function<'tcx>(
9951051
tcx: TyCtxt<'tcx>,
9961052
transform: TransformVisitor<'tcx>,
9971053
def_id: DefId,
9981054
source: MirSource<'tcx>,
9991055
body: &mut BodyAndCache<'tcx>,
1056+
can_return: bool,
10001057
) {
1058+
let can_unwind = can_unwind(tcx, body);
1059+
10011060
// Poison the generator when it unwinds
1002-
for block in body.basic_blocks_mut() {
1003-
let source_info = block.terminator().source_info;
1004-
if let &TerminatorKind::Resume = &block.terminator().kind {
1005-
block.statements.push(transform.set_discr(VariantIdx::new(POISONED), source_info));
1061+
if can_unwind {
1062+
let poison_block = BasicBlock::new(body.basic_blocks().len());
1063+
let source_info = source_info(body);
1064+
body.basic_blocks_mut().push(BasicBlockData {
1065+
statements: vec![transform.set_discr(VariantIdx::new(POISONED), source_info)],
1066+
terminator: Some(Terminator { source_info, kind: TerminatorKind::Resume }),
1067+
is_cleanup: true,
1068+
});
1069+
1070+
for (idx, block) in body.basic_blocks_mut().iter_enumerated_mut() {
1071+
let source_info = block.terminator().source_info;
1072+
1073+
if let TerminatorKind::Resume = block.terminator().kind {
1074+
// An existing `Resume` terminator is redirected to jump to our dedicated
1075+
// "poisoning block" above.
1076+
if idx != poison_block {
1077+
*block.terminator_mut() = Terminator {
1078+
source_info,
1079+
kind: TerminatorKind::Goto { target: poison_block },
1080+
};
1081+
}
1082+
} else if !block.is_cleanup {
1083+
// Any terminators that *can* unwind but don't have an unwind target set are also
1084+
// pointed at our poisoning block (unless they're part of the cleanup path).
1085+
if let Some(unwind @ None) = block.terminator_mut().unwind_mut() {
1086+
*unwind = Some(poison_block);
1087+
}
1088+
}
10061089
}
10071090
}
10081091

@@ -1015,8 +1098,20 @@ fn create_generator_resume_function<'tcx>(
10151098

10161099
// Panic when resumed on the returned or poisoned state
10171100
let generator_kind = body.generator_kind.unwrap();
1018-
cases.insert(1, (RETURNED, insert_panic_block(tcx, body, ResumedAfterReturn(generator_kind))));
1019-
cases.insert(2, (POISONED, insert_panic_block(tcx, body, ResumedAfterPanic(generator_kind))));
1101+
1102+
if can_unwind {
1103+
cases.insert(
1104+
1,
1105+
(POISONED, insert_panic_block(tcx, body, ResumedAfterPanic(generator_kind))),
1106+
);
1107+
}
1108+
1109+
if can_return {
1110+
cases.insert(
1111+
1,
1112+
(RETURNED, insert_panic_block(tcx, body, ResumedAfterReturn(generator_kind))),
1113+
);
1114+
}
10201115

10211116
insert_switch(body, cases, &transform, TerminatorKind::Unreachable);
10221117

@@ -1200,6 +1295,8 @@ impl<'tcx> MirPass<'tcx> for StateTransform {
12001295
let (remap, layout, storage_liveness) =
12011296
compute_layout(tcx, source, &upvars, interior, movable, body);
12021297

1298+
let can_return = can_return(tcx, body);
1299+
12031300
// Run the transformation which converts Places from Local to generator struct
12041301
// accesses for locals in `remap`.
12051302
// It also rewrites `return x` and `yield y` as writing a new generator state and returning
@@ -1243,6 +1340,6 @@ impl<'tcx> MirPass<'tcx> for StateTransform {
12431340
body.generator_drop = Some(box drop_shim);
12441341

12451342
// Create the Generator::resume function
1246-
create_generator_resume_function(tcx, transform, def_id, source, body);
1343+
create_generator_resume_function(tcx, transform, def_id, source, body, can_return);
12471344
}
12481345
}

src/test/mir-opt/generator-tiny.rs

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//! Tests that generators that cannot return or unwind don't have unnecessary
2+
//! panic branches.
3+
4+
// compile-flags: -Zno-landing-pads
5+
6+
#![feature(generators, generator_trait)]
7+
8+
struct HasDrop;
9+
10+
impl Drop for HasDrop {
11+
fn drop(&mut self) {}
12+
}
13+
14+
fn callee() {}
15+
16+
fn main() {
17+
let _gen = |_x: u8| {
18+
let _d = HasDrop;
19+
loop {
20+
yield;
21+
callee();
22+
}
23+
};
24+
}
25+
26+
// END RUST SOURCE
27+
28+
// START rustc.main-{{closure}}.generator_resume.0.mir
29+
// bb0: {
30+
// ...
31+
// switchInt(move _11) -> [0u32: bb1, 3u32: bb5, otherwise: bb6];
32+
// }
33+
// ...
34+
// END rustc.main-{{closure}}.generator_resume.0.mir

0 commit comments

Comments
 (0)