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 all commits
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
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
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 @@ -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
Loading