table_ops: allow 0-sized tables, locals, globals (#4495)

I noticed that `TableOp::insert` had assertions that `num_params` and
`table_size` were greater than 0, but no assert for `num_globals`. These
asserts couldn't be hit because the `*_RANGE` constants were all set to
a minimum of 1.

But the only reason I can see to prohibit 0-sized tables, locals, or
globals, was because indexes into those spaces were generated with the
`%` operator. Allowing 0-sized spaces requires not generating the
corresponding instructions at all when there are no valid indexes.

So I pushed the final selection of which table/local/global to access
earlier, to the moment when we're picking which TableOps to run. Then,
instead of generating a random u8 or u32 and taking the remainder to get
it into the right range, I can just ask `arbitrary` to generate a number
in the right range to begin with.

So this now explores some size-0 corners that it didn't before, and it
doesn't require reasoning about whether remainder can divide by zero.
Also I think it uses fewer bits of the `Unstructured` input to produce
the same cases, and I hope that lets libFuzzer more quickly find bits it
can mutate to get to novel coverage paths.

crates/fuzzing/src/generators/table_ops.rs

@@ -11,29 +11,29 @@ use wasm_encoder::{
 /// operations.
 #[derive(Debug)]
 pub struct TableOps {
-    pub(crate) num_params: u8,
+    pub(crate) num_params: u32,
-    pub(crate) num_globals: u8,
+    pub(crate) num_globals: u32,
-    pub(crate) table_size: u32,
+    pub(crate) table_size: i32,
     ops: Vec<TableOp>,
 }
 
-const NUM_PARAMS_RANGE: RangeInclusive<u8> = 1..=10;
+const NUM_PARAMS_RANGE: RangeInclusive<u32> = 0..=10;
-const NUM_GLOBALS_RANGE: RangeInclusive<u8> = 1..=10;
+const NUM_GLOBALS_RANGE: RangeInclusive<u32> = 0..=10;
-const TABLE_SIZE_RANGE: RangeInclusive<u32> = 1..=100;
+const TABLE_SIZE_RANGE: RangeInclusive<i32> = 0..=100;
 const MAX_OPS: usize = 100;
 
 impl TableOps {
     /// Serialize this module into a Wasm binary.
     ///
-    /// The module requires a single import: `(import "" "gc" (func))`. This
+    /// The module requires several function imports. See this function's
-    /// should be a function to trigger GC.
+    /// implementation for their exact types.
     ///
     /// The single export of the module is a function "run" that takes
-    /// `self.num_params()` parameters of type `externref`.
+    /// `self.num_params` parameters of type `externref`.
     ///
-    /// The "run" function is guaranteed to terminate (no loops or recursive
+    /// The "run" function does not terminate; you should run it with limited
-    /// calls), but is not guaranteed to avoid traps (might access out-of-bounds
+    /// fuel. It also is not guaranteed to avoid traps: it may access
-    /// of the table).
+    /// out-of-bounds of the table.
     pub fn to_wasm_binary(&self) -> Vec<u8> {
         let mut module = Module::new();
 
@@ -82,7 +82,7 @@ impl TableOps {
         let mut tables = TableSection::new();
         tables.table(TableType {
             element_type: ValType::ExternRef,
-            minimum: self.table_size,
+            minimum: self.table_size as u32,
             maximum: None,
         });
 
@@ -111,12 +111,7 @@ impl TableOps {
 
         func.instruction(&Instruction::Loop(wasm_encoder::BlockType::Empty));
         for op in &self.ops {
-            op.insert(
+            op.insert(&mut func, self.num_params as u32);
-                &mut func,
-                self.num_params as u32,
-                self.table_size,
-                self.num_globals as u32,
-            );
         }
         func.instruction(&Instruction::Br(0));
         func.instruction(&Instruction::End);
@@ -140,67 +135,64 @@ impl TableOps {
 
 impl<'a> Arbitrary<'a> for TableOps {
     fn arbitrary(u: &mut Unstructured<'a>) -> Result<Self> {
-        let num_params = u.int_in_range(NUM_PARAMS_RANGE)?;
+        let mut result = TableOps {
-        let num_globals = u.int_in_range(NUM_GLOBALS_RANGE)?;
+            num_params: u.int_in_range(NUM_PARAMS_RANGE)?,
-        let table_size = u.int_in_range(TABLE_SIZE_RANGE)?;
+            num_globals: u.int_in_range(NUM_GLOBALS_RANGE)?,
+            table_size: u.int_in_range(TABLE_SIZE_RANGE)?,
+            ops: Vec::new(),
+        };
 
         let mut stack = 0;
-        let mut ops = vec![];
         let mut choices = vec![];
-        while ops.len() < MAX_OPS && !u.is_empty() {
+        while result.ops.len() < MAX_OPS && !u.is_empty() {
-            ops.push(TableOp::arbitrary(u, &mut stack, &mut choices)?);
+            add_table_op(&mut result, u, &mut stack, &mut choices)?;
         }
 
         // Drop any extant refs on the stack.
         for _ in 0..stack {
-            ops.push(TableOp::Drop);
+            result.ops.push(TableOp::Drop);
         }
 
-        Ok(TableOps {
+        Ok(result)
-            num_params,
-            num_globals,
-            table_size,
-            ops,
-        })
     }
 }
 
 macro_rules! define_table_ops {
 	(
         $(
-            $op:ident $( ( $($imm:ty),* $(,)* ) )? : $params:expr => $results:expr ,
+            $op:ident $( ( $($limit:expr => $ty:ty),* ) )? : $params:expr => $results:expr ,
         )*
     ) => {
         #[derive(Copy, Clone, Debug)]
         pub(crate) enum TableOp {
             $(
-                $op $( ( $($imm),* ) )? ,
+                $op $( ( $($ty),* ) )? ,
             )*
         }
 
-        impl TableOp {
+        fn add_table_op(
-            fn arbitrary(
+            ops: &mut TableOps,
             u: &mut Unstructured,
             stack: &mut u32,
-                choices: &mut Vec<fn(&mut Unstructured, &mut u32) -> Result<TableOp>>,
+            choices: &mut Vec<fn(&TableOps, &mut Unstructured, &mut u32) -> Result<TableOp>>,
-            ) -> Result<TableOp> {
+        ) -> Result<()> {
             choices.clear();
 
             // Add all the choices of valid `TableOp`s we could generate.
             $(
                 #[allow(unused_comparisons)]
-                    if *stack >= $params {
+                if $( $(($limit as fn(&TableOps) -> $ty)(&*ops) > 0 &&)* )? *stack >= $params {
-                        choices.push(|_u, stack| {
+                    choices.push(|_ops, _u, stack| {
                         *stack = *stack - $params + $results;
-                            Ok(TableOp::$op $( ( $( <$imm>::arbitrary(_u)? ),* ) )? )
+                        Ok(TableOp::$op $( ( $(_u.int_in_range(0..=($limit as fn(&TableOps) -> $ty)(_ops) - 1)?),* ) )? )
                     });
                 }
             )*
 
             // Choose a table op to insert.
             let f = u.choose(&choices)?;
-                f(u, stack)
+            let v = f(ops, u, stack)?;
-            }
+            Ok(ops.ops.push(v))
         }
 	};
 }
@@ -211,14 +203,15 @@ define_table_ops! {
     MakeRefs : 0 => 3,
     TakeRefs : 3 => 0,
 
-    TableGet(u32) : 0 => 1,
+    // Add one to make sure that out of bounds table accesses are possible, but still rare.
-    TableSet(u32) : 1 => 0,
+    TableGet(|ops| ops.table_size + 1 => i32) : 0 => 1,
+    TableSet(|ops| ops.table_size + 1 => i32) : 1 => 0,
 
-    GlobalGet(u32) : 0 => 1,
+    GlobalGet(|ops| ops.num_globals => u32) : 0 => 1,
-    GlobalSet(u32) : 1 => 0,
+    GlobalSet(|ops| ops.num_globals => u32) : 1 => 0,
 
-    LocalGet(u32) : 0 => 1,
+    LocalGet(|ops| ops.num_params => u32) : 0 => 1,
-    LocalSet(u32) : 1 => 0,
+    LocalSet(|ops| ops.num_params => u32) : 1 => 0,
 
     Drop : 1 => 0,
 
@@ -226,20 +219,11 @@ define_table_ops! {
 }
 
 impl TableOp {
-    fn insert(self, func: &mut Function, num_params: u32, table_size: u32, num_globals: u32) {
+    fn insert(self, func: &mut Function, scratch_local: u32) {
-        assert!(num_params > 0);
-        assert!(table_size > 0);
-
-        // Add one to make sure that out of bounds table accesses are possible,
-        // but still rare.
-        let table_mod = table_size + 1;
-
         let gc_func_idx = 0;
         let take_refs_func_idx = 1;
         let make_refs_func_idx = 2;
 
-        let scratch_local = num_params;
-
         match self {
             Self::Gc => {
                 func.instruction(&Instruction::Call(gc_func_idx));
@@ -251,26 +235,26 @@ impl TableOp {
                 func.instruction(&Instruction::Call(take_refs_func_idx));
             }
             Self::TableGet(x) => {
-                func.instruction(&Instruction::I32Const((x % table_mod) as i32));
+                func.instruction(&Instruction::I32Const(x));
                 func.instruction(&Instruction::TableGet { table: 0 });
             }
             Self::TableSet(x) => {
                 func.instruction(&Instruction::LocalSet(scratch_local));
-                func.instruction(&Instruction::I32Const((x % table_mod) as i32));
+                func.instruction(&Instruction::I32Const(x));
                 func.instruction(&Instruction::LocalGet(scratch_local));
                 func.instruction(&Instruction::TableSet { table: 0 });
             }
             Self::GlobalGet(x) => {
-                func.instruction(&Instruction::GlobalGet(x % num_globals));
+                func.instruction(&Instruction::GlobalGet(x));
             }
             Self::GlobalSet(x) => {
-                func.instruction(&Instruction::GlobalSet(x % num_globals));
+                func.instruction(&Instruction::GlobalSet(x));
             }
             Self::LocalGet(x) => {
-                func.instruction(&Instruction::LocalGet(x % num_params));
+                func.instruction(&Instruction::LocalGet(x));
             }
             Self::LocalSet(x) => {
-                func.instruction(&Instruction::LocalSet(x % num_params));
+                func.instruction(&Instruction::LocalSet(x));
             }
             Self::Drop => {
                 func.instruction(&Instruction::Drop);