Auto merge of #88968 - tmiasko:start-block-no-predecessors, r=davidtwco

Start block is not allowed to have basic block predecessors

* The MIR validator is extended to detect potential violations.
* The start block has no predecessors after building MIR, so no changes are required there.
* The SimplifyCfg could previously violate this requirement when collapsing goto chains, so transformation is disabled for the start block, which also substantially simplifies the implementation.
* The LLVM function entry block also must not have basic block predecessors. Previously, to ensure that code generation had to perform necessary adjustments. This now became unnecessary.

The motivation behind the change is to align with analogous requirement in LLVM, and to avoid potential latent bugs like the one reported in #88043.
This commit is contained in:
bors 2021-09-19 01:38:17 +00:00
commit 10967a1dcc
9 changed files with 85 additions and 111 deletions

View File

@ -152,20 +152,11 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
}
let cleanup_kinds = analyze::cleanup_kinds(&mir);
// Allocate a `Block` for every basic block, except
// the start block, if nothing loops back to it.
let reentrant_start_block = !mir.predecessors()[mir::START_BLOCK].is_empty();
let cached_llbbs: IndexVec<mir::BasicBlock, Option<Bx::BasicBlock>> =
mir.basic_blocks()
.indices()
.map(|bb| {
if bb == mir::START_BLOCK && !reentrant_start_block {
Some(start_llbb)
} else {
None
}
})
.collect();
let cached_llbbs: IndexVec<mir::BasicBlock, Option<Bx::BasicBlock>> = mir
.basic_blocks()
.indices()
.map(|bb| if bb == mir::START_BLOCK { Some(start_llbb) } else { None })
.collect();
let mut fx = FunctionCx {
instance,
@ -247,11 +238,6 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
// Apply debuginfo to the newly allocated locals.
fx.debug_introduce_locals(&mut bx);
// Branch to the START block, if it's not the entry block.
if reentrant_start_block {
bx.br(fx.llbb(mir::START_BLOCK));
}
// Codegen the body of each block using reverse postorder
// FIXME(eddyb) reuse RPO iterator between `analysis` and this.
for (bb, _) in traversal::reverse_postorder(&mir) {

View File

@ -9,7 +9,7 @@ use rustc_middle::mir::visit::{PlaceContext, Visitor};
use rustc_middle::mir::{
AggregateKind, BasicBlock, Body, BorrowKind, Local, Location, MirPhase, Operand, PlaceElem,
PlaceRef, ProjectionElem, Rvalue, SourceScope, Statement, StatementKind, Terminator,
TerminatorKind,
TerminatorKind, START_BLOCK,
};
use rustc_middle::ty::fold::BottomUpFolder;
use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt, TypeFoldable};
@ -130,6 +130,9 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
fn check_edge(&self, location: Location, bb: BasicBlock, edge_kind: EdgeKind) {
if bb == START_BLOCK {
self.fail(location, "start block must not have predecessors")
}
if let Some(bb) = self.body.basic_blocks().get(bb) {
let src = self.body.basic_blocks().get(location.block).unwrap();
match (src.is_cleanup, bb.is_cleanup, edge_kind) {

View File

@ -95,8 +95,6 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
pub fn simplify(mut self) {
self.strip_nops();
let mut start = START_BLOCK;
// Vec of the blocks that should be merged. We store the indices here, instead of the
// statements itself to avoid moving the (relatively) large statements twice.
// We do not push the statements directly into the target block (`bb`) as that is slower
@ -105,8 +103,6 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
loop {
let mut changed = false;
self.collapse_goto_chain(&mut start, &mut changed);
for bb in self.basic_blocks.indices() {
if self.pred_count[bb] == 0 {
continue;
@ -149,27 +145,6 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
break;
}
}
if start != START_BLOCK {
debug_assert!(self.pred_count[START_BLOCK] == 0);
self.basic_blocks.swap(START_BLOCK, start);
self.pred_count.swap(START_BLOCK, start);
// pred_count == 1 if the start block has no predecessor _blocks_.
if self.pred_count[START_BLOCK] > 1 {
for (bb, data) in self.basic_blocks.iter_enumerated_mut() {
if self.pred_count[bb] == 0 {
continue;
}
for target in data.terminator_mut().successors_mut() {
if *target == start {
*target = START_BLOCK;
}
}
}
}
}
}
/// This function will return `None` if

View File

@ -2,10 +2,12 @@ digraph Cov_0_3 {
graph [fontname="Courier, monospace"];
node [fontname="Courier, monospace"];
edge [fontname="Courier, monospace"];
bcb2__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb2</td></tr><tr><td align="left" balign="left">Expression(bcb0 - bcb1) at 13:10-13:10<br/> 13:10-13:10: @4[0]: Coverage::Expression(4294967295) = 1 - 2 for $DIR/coverage_graphviz.rs:13:10 - 13:11</td></tr><tr><td align="left" balign="left">bb4: Goto</td></tr></table>>];
bcb1__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb1</td></tr><tr><td align="left" balign="left">Counter(bcb1) at 12:13-12:18<br/> 12:13-12:18: @3[0]: Coverage::Expression(4294967294) = 2 + 0 for $DIR/coverage_graphviz.rs:15:1 - 15:2<br/>Expression(bcb1 + 0) at 15:2-15:2<br/> 15:2-15:2: @3.Return: return</td></tr><tr><td align="left" balign="left">bb3: Return</td></tr></table>>];
bcb0__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb0</td></tr><tr><td align="left" balign="left"></td></tr><tr><td align="left" balign="left">Counter(bcb0) at 9:1-11:17<br/> 11:12-11:17: @1.Call: _2 = bar() -&gt; [return: bb2, unwind: bb5]</td></tr><tr><td align="left" balign="left">bb0: FalseUnwind<br/>bb1: Call</td></tr><tr><td align="left" balign="left">bb2: SwitchInt</td></tr></table>>];
bcb2__Cov_0_3 -> bcb0__Cov_0_3 [label=<>];
bcb0__Cov_0_3 -> bcb2__Cov_0_3 [label=<false>];
bcb0__Cov_0_3 -> bcb1__Cov_0_3 [label=<otherwise>];
bcb3__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb3</td></tr><tr><td align="left" balign="left">Counter(bcb3) at 13:10-13:10<br/> 13:10-13:10: @5[0]: Coverage::Counter(2) for $DIR/coverage_graphviz.rs:13:10 - 13:11</td></tr><tr><td align="left" balign="left">bb5: Goto</td></tr></table>>];
bcb2__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb2</td></tr><tr><td align="left" balign="left">Expression(bcb1:(bcb0 + bcb3) - bcb3) at 12:13-12:18<br/> 12:13-12:18: @4[0]: Coverage::Expression(4294967293) = 4294967294 + 0 for $DIR/coverage_graphviz.rs:15:1 - 15:2<br/>Expression(bcb2:(bcb1:(bcb0 + bcb3) - bcb3) + 0) at 15:2-15:2<br/> 15:2-15:2: @4.Return: return</td></tr><tr><td align="left" balign="left">bb4: Return</td></tr></table>>];
bcb1__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb1</td></tr><tr><td align="left" balign="left">Expression(bcb0 + bcb3) at 10:5-11:17<br/> 11:12-11:17: @2.Call: _2 = bar() -&gt; [return: bb3, unwind: bb6]</td></tr><tr><td align="left" balign="left">bb1: FalseUnwind<br/>bb2: Call</td></tr><tr><td align="left" balign="left">bb3: SwitchInt</td></tr></table>>];
bcb0__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb0</td></tr><tr><td align="left" balign="left"></td></tr><tr><td align="left" balign="left">Counter(bcb0) at 9:1-9:11<br/> </td></tr><tr><td align="left" balign="left">bb0: Goto</td></tr></table>>];
bcb3__Cov_0_3 -> bcb1__Cov_0_3 [label=<>];
bcb1__Cov_0_3 -> bcb3__Cov_0_3 [label=<false>];
bcb1__Cov_0_3 -> bcb2__Cov_0_3 [label=<otherwise>];
bcb0__Cov_0_3 -> bcb1__Cov_0_3 [label=<>];
}

View File

@ -47,7 +47,7 @@
+ _4 = &_2; // scope 1 at $DIR/inline-diverging.rs:22:5: 22:22
+ StorageLive(_9); // scope 1 at $DIR/inline-diverging.rs:22:5: 22:22
+ _9 = const (); // scope 1 at $DIR/inline-diverging.rs:22:5: 22:22
+ goto -> bb1; // scope 4 at $DIR/inline-diverging.rs:22:5: 22:22
+ goto -> bb1; // scope 5 at $DIR/inline-diverging.rs:22:5: 22:22
}
bb1: {

View File

@ -8,38 +8,43 @@
let mut _3: !; // in scope 0 at /the/src/instrument_coverage.rs:12:18: 14:10
bb0: {
+ Coverage::Counter(1) for /the/src/instrument_coverage.rs:10:1 - 12:17; // scope 0 at /the/src/instrument_coverage.rs:11:5: 15:6
falseUnwind -> [real: bb1, cleanup: bb5]; // scope 0 at /the/src/instrument_coverage.rs:11:5: 15:6
+ Coverage::Counter(1) for /the/src/instrument_coverage.rs:10:1 - 10:11; // scope 0 at /the/src/instrument_coverage.rs:11:5: 15:6
goto -> bb1; // scope 0 at /the/src/instrument_coverage.rs:11:5: 15:6
}
bb1: {
+ Coverage::Expression(4294967295) = 1 + 2 for /the/src/instrument_coverage.rs:11:5 - 12:17; // scope 0 at /the/src/instrument_coverage.rs:11:5: 15:6
falseUnwind -> [real: bb2, cleanup: bb6]; // scope 0 at /the/src/instrument_coverage.rs:11:5: 15:6
}
bb2: {
StorageLive(_2); // scope 0 at /the/src/instrument_coverage.rs:12:12: 12:17
_2 = bar() -> [return: bb2, unwind: bb5]; // scope 0 at /the/src/instrument_coverage.rs:12:12: 12:17
_2 = bar() -> [return: bb3, unwind: bb6]; // scope 0 at /the/src/instrument_coverage.rs:12:12: 12:17
// mir::Constant
// + span: /the/src/instrument_coverage.rs:12:12: 12:15
// + literal: Const { ty: fn() -> bool {bar}, val: Value(Scalar(<ZST>)) }
}
bb2: {
switchInt(move _2) -> [false: bb4, otherwise: bb3]; // scope 0 at /the/src/instrument_coverage.rs:12:12: 12:17
bb3: {
switchInt(move _2) -> [false: bb5, otherwise: bb4]; // scope 0 at /the/src/instrument_coverage.rs:12:12: 12:17
}
bb3: {
+ Coverage::Expression(4294967294) = 2 + 0 for /the/src/instrument_coverage.rs:16:1 - 16:2; // scope 0 at /the/src/instrument_coverage.rs:16:2: 16:2
+ Coverage::Counter(2) for /the/src/instrument_coverage.rs:13:13 - 13:18; // scope 0 at /the/src/instrument_coverage.rs:16:2: 16:2
bb4: {
+ Coverage::Expression(4294967293) = 4294967294 + 0 for /the/src/instrument_coverage.rs:16:1 - 16:2; // scope 0 at /the/src/instrument_coverage.rs:16:2: 16:2
+ Coverage::Expression(4294967294) = 4294967295 - 2 for /the/src/instrument_coverage.rs:13:13 - 13:18; // scope 0 at /the/src/instrument_coverage.rs:16:2: 16:2
_0 = const (); // scope 0 at /the/src/instrument_coverage.rs:13:13: 13:18
StorageDead(_2); // scope 0 at /the/src/instrument_coverage.rs:14:9: 14:10
return; // scope 0 at /the/src/instrument_coverage.rs:16:2: 16:2
}
bb4: {
+ Coverage::Expression(4294967295) = 1 - 2 for /the/src/instrument_coverage.rs:14:10 - 14:11; // scope 0 at /the/src/instrument_coverage.rs:11:5: 15:6
bb5: {
+ Coverage::Counter(2) for /the/src/instrument_coverage.rs:14:10 - 14:11; // scope 0 at /the/src/instrument_coverage.rs:11:5: 15:6
_1 = const (); // scope 0 at /the/src/instrument_coverage.rs:14:10: 14:10
StorageDead(_2); // scope 0 at /the/src/instrument_coverage.rs:14:9: 14:10
goto -> bb0; // scope 0 at /the/src/instrument_coverage.rs:11:5: 15:6
goto -> bb1; // scope 0 at /the/src/instrument_coverage.rs:11:5: 15:6
}
bb5 (cleanup): {
bb6 (cleanup): {
resume; // scope 0 at /the/src/instrument_coverage.rs:10:1: 16:2
}
}

View File

@ -8,40 +8,44 @@
let mut _3: !; // in scope 0 at $DIR/simplify_cfg.rs:9:18: 11:10
bb0: {
- goto -> bb1; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
goto -> bb1; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
}
bb1: {
- goto -> bb2; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
- }
-
- bb1: {
- bb2: {
StorageLive(_2); // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
- _2 = bar() -> [return: bb2, unwind: bb5]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
+ _2 = bar() -> [return: bb1, unwind: bb4]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
- _2 = bar() -> [return: bb3, unwind: bb6]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
+ _2 = bar() -> [return: bb2, unwind: bb5]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
// mir::Constant
// + span: $DIR/simplify_cfg.rs:9:12: 9:15
// + literal: Const { ty: fn() -> bool {bar}, val: Value(Scalar(<ZST>)) }
}
- bb2: {
- switchInt(move _2) -> [false: bb4, otherwise: bb3]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
+ bb1: {
+ switchInt(move _2) -> [false: bb3, otherwise: bb2]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
- bb3: {
- switchInt(move _2) -> [false: bb5, otherwise: bb4]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
+ bb2: {
+ switchInt(move _2) -> [false: bb4, otherwise: bb3]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
}
- bb3: {
+ bb2: {
- bb4: {
+ bb3: {
_0 = const (); // scope 0 at $DIR/simplify_cfg.rs:10:13: 10:18
StorageDead(_2); // scope 0 at $DIR/simplify_cfg.rs:11:9: 11:10
return; // scope 0 at $DIR/simplify_cfg.rs:13:2: 13:2
}
- bb4: {
+ bb3: {
- bb5: {
+ bb4: {
_1 = const (); // scope 0 at $DIR/simplify_cfg.rs:11:10: 11:10
StorageDead(_2); // scope 0 at $DIR/simplify_cfg.rs:11:9: 11:10
goto -> bb0; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
goto -> bb1; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
}
- bb5 (cleanup): {
+ bb4 (cleanup): {
- bb6 (cleanup): {
+ bb5 (cleanup): {
resume; // scope 0 at $DIR/simplify_cfg.rs:7:1: 13:2
}
}

View File

@ -8,38 +8,35 @@
let mut _3: !; // in scope 0 at $DIR/simplify_cfg.rs:9:18: 11:10
bb0: {
- goto -> bb1; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
+ falseUnwind -> [real: bb1, cleanup: bb5]; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
goto -> bb1; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
}
bb1: {
- falseUnwind -> [real: bb2, cleanup: bb11]; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
- }
-
- bb2: {
+ falseUnwind -> [real: bb2, cleanup: bb6]; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
}
bb2: {
StorageLive(_2); // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
- _2 = bar() -> [return: bb3, unwind: bb11]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
+ _2 = bar() -> [return: bb2, unwind: bb5]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
+ _2 = bar() -> [return: bb3, unwind: bb6]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
// mir::Constant
// + span: $DIR/simplify_cfg.rs:9:12: 9:15
// + literal: Const { ty: fn() -> bool {bar}, val: Value(Scalar(<ZST>)) }
}
- bb3: {
- switchInt(move _2) -> [false: bb5, otherwise: bb4]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
+ bb2: {
+ switchInt(move _2) -> [false: bb4, otherwise: bb3]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
bb3: {
switchInt(move _2) -> [false: bb5, otherwise: bb4]; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
}
- bb4: {
+ bb3: {
bb4: {
_0 = const (); // scope 0 at $DIR/simplify_cfg.rs:10:13: 10:18
- goto -> bb10; // scope 0 at $DIR/simplify_cfg.rs:10:13: 10:18
+ StorageDead(_2); // scope 0 at $DIR/simplify_cfg.rs:11:9: 11:10
+ return; // scope 0 at $DIR/simplify_cfg.rs:13:2: 13:2
}
- bb5: {
bb5: {
- goto -> bb8; // scope 0 at $DIR/simplify_cfg.rs:9:12: 9:17
- }
-
@ -52,15 +49,13 @@
- }
-
- bb8: {
+ bb4: {
_1 = const (); // scope 0 at $DIR/simplify_cfg.rs:11:10: 11:10
- goto -> bb9; // scope 0 at $DIR/simplify_cfg.rs:9:9: 11:10
- }
-
- bb9: {
StorageDead(_2); // scope 0 at $DIR/simplify_cfg.rs:11:9: 11:10
- goto -> bb1; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
+ goto -> bb0; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
goto -> bb1; // scope 0 at $DIR/simplify_cfg.rs:8:5: 12:6
}
- bb10: {
@ -69,7 +64,7 @@
- }
-
- bb11 (cleanup): {
+ bb5 (cleanup): {
+ bb6 (cleanup): {
resume; // scope 0 at $DIR/simplify_cfg.rs:7:1: 13:2
}
}

View File

@ -9,51 +9,55 @@ fn while_loop(_1: bool) -> () {
let mut _5: bool; // in scope 0 at $DIR/while-storage.rs:11:21: 11:22
bb0: {
goto -> bb1; // scope 0 at $DIR/while-storage.rs:10:5: 14:6
}
bb1: {
StorageLive(_2); // scope 0 at $DIR/while-storage.rs:10:11: 10:22
StorageLive(_3); // scope 0 at $DIR/while-storage.rs:10:20: 10:21
_3 = _1; // scope 0 at $DIR/while-storage.rs:10:20: 10:21
_2 = get_bool(move _3) -> bb1; // scope 0 at $DIR/while-storage.rs:10:11: 10:22
_2 = get_bool(move _3) -> bb2; // scope 0 at $DIR/while-storage.rs:10:11: 10:22
// mir::Constant
// + span: $DIR/while-storage.rs:10:11: 10:19
// + literal: Const { ty: fn(bool) -> bool {get_bool}, val: Value(Scalar(<ZST>)) }
}
bb1: {
bb2: {
StorageDead(_3); // scope 0 at $DIR/while-storage.rs:10:21: 10:22
switchInt(move _2) -> [false: bb6, otherwise: bb2]; // scope 0 at $DIR/while-storage.rs:10:11: 10:22
switchInt(move _2) -> [false: bb7, otherwise: bb3]; // scope 0 at $DIR/while-storage.rs:10:11: 10:22
}
bb2: {
bb3: {
StorageLive(_4); // scope 0 at $DIR/while-storage.rs:11:12: 11:23
StorageLive(_5); // scope 0 at $DIR/while-storage.rs:11:21: 11:22
_5 = _1; // scope 0 at $DIR/while-storage.rs:11:21: 11:22
_4 = get_bool(move _5) -> bb3; // scope 0 at $DIR/while-storage.rs:11:12: 11:23
_4 = get_bool(move _5) -> bb4; // scope 0 at $DIR/while-storage.rs:11:12: 11:23
// mir::Constant
// + span: $DIR/while-storage.rs:11:12: 11:20
// + literal: Const { ty: fn(bool) -> bool {get_bool}, val: Value(Scalar(<ZST>)) }
}
bb3: {
StorageDead(_5); // scope 0 at $DIR/while-storage.rs:11:22: 11:23
switchInt(move _4) -> [false: bb5, otherwise: bb4]; // scope 0 at $DIR/while-storage.rs:11:12: 11:23
}
bb4: {
StorageDead(_4); // scope 0 at $DIR/while-storage.rs:13:9: 13:10
goto -> bb7; // scope 0 at no-location
StorageDead(_5); // scope 0 at $DIR/while-storage.rs:11:22: 11:23
switchInt(move _4) -> [false: bb6, otherwise: bb5]; // scope 0 at $DIR/while-storage.rs:11:12: 11:23
}
bb5: {
StorageDead(_4); // scope 0 at $DIR/while-storage.rs:13:9: 13:10
StorageDead(_2); // scope 0 at $DIR/while-storage.rs:14:5: 14:6
goto -> bb0; // scope 0 at $DIR/while-storage.rs:10:5: 14:6
goto -> bb8; // scope 0 at no-location
}
bb6: {
goto -> bb7; // scope 0 at no-location
StorageDead(_4); // scope 0 at $DIR/while-storage.rs:13:9: 13:10
StorageDead(_2); // scope 0 at $DIR/while-storage.rs:14:5: 14:6
goto -> bb1; // scope 0 at $DIR/while-storage.rs:10:5: 14:6
}
bb7: {
goto -> bb8; // scope 0 at no-location
}
bb8: {
StorageDead(_2); // scope 0 at $DIR/while-storage.rs:14:5: 14:6
return; // scope 0 at $DIR/while-storage.rs:15:2: 15:2
}