Cranelift: fix a bug with inlining and unreachable callee blocks (#11282)

We copy *all* callee blocks into the caller's layout, but were then only copying
the callee instructions in *reachable* callee blocks into the caller. Therefore,
any *unreachable* blocks would remain empty in the caller, which is invalid CLIF
because all blocks must end in a terminator, so this commit adds a quick pass
over the inlined blocks to remove any empty blocks from the caller's layout.

Author: fitzgen

cranelift/codegen/src/inline.rs

@@ -414,6 +414,18 @@ fn inline_one(
         }
     }
 
+    // We copied *all* callee blocks into the caller's layout, but only copied
+    // the callee instructions in *reachable* callee blocks into the caller's
+    // associated blocks. Therefore, any *unreachable* blocks are empty in the
+    // caller, which is invalid CLIF because all blocks must end in a
+    // terminator, so do a quick pass over the inlined blocks and remove any
+    // empty blocks from the caller's layout.
+    for block in entity_map.iter_inlined_blocks(func) {
+        if func.layout.first_inst(block).is_none() {
+            func.layout.remove_block(block);
+        }
+    }
+
     // Final step: fixup the exception tables of any inlined calls when we are
     // inlining a `try_call` site.
     //
@@ -1117,6 +1129,17 @@ impl EntityMap {
         ir::Block::from_u32(offset + callee_block.as_u32())
     }
 
+    fn iter_inlined_blocks(&self, func: &ir::Function) -> impl Iterator<Item = ir::Block> + use<> {
+        let start = self.block_offset.expect(
+            "must create inlined `ir::Block`s before calling `EntityMap::iter_inlined_blocks`",
+        );
+
+        let end = func.dfg.blocks.len();
+        let end = u32::try_from(end).unwrap();
+
+        (start..end).map(|i| ir::Block::from_u32(i))
+    }
+
     fn inlined_global_value(&self, callee_global_value: ir::GlobalValue) -> ir::GlobalValue {
         let offset = self
             .global_value_offset

cranelift/filetests/filetests/inline/unreachable-block.clif

@@ -0,0 +1,46 @@
+test inline precise-output
+target x86_64
+
+function %f0(i32, i32) -> i32 {
+block0(v0: i32, v1: i32):
+    v2 = iadd v0, v1
+    return v2
+
+;; This block is unreachable, despite it being in the function's layout! We
+;; should not include this block in callers' layouts when inlining because we
+;; will skip copying its instructions over into the caller due to how we use a
+;; reachability-based DFS traversal when inlining instructions. Ideally, we
+;; wouldn't even include it in callers' entity maps at all, but that is not
+;; required for correctness, while producing valid CLIF that does not have empty
+;; blocks without terminators is.
+block1:
+    trap user42
+
+}
+
+; (no functions inlined into %f0)
+
+function %f1() -> i32 {
+    fn0 = %f0(i32, i32) -> i32
+block0():
+    v0 = iconst.i32 10
+    v1 = call fn0(v0, v0)
+    return v1
+}
+
+; function %f1() -> i32 fast {
+;     sig0 = (i32, i32) -> i32 fast
+;     fn0 = %f0 sig0
+;
+; block0:
+;     v0 = iconst.i32 10
+;     jump block1
+;
+; block1:
+;     v3 = iadd.i32 v0, v0  ; v0 = 10, v0 = 10
+;     jump block3(v3)
+;
+; block3(v2: i32):
+;     v1 -> v2
+;     return v1
+; }