Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NLL] Add false edges out of infinite loops #47802

Merged
merged 6 commits into from
Feb 9, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
mir: Add TerminatorKind::FalseUnwind
Sometimes a simple goto misses the cleanup/unwind edges. Specifically, in the
case of infinite loops such as those introduced by a loop statement without any
other out edges. Analogous to TerminatorKind::FalseEdges; this new terminator
kind is used when we want borrowck to consider an unwind path, but real control
flow should never actually take it.
  • Loading branch information
sapphire-arches committed Feb 5, 2018
commit bdc37aa05722818e8edb5d93825a62921f351913
4 changes: 4 additions & 0 deletions src/librustc/ich/impls_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ for mir::TerminatorKind<'gcx> {
target.hash_stable(hcx, hasher);
}
}
mir::TerminatorKind::FalseUnwind { ref real_target, ref unwind } => {
real_target.hash_stable(hcx, hasher);
unwind.hash_stable(hcx, hasher);
}
}
}
}
Expand Down
39 changes: 34 additions & 5 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,9 +803,28 @@ pub enum TerminatorKind<'tcx> {
/// Indicates the end of the dropping of a generator
GeneratorDrop,

/// A block where control flow only ever takes one real path, but borrowck
/// needs to be more conservative.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay for comments 💯

FalseEdges {
/// The target normal control flow will take
real_target: BasicBlock,
imaginary_targets: Vec<BasicBlock>
/// The list of blocks control flow could conceptually take, but won't
/// in practice
imaginary_targets: Vec<BasicBlock>,
},
/// A terminator for blocks that only take one path in reality, but where we
/// reserve the right to unwind in borrowck, even if it won't happen in practice.
/// This can arise in infinite loops with no function calls for example.
FalseUnwind {
/// The target normal control flow will take
real_target: BasicBlock,
/// The imaginary cleanup block link. This particular path will never be taken
/// in practice, but in order to avoid fragility we want to always
/// consider it in borrowck. We don't want to accept programs which
/// pass borrowck only when panic=abort or some assertions are disabled
/// due to release vs. debug mode builds. This needs to be an Option because
/// of the remove_noop_landing_pads and no_landing_pads passes
unwind: Option<BasicBlock>,
},
}

Expand Down Expand Up @@ -865,6 +884,8 @@ impl<'tcx> TerminatorKind<'tcx> {
s.extend_from_slice(imaginary_targets);
s.into_cow()
}
FalseUnwind { real_target: t, unwind: Some(u) } => vec![t, u].into_cow(),
FalseUnwind { real_target: ref t, unwind: None } => slice::from_ref(t).into_cow(),
}
}

Expand Down Expand Up @@ -897,6 +918,8 @@ impl<'tcx> TerminatorKind<'tcx> {
s.extend(imaginary_targets.iter_mut());
s
}
FalseUnwind { real_target: ref mut t, unwind: Some(ref mut u) } => vec![t, u],
FalseUnwind { ref mut real_target, unwind: None } => vec![real_target],
}
}

Expand All @@ -916,7 +939,8 @@ impl<'tcx> TerminatorKind<'tcx> {
TerminatorKind::Call { cleanup: ref mut unwind, .. } |
TerminatorKind::Assert { cleanup: ref mut unwind, .. } |
TerminatorKind::DropAndReplace { ref mut unwind, .. } |
TerminatorKind::Drop { ref mut unwind, .. } => {
TerminatorKind::Drop { ref mut unwind, .. } |
TerminatorKind::FalseUnwind { ref mut unwind, .. } => {
Some(unwind)
}
}
Expand Down Expand Up @@ -1045,7 +1069,8 @@ impl<'tcx> TerminatorKind<'tcx> {

write!(fmt, ")")
},
FalseEdges { .. } => write!(fmt, "falseEdges")
FalseEdges { .. } => write!(fmt, "falseEdges"),
FalseUnwind { .. } => write!(fmt, "falseUnwind"),
}
}

Expand Down Expand Up @@ -1087,6 +1112,8 @@ impl<'tcx> TerminatorKind<'tcx> {
l.resize(imaginary_targets.len() + 1, "imaginary".into());
l
}
FalseUnwind { unwind: Some(_), .. } => vec!["real".into(), "cleanup".into()],
FalseUnwind { unwind: None, .. } => vec!["real".into()],
}
}
}
Expand Down Expand Up @@ -2189,7 +2216,8 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
Return => Return,
Unreachable => Unreachable,
FalseEdges { real_target, ref imaginary_targets } =>
FalseEdges { real_target, imaginary_targets: imaginary_targets.clone() }
FalseEdges { real_target, imaginary_targets: imaginary_targets.clone() },
FalseUnwind { real_target, unwind } => FalseUnwind { real_target, unwind },
};
Terminator {
source_info: self.source_info,
Expand Down Expand Up @@ -2231,7 +2259,8 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
Return |
GeneratorDrop |
Unreachable |
FalseEdges { .. } => false
FalseEdges { .. } |
FalseUnwind { .. } => false
}
}
}
Expand Down
10 changes: 8 additions & 2 deletions src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,15 +495,21 @@ macro_rules! make_mir_visitor {
self.visit_operand(value, source_location);
self.visit_branch(block, resume);
drop.map(|t| self.visit_branch(block, t));

}

TerminatorKind::FalseEdges { real_target, ref imaginary_targets } => {
TerminatorKind::FalseEdges { real_target, ref imaginary_targets} => {
self.visit_branch(block, real_target);
for target in imaginary_targets {
self.visit_branch(block, *target);
}
}

TerminatorKind::FalseUnwind { real_target, unwind } => {
self.visit_branch(block, real_target);
if let Some(unwind) = unwind {
self.visit_branch(block, unwind);
}
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,8 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx
TerminatorKind::Goto { target: _ }
| TerminatorKind::Abort
| TerminatorKind::Unreachable
| TerminatorKind::FalseEdges { .. } => {
| TerminatorKind::FalseEdges { real_target: _, imaginary_targets: _ }
| TerminatorKind::FalseUnwind { real_target: _, unwind: _ } => {
// no data used, thus irrelevant to borrowck
}
}
Expand Down
15 changes: 14 additions & 1 deletion src/librustc_mir/borrow_check/nll/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,8 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
| TerminatorKind::GeneratorDrop
| TerminatorKind::Unreachable
| TerminatorKind::Drop { .. }
| TerminatorKind::FalseEdges { .. } => {
| TerminatorKind::FalseEdges { .. }
| TerminatorKind::FalseUnwind { .. } => {
// no checks needed for these
}

Expand Down Expand Up @@ -1152,6 +1153,18 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
self.assert_iscleanup(mir, block_data, *target, is_cleanup);
}
}
TerminatorKind::FalseUnwind {
real_target,
unwind
} => {
self.assert_iscleanup(mir, block_data, real_target, is_cleanup);
if let Some(unwind) = unwind {
if is_cleanup {
span_mirbug!(self, block_data, "cleanup in cleanup block via false unwind");
}
self.assert_iscleanup(mir, block_data, unwind, true);
}
}
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
TerminatorKind::FalseEdges {
real_target: block,
imaginary_targets:
vec![candidate.next_candidate_pre_binding_block]});
vec![candidate.next_candidate_pre_binding_block],
});

self.bind_matched_candidate(block, candidate.bindings);

Expand All @@ -749,7 +750,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
TerminatorKind::FalseEdges {
real_target: otherwise,
imaginary_targets:
vec![candidate.next_candidate_pre_binding_block] });
vec![candidate.next_candidate_pre_binding_block],
});
Some(otherwise)
} else {
self.cfg.terminate(block, candidate_source_info,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/dataflow/impls/borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,7 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
mir::TerminatorKind::Yield {..} |
mir::TerminatorKind::Goto {..} |
mir::TerminatorKind::FalseEdges {..} |
mir::TerminatorKind::FalseUnwind {..} |
mir::TerminatorKind::Unreachable => {}
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/librustc_mir/dataflow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,14 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation
self.propagate_bits_into_entry_set_for(in_out, changed, target);
}
}
mir::TerminatorKind::FalseUnwind { ref real_target, unwind } => {
self.propagate_bits_into_entry_set_for(in_out, changed, real_target);
if let Some(ref unwind) = unwind {
if !self.dead_unwinds.contains(&bb) {
self.propagate_bits_into_entry_set_for(in_out, changed, unwind);
}
}
}
}
}

Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/dataflow/move_paths/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
TerminatorKind::Abort |
TerminatorKind::GeneratorDrop |
TerminatorKind::FalseEdges { .. } |
TerminatorKind::FalseUnwind { .. } |
TerminatorKind::Unreachable => { }

TerminatorKind::Return => {
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/interpret/terminator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> {
Resume => unimplemented!(),
Abort => unimplemented!(),
FalseEdges { .. } => bug!("should have been eliminated by `simplify_branches` mir pass"),
FalseUnwind { .. } => bug!("should have been eliminated by `simplify_branches` mir pass"),
Unreachable => return err!(Unreachable),
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/monomorphize/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,8 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
mir::TerminatorKind::Assert { .. } => {}
mir::TerminatorKind::GeneratorDrop |
mir::TerminatorKind::Yield { .. } |
mir::TerminatorKind::FalseEdges { .. } => bug!(),
mir::TerminatorKind::FalseEdges { .. } |
mir::TerminatorKind::FalseUnwind { .. } => bug!(),
}

self.super_terminator_kind(block, kind, location);
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/transform/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
TerminatorKind::Abort |
TerminatorKind::Return |
TerminatorKind::Unreachable |
TerminatorKind::FalseEdges { .. } => {
TerminatorKind::FalseEdges { .. } |
TerminatorKind::FalseUnwind { .. } => {
// safe (at least as emitted during MIR construction)
}

Expand Down
3 changes: 3 additions & 0 deletions src/librustc_mir/transform/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,9 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Integrator<'a, 'tcx> {
*target = self.update_target(*target);
}
}
TerminatorKind::FalseUnwind { real_target: _ , unwind: _ } =>
// see the ordering of passes in the optimized_mir query.
bug!("False unwinds should have been removed before inlining")
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/transform/qualify_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,8 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
TerminatorKind::GeneratorDrop |
TerminatorKind::Yield { .. } |
TerminatorKind::Unreachable |
TerminatorKind::FalseEdges { .. } => None,
TerminatorKind::FalseEdges { .. } |
TerminatorKind::FalseUnwind { .. } => None,

TerminatorKind::Return => {
// Check for unused values. This usually means
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/transform/remove_noop_landing_pads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ impl RemoveNoopLandingPads {
TerminatorKind::Goto { .. } |
TerminatorKind::Resume |
TerminatorKind::SwitchInt { .. } |
TerminatorKind::FalseEdges { .. } => {
TerminatorKind::FalseEdges { .. } |
TerminatorKind::FalseUnwind { .. } => {
terminator.successors().iter().all(|succ| {
nop_landing_pads.contains(succ.index())
})
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_mir/transform/simplify_branches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ impl MirPass for SimplifyBranches {
TerminatorKind::FalseEdges { real_target, .. } => {
TerminatorKind::Goto { target: real_target }
},
TerminatorKind::FalseUnwind { real_target, .. } => {
TerminatorKind::Goto { target: real_target }
},
_ => continue
};
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_passes/mir_stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ impl<'a, 'tcx> mir_visit::Visitor<'tcx> for StatCollector<'a, 'tcx> {
TerminatorKind::GeneratorDrop => "TerminatorKind::GeneratorDrop",
TerminatorKind::Yield { .. } => "TerminatorKind::Yield",
TerminatorKind::FalseEdges { .. } => "TerminatorKind::FalseEdges",
TerminatorKind::FalseUnwind { .. } => "TerminatorKind::FalseUnwind",
}, kind);
self.super_terminator_kind(block, kind, location);
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_trans/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,8 @@ pub fn cleanup_kinds<'a, 'tcx>(mir: &mir::Mir<'tcx>) -> IndexVec<mir::BasicBlock
TerminatorKind::Unreachable |
TerminatorKind::SwitchInt { .. } |
TerminatorKind::Yield { .. } |
TerminatorKind::FalseEdges { .. } => {
TerminatorKind::FalseEdges { .. } |
TerminatorKind::FalseUnwind { .. } => {
/* nothing to do */
}
TerminatorKind::Call { cleanup: unwind, .. } |
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_trans/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,8 +608,9 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
cleanup);
}
mir::TerminatorKind::GeneratorDrop |
mir::TerminatorKind::Yield { .. } |
mir::TerminatorKind::FalseEdges { .. } => bug!("generator ops in trans"),
mir::TerminatorKind::Yield { .. } => bug!("generator ops in trans"),
mir::TerminatorKind::FalseEdges { .. } |
mir::TerminatorKind::FalseUnwind { .. } => bug!("borrowck false edges in trans"),
}
}

Expand Down