Auto merge of rust-lang#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 rust-lang#88043.
rust-lang-ci · Sep 19, 2021 · 10967a1 · 10967a1
2 parents e20a4ec + f1f1c8f
commit 10967a1
Show file tree

Hide file tree

Showing 9 changed files with 84 additions and 110 deletions.
diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs
@@ -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) {

diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs
@@ -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) {

diff --git a/compiler/rustc_mir_transform/src/simplify.rs b/compiler/rustc_mir_transform/src/simplify.rs
@@ -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

diff --git a/src/test/mir-opt/coverage_graphviz.main.InstrumentCoverage.0.dot b/src/test/mir-opt/coverage_graphviz.main.InstrumentCoverage.0.dot
@@ -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=<>];
 }
diff --git a/src/test/mir-opt/inline/inline_diverging.h.Inline.diff b/src/test/mir-opt/inline/inline_diverging.h.Inline.diff
@@ -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: {

diff --git a/src/test/mir-opt/instrument_coverage.main.InstrumentCoverage.diff b/src/test/mir-opt/instrument_coverage.main.InstrumentCoverage.diff
@@ -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
       }
   }

diff --git a/src/test/mir-opt/simplify_cfg.main.SimplifyCfg-early-opt.diff b/src/test/mir-opt/simplify_cfg.main.SimplifyCfg-early-opt.diff
@@ -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
+      }
+
+-     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
       }
   }

diff --git a/src/test/mir-opt/simplify_cfg.main.SimplifyCfg-initial.diff b/src/test/mir-opt/simplify_cfg.main.SimplifyCfg-initial.diff
@@ -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
       }
   }