Skip to content

Commit

Permalink
Auto merge of #47802 - bobtwinkles:loop_false_edge, r=nikomatsakis
Browse files Browse the repository at this point in the history
[NLL] Add false edges out of infinite loops

Resolves #46036 by adding a `cleanup` member to the `FalseEdges` terminator kind. There's also a small doc fix to one of the other comments in `into.rs` which I can pull out in to another PR if desired =)

This PR should pass CI but the test suite has been relatively unstable on my system so I'm not 100% sure.

r? @nikomatsakis
  • Loading branch information
bors committed Feb 9, 2018
2 parents 02537fb + 85dfa9d commit 3bcda48
Show file tree
Hide file tree
Showing 29 changed files with 261 additions and 74 deletions.
4 changes: 4 additions & 0 deletions src/librustc/ich/impls_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,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 @@ -816,9 +816,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.
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 @@ -878,6 +897,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 @@ -910,6 +931,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 @@ -929,7 +952,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 @@ -1058,7 +1082,8 @@ impl<'tcx> TerminatorKind<'tcx> {

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

Expand Down Expand Up @@ -1100,6 +1125,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 @@ -2202,7 +2229,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 @@ -2244,7 +2272,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 @@ -575,7 +575,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
38 changes: 24 additions & 14 deletions src/librustc_mir/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// Or:
//
// [block: If(lhs)] -false-> [else_block: If(rhs)] -true-> [true_block]
// | | (false)
// +----------true------------+-------------------> [false_block]
// | (true) | (false)
// [true_block] [false_block]

let (true_block, false_block, mut else_block, join_block) =
(this.cfg.start_new_block(), this.cfg.start_new_block(),
Expand Down Expand Up @@ -147,20 +147,24 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
join_block.unit()
}
ExprKind::Loop { condition: opt_cond_expr, body } => {
// [block] --> [loop_block] ~~> [loop_block_end] -1-> [exit_block]
// ^ |
// | 0
// | |
// | v
// [body_block_end] <~~~ [body_block]
// [block] --> [loop_block] -/eval. cond./-> [loop_block_end] -1-> [exit_block]
// ^ |
// | 0
// | |
// | v
// [body_block_end] <-/eval. body/-- [body_block]
//
// If `opt_cond_expr` is `None`, then the graph is somewhat simplified:
//
// [block] --> [loop_block / body_block ] ~~> [body_block_end] [exit_block]
// ^ |
// | |
// +--------------------------+
//
// [block]
// |
// [loop_block] -> [body_block] -/eval. body/-> [body_block_end]
// | ^ |
// false link | |
// | +-----------------------------------------+
// +-> [diverge_cleanup]
// The false link is required to make sure borrowck considers unwinds through the
// body, even when the exact code in the body cannot unwind

let loop_block = this.cfg.start_new_block();
let exit_block = this.cfg.start_new_block();
Expand Down Expand Up @@ -188,7 +192,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// always `()` anyway
this.cfg.push_assign_unit(exit_block, source_info, destination);
} else {
body_block = loop_block;
body_block = this.cfg.start_new_block();
let diverge_cleanup = this.diverge_cleanup();
this.cfg.terminate(loop_block, source_info,
TerminatorKind::FalseUnwind {
real_target: body_block,
unwind: Some(diverge_cleanup)
})
}

// The “return” value of the loop body must always be an unit. We therefore
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 @@ -329,7 +329,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
Loading

0 comments on commit 3bcda48

Please sign in to comment.