cranelift-frontend: Allow jump table reuse (#4429)

* Allow using jump-tables multiple times (fixes #3347)

If there are multiple `br_table` instructions using the same jump table,
then `append_jump_argument` must not modify the jump table in-place.

When this function is called, we don't know if more `br_table`
instructions might be added later. So this patch conservatively assumes
that all jump tables might be reused. If Cranelift needs to add a block
argument to a block that's the target of some jump table, then the jump
table will be unconditionally cloned.

I'm not sure if having duplicated and unused jump tables will turn out
to be a compile-time performance issue. If it is, there's discussion in
issue #3347 about ways to determine that there can't be any more uses of
a jump table, so that it's safe to modify in-place.

* Re-enable cranelift-fuzzgen fuzz target

I've been running this fuzz target for an hour without finding new bugs.
Let's see if oss-fuzz finds anything now.

cranelift/frontend/src/ssa.rs

@@ -17,7 +17,9 @@ use cranelift_codegen::entity::SecondaryMap;
 use cranelift_codegen::ir::immediates::{Ieee32, Ieee64};
 use cranelift_codegen::ir::instructions::BranchInfo;
 use cranelift_codegen::ir::types::{F32, F64};
-use cranelift_codegen::ir::{Block, Function, Inst, InstBuilder, InstructionData, Type, Value};
+use cranelift_codegen::ir::{
+    Block, Function, Inst, InstBuilder, InstructionData, JumpTableData, Type, Value,
+};
 use cranelift_codegen::packed_option::PackedOption;
 use smallvec::SmallVec;
 
@@ -652,7 +654,7 @@ impl SSABuilder {
                 func.dfg.append_inst_arg(jump_inst, val);
                 None
             }
-            BranchInfo::Table(jt, default_block) => {
+            BranchInfo::Table(mut jt, _default_block) => {
                 // In the case of a jump table, the situation is tricky because br_table doesn't
                 // support arguments.
                 // We have to split the critical edge
@@ -662,25 +664,35 @@ impl SSABuilder {
                 self.ssa_blocks[middle_block].add_predecessor(jump_inst_block, jump_inst);
                 self.mark_block_sealed(middle_block);
 
-                if let Some(default_block) = default_block {
+                let table = &func.jump_tables[jt];
-                    if dest_block == default_block {
+                let mut copied = JumpTableData::with_capacity(table.len());
-                        match func.dfg[jump_inst] {
+                let mut changed = false;
-                            InstructionData::BranchTable {
+                for &destination in table.iter() {
-                                destination: ref mut dest,
+                    if destination == dest_block {
-                                ..
+                        copied.push_entry(middle_block);
-                            } => {
+                        changed = true;
-                                *dest = middle_block;
+                    } else {
-                            }
+                        copied.push_entry(destination);
-                            _ => panic!("should not happen"),
-                        }
                     }
                 }
 
-                for old_dest in func.jump_tables[jt].as_mut_slice() {
+                if changed {
-                    if *old_dest == dest_block {
+                    jt = func.create_jump_table(copied);
-                        *old_dest = middle_block;
-                    }
                 }
+
+                // Redo the match from `analyze_branch` but this time capture mutable references
+                match &mut func.dfg[jump_inst] {
+                    InstructionData::BranchTable {
+                        destination, table, ..
+                    } => {
+                        if *destination == dest_block {
+                            *destination = middle_block;
+                        }
+                        *table = jt;
+                    }
+                    _ => unreachable!(),
+                }
+
                 let mut cur = FuncCursor::new(func).at_bottom(middle_block);
                 let middle_jump_inst = cur.ins().jump(dest_block, &[val]);
                 self.def_var(var, val, middle_block);

fuzz/Cargo.toml

@@ -85,16 +85,11 @@ path = "fuzz_targets/compile-maybe-invalid.rs"
 test = false
 doc = false
 
-# FIXME: the cranelift-fuzzgen fuzz targets are temporarily disabled until
+[[bin]]
-# the crashes they're finding are fixed. One issue is #3347 but otherwise the
+name = "cranelift-fuzzgen"
-# oss-fuzz bots are reporting a 100% crash rate with these fuzzers so there may
+path = "fuzz_targets/cranelift-fuzzgen.rs"
-# be more issues as well. It's recommended to locally run these fuzzers for a
+test = false
-# few hours locally before re-enabling.
+doc = false
-# [[bin]]
-# name = "cranelift-fuzzgen"
-# path = "fuzz_targets/cranelift-fuzzgen.rs"
-# test = false
-# doc = false
 
 [[bin]]
 name = "instantiate-many"