Cranelift: disallow marking entry block 'cold'. (#4659)

This is a nonsensical constraint: the entry block must come first in the
compiled code's layout, so it cannot also be sunk to the end of the
function.

This PR modifies the CLIF verifier to disallow this situation entirely.
It also adds an assert during final block-order computation to catch the
problem (and avoid a silent miscompile) even if the verifier is
disabled.

Fixes #4656.

cranelift/codegen/src/machinst/blockorder.rs

@@ -216,6 +216,13 @@ impl BlockLoweringOrder {
     pub fn new(f: &Function) -> BlockLoweringOrder {
         trace!("BlockLoweringOrder: function body {:?}", f);
 
+        // Make sure that we have an entry block, and the entry block is
+        // not marked as cold. (The verifier ensures this as well, but
+        // the user may not have run the verifier, and this property is
+        // critical to avoid a miscompile, so we assert it here too.)
+        let entry = f.layout.entry_block().expect("Must have entry block");
+        assert!(!f.layout.is_cold(entry));
+
         // Step 1: compute the in-edge and out-edge count of every block.
         let mut block_in_count = SecondaryMap::with_default(0);
         let mut block_out_count = SecondaryMap::with_default(0);
@@ -243,9 +250,7 @@ impl BlockLoweringOrder {
             }
         }
         // Implicit input edge for entry block.
-        if let Some(entry) = f.layout.entry_block() {
-            block_in_count[entry] += 1;
-        }
+        block_in_count[entry] += 1;
 
         // All blocks ending in conditional branches or br_tables must
         // have edge-moves inserted at the top of successor blocks,
@@ -376,19 +381,20 @@ impl BlockLoweringOrder {
         let mut stack: SmallVec<[StackEntry; 16]> = SmallVec::new();
         let mut visited = FxHashSet::default();
         let mut postorder = vec![];
-        if let Some(entry) = f.layout.entry_block() {
-            // FIXME(cfallin): we might be able to use OrigAndEdge. Find a way
-            // to not special-case the entry block here.
-            let block = LoweredBlock::Orig { block: entry };
-            visited.insert(block);
-            let range = compute_lowered_succs(&mut lowered_succs, block);
-            lowered_succ_indices.resize(lowered_succs.len(), 0);
-            stack.push(StackEntry {
-                this: block,
-                succs: range,
-                cur_succ: range.1,
-            });
-        }
+
+        // Add the entry block.
+        //
+        // FIXME(cfallin): we might be able to use OrigAndEdge. Find a
+        // way to not special-case the entry block here.
+        let block = LoweredBlock::Orig { block: entry };
+        visited.insert(block);
+        let range = compute_lowered_succs(&mut lowered_succs, block);
+        lowered_succ_indices.resize(lowered_succs.len(), 0);
+        stack.push(StackEntry {
+            this: block,
+            succs: range,
+            cur_succ: range.1,
+        });
 
         while !stack.is_empty() {
             let stack_entry = stack.last_mut().unwrap();

cranelift/codegen/src/verifier/mod.rs

@@ -27,6 +27,7 @@
 //! - All predecessors in the CFG must be branches to the block.
 //! - All branches to a block must be present in the CFG.
 //! - A recomputed dominator tree is identical to the existing one.
+//! - The entry block must not be a cold block.
 //!
 //! Type checking
 //!
@@ -1224,6 +1225,16 @@ impl<'a> Verifier<'a> {
         errors.as_result()
     }
 
+    fn check_entry_not_cold(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> {
+        if let Some(entry_block) = self.func.layout.entry_block() {
+            if self.func.layout.is_cold(entry_block) {
+                return errors
+                    .fatal((entry_block, format!("entry block cannot be marked as cold")));
+            }
+        }
+        errors.as_result()
+    }
+
     fn typecheck(&self, inst: Inst, errors: &mut VerifierErrors) -> VerifierStepResult<()> {
         let inst_data = &self.func.dfg[inst];
         let constraints = inst_data.opcode().constraints();
@@ -1759,6 +1770,7 @@ impl<'a> Verifier<'a> {
         self.verify_tables(errors)?;
         self.verify_jump_tables(errors)?;
         self.typecheck_entry_block_params(errors)?;
+        self.check_entry_not_cold(errors)?;
         self.typecheck_function_signature(errors)?;
 
         for block in self.func.layout.blocks() {

cranelift/filetests/filetests/verifier/cold_entry.clif

@@ -0,0 +1,6 @@
+test verifier
+
+function %entry_block_not_cold() {
+    block0 cold: ; error: entry block cannot be marked as cold
+        return
+}