Repository: bytecodealliance/wasmtime
Pull request #5765 (2023-02-11 21:11:38)

fuzzgen: Generate multiple functions per testcase
👋  Hey,

This PR is a follow up to #5764, so we probably should merge that first before reviewing this. (Or just review the last 2 commits).

This changes fuzzgen to generate multiple functions in a testcase 🥳

Functions are only allowed to call previously generated functions, so we never generate loops or recursive calls. Though that is probably something we should reconsider at some point.

We always pass all allowed functions into `FunctionGenerator` and so the function headers are somewhat poluted with definitions, maybe this is something we want to improve.

<details>
<summary>Example Testcase:</summary>

```
Output of `std::fmt::Debug`:

;; Fuzzgen test case

test interpret
test run
set opt_level=speed
set regalloc_checker=true
set enable_alias_analysis=false
set enable_simd=true
set enable_llvm_abi_extensions=true
set unwind_info=false
set machine_code_cfg_info=true
set enable_jump_tables=false
set enable_heap_access_spectre_mitigation=false
set enable_table_access_spectre_mitigation=false
target x86_64

function u1:1(i16 sext, i8 sext, i8 sext, i8 sext, i16 uext, i64 uext, i16 uext, i8 sext, i8 sext, i8 sext, i8, i128) -> i64 sext, i8 sext, i8 sext, i8 sext, i8 sext system_v {
    sig0 = (f32) -> f32 system_v
    sig1 = (f64) -> f64 system_v
    sig2 = (f32) -> f32 system_v
    sig3 = (f64) -> f64 system_v
    sig4 = (f32) -> f32 system_v
    sig5 = (f64) -> f64 system_v
    fn0 = colocated %CeilF32 sig0
    fn1 = %CeilF64 sig1
    fn2 = %FloorF32 sig2
    fn3 = %FloorF64 sig3
    fn4 = %TruncF32 sig4
    fn5 = %TruncF64 sig5

block0(v0: i16, v1: i8, v2: i8, v3: i8, v4: i16, v5: i64, v6: i16, v7: i8, v8: i8, v9: i8, v10: i8, v11: i128):
    v12 = iconst.i8 0
    v13 = iconst.i16 0
    v14 = iconst.i32 0
    v15 = iconst.i64 0
    v16 = uextend.i128 v15  ; v15 = 0
    return v5, v1, v9, v1, v3
}

function u1:0(f32, f32, i8 sext, i8 sext) -> i8 sext, i8 sext, i8 sext, i8 sext system_v {
    sig0 = (i16 sext, i8 sext, i8 sext, i8 sext, i16 uext, i64 uext, i16 uext, i8 sext, i8 sext, i8 sext, i8, i128) -> i64 sext, i8 sext, i8 sext, i8 sext, i8 sext system_v
    sig1 = (f32) -> f32 system_v
    sig2 = (f64) -> f64 system_v
    sig3 = (f32) -> f32 system_v
    sig4 = (f64) -> f64 system_v
    sig5 = (f32) -> f32 system_v
    sig6 = (f64) -> f64 system_v
    fn0 = u1:1 sig0
    fn1 = %CeilF32 sig1
    fn2 = %CeilF64 sig2
    fn3 = %FloorF32 sig3
    fn4 = %FloorF64 sig4
    fn5 = %TruncF32 sig5
    fn6 = %TruncF64 sig6

block0(v0: f32, v1: f32, v2: i8, v3: i8):
    v4 = iconst.i64 0x8000_1212_1212
    v5 = iconst.i64 0x1212_1212_1212_1212
    v6 = iconcat v5, v4  ; v5 = 0x1212_1212_1212_1212, v4 = 0x8000_1212_1212
    v7 = iconst.i8 0
    v8 = iconst.i16 0
    v9 = iconst.i32 0
    v10 = iconst.i64 0
    v11 = uextend.i128 v10  ; v10 = 0
    return v2, v2, v2, v2
}


; Note: the results in the below test cases are simply a placeholder and probably will be wrong

; run: u1:0(0x1.420400p17, -NaN:0x3f0000, -1, -1) == [0, 0, 0, 0]
; run: u1:0(0x0.4ffffep-126, 0.0, 0, 0) == [0, 0, 0, 0]
; run: u1:0(0.0, 0.0, 0, 0) == [0, 0, 0, 0]
```
</details>

Ran this on AArch64 and x86 for a while and it didn't complain.

github-actions[bot]: #### Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "fuzzing"

Thus the following users have been cc'd because of the following labels:

* fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

[Learn more.](https://github.com/bytecodealliance/subscribe-to-label-action)
</details>
jameysharp: I'm very excited to merge this, but I'm holding off on reviewing it until we merge #5764.
afonso360: Well, this has been running for the past 8 hours successfully on x64 and AArch64 without crashing. It ran into https://github.com/bytecodealliance/wasmtime/issues/5832 on RISC-V.

I'm going to try to fix that one, otherwise if it ends up being complicated we can disable the colocated flag for RISC-V.
afonso360: Rebased this on top of main, and given the amount of fuzzgen changes since this was first posted I'm going to give this a few hours of fuzzing again before merging.

Patch:
diff --git a/cranelift/codegen/src/ir/extname.rs b/cranelift/codegen/src/ir/extname.rs
index 00552bbd6..f6aac3b97 100644
--- a/cranelift/codegen/src/ir/extname.rs
+++ b/cranelift/codegen/src/ir/extname.rs
@@ -41,6 +41,14 @@ impl UserFuncName {
     pub fn user(namespace: u32, index: u32) -> Self {
         Self::User(UserExternalName::new(namespace, index))
     }
+
+    /// Get a `UserExternalName` if this is a user-defined name.
+    pub fn get_user(&self) -> Option<&UserExternalName> {
+        match self {
+            UserFuncName::User(user) => Some(user),
+            UserFuncName::Testcase(_) => None,
+        }
+    }
 }
 
 impl Default for UserFuncName {
diff --git a/cranelift/fuzzgen/src/config.rs b/cranelift/fuzzgen/src/config.rs
index 2ccdf9ef6..9e7a094a9 100644
--- a/cranelift/fuzzgen/src/config.rs
+++ b/cranelift/fuzzgen/src/config.rs
@@ -8,6 +8,8 @@ pub struct Config {
     /// so we allow the fuzzer to control this by feeding us more or less bytes.
     /// The upper bound here is to prevent too many inputs that cause long test times
     pub max_test_case_inputs: usize,
+    // Number of functions that we generate per testcase
+    pub testcase_funcs: RangeInclusive<usize>,
     pub signature_params: RangeInclusive<usize>,
     pub signature_rets: RangeInclusive<usize>,
     pub instructions_per_block: RangeInclusive<usize>,
@@ -30,9 +32,6 @@ pub struct Config {
     pub switch_cases: RangeInclusive<usize>,
     pub switch_max_range_size: RangeInclusive<usize>,
 
-    /// Number of distinct functions in the same testsuite that we allow calling per function.
-    pub usercalls: RangeInclusive<usize>,
-
     /// Stack slots.
     /// The combination of these two determines stack usage per function
     pub static_stack_slots_per_function: RangeInclusive<usize>,
@@ -70,6 +69,7 @@ impl Default for Config {
     fn default() -> Self {
         Config {
             max_test_case_inputs: 100,
+            testcase_funcs: 1..=8,
             signature_params: 0..=16,
             signature_rets: 0..=16,
             instructions_per_block: 0..=64,
@@ -80,7 +80,6 @@ impl Default for Config {
             switch_cases: 0..=64,
             // Ranges smaller than 2 don't make sense.
             switch_max_range_size: 2..=32,
-            usercalls: 0..=8,
             static_stack_slots_per_function: 0..=8,
             static_stack_slot_size: 0..=128,
             // We need the mix of sizes that allows us to:
diff --git a/cranelift/fuzzgen/src/lib.rs b/cranelift/fuzzgen/src/lib.rs
index d03665004..04d2815ba 100644
--- a/cranelift/fuzzgen/src/lib.rs
+++ b/cranelift/fuzzgen/src/lib.rs
@@ -76,17 +76,32 @@ impl<'a> Arbitrary<'a> for FunctionWithIsa {
         // configurations.
         let target = u.choose(isa::ALL_ARCHITECTURES)?;
         let builder = isa::lookup_by_name(target).map_err(|_| arbitrary::Error::IncorrectFormat)?;
+        let architecture = builder.triple().architecture;
 
         let mut gen = FuzzGen::new(u);
         let flags = gen
-            .generate_flags(builder.triple().architecture)
+            .generate_flags(architecture)
             .map_err(|_| arbitrary::Error::IncorrectFormat)?;
         let isa = builder
             .finish(flags)
             .map_err(|_| arbitrary::Error::IncorrectFormat)?;
 
+        // Function name must be in a different namespace than TESTFILE_NAMESPACE (0)
+        let fname = UserFuncName::user(1, 0);
+
+        // We don't actually generate these functions, we just simulate their signatures and names
+        let func_count = gen.u.int_in_range(gen.config.testcase_funcs.clone())?;
+        let usercalls = (0..func_count)
+            .map(|i| {
+                let name = UserExternalName::new(2, i as u32);
+                let sig = gen.generate_signature(architecture)?;
+                Ok((name, sig))
+            })
+            .collect::<Result<Vec<(UserExternalName, Signature)>>>()
+            .map_err(|_| arbitrary::Error::IncorrectFormat)?;
+
         let func = gen
-            .generate_func(isa.triple().clone())
+            .generate_func(fname, isa.triple().clone(), usercalls)
             .map_err(|_| arbitrary::Error::IncorrectFormat)?;
 
         Ok(FunctionWithIsa { isa, func })
@@ -96,8 +111,9 @@ impl<'a> Arbitrary<'a> for FunctionWithIsa {
 pub struct TestCase {
     /// TargetIsa to use when compiling this test case
     pub isa: isa::OwnedTargetIsa,
-    /// Function under test
-    pub func: Function,
+    /// Functions under test
+    /// By convention the first function is the main function.
+    pub functions: Vec<Function>,
     /// Generate multiple test inputs for each test case.
     /// This allows us to get more coverage per compilation, which may be somewhat expensive.
     pub inputs: Vec<TestCaseInput>,
@@ -111,8 +127,14 @@ impl fmt::Debug for TestCase {
 
         write_non_default_flags(f, self.isa.flags())?;
 
-        writeln!(f, "target {}", self.isa.triple().architecture)?;
-        writeln!(f, "{}", self.func)?;
+        writeln!(f, "target {}\n", self.isa.triple().architecture)?;
+
+        // Print the functions backwards, so that the main function is printed last
+        // and near the test inputs.
+        for func in self.functions.iter().rev() {
+            writeln!(f, "{}\n", func)?;
+        }
+
         writeln!(f, "; Note: the results in the below test cases are simply a placeholder and probably will be wrong\n")?;
 
         for input in self.inputs.iter() {
@@ -120,7 +142,7 @@ impl fmt::Debug for TestCase {
             // here to figure them out? Should work, however we need to be careful to catch
             // panics in case its the interpreter that is failing.
             // For now create a placeholder output consisting of the zero value for the type
-            let returns = &self.func.signature.returns;
+            let returns = &self.main().signature.returns;
             let placeholder_output = returns
                 .iter()
                 .map(|param| DataValue::read_from_slice(&[0; 16][..], param.value_type))
@@ -141,7 +163,7 @@ impl fmt::Debug for TestCase {
                 .collect::<Vec<_>>()
                 .join(", ");
 
-            writeln!(f, "; run: {}({}){}", self.func.name, args, test_condition)?;
+            writeln!(f, "; run: {}({}){}", self.main().name, args, test_condition)?;
         }
 
         Ok(())
@@ -156,6 +178,13 @@ impl<'a> Arbitrary<'a> for TestCase {
     }
 }
 
+impl TestCase {
+    /// Returns the main function of this test case.
+    pub fn main(&self) -> &Function {
+        &self.functions[0]
+    }
+}
+
 pub struct FuzzGen<'r, 'data>
 where
     'data: 'r,
@@ -175,6 +204,12 @@ where
         }
     }
 
+    fn generate_signature(&mut self, architecture: Architecture) -> Result<Signature> {
+        let max_params = self.u.int_in_range(self.config.signature_params.clone())?;
+        let max_rets = self.u.int_in_range(self.config.signature_rets.clone())?;
+        Ok(self.u.signature(architecture, max_params, max_rets)?)
+    }
+
     fn generate_test_inputs(mut self, signature: &Signature) -> Result<Vec<TestCaseInput>> {
         let mut inputs = Vec::new();
 
@@ -253,37 +288,19 @@ where
         Ok(ctx.func)
     }
 
-    fn generate_func(&mut self, target_triple: Triple) -> Result<Function> {
-        let max_params = self.u.int_in_range(self.config.signature_params.clone())?;
-        let max_rets = self.u.int_in_range(self.config.signature_rets.clone())?;
-        let sig = self
-            .u
-            .signature(target_triple.architecture, max_params, max_rets)?;
-
-        // Function name must be in a different namespace than TESTFILE_NAMESPACE (0)
-        let fname = UserFuncName::user(1, 0);
-
-        // Generate the external functions that we allow calling in this function.
-        let usercalls = (0..self.u.int_in_range(self.config.usercalls.clone())?)
-            .map(|i| {
-                let max_params = self.u.int_in_range(self.config.signature_params.clone())?;
-                let max_rets = self.u.int_in_range(self.config.signature_rets.clone())?;
-                let sig = self
-                    .u
-                    .signature(target_triple.architecture, max_params, max_rets)?;
-                let name = UserExternalName {
-                    namespace: 2,
-                    index: i as u32,
-                };
-                Ok((name, sig))
-            })
-            .collect::<Result<Vec<(UserExternalName, Signature)>>>()?;
+    fn generate_func(
+        &mut self,
+        name: UserFuncName,
+        target_triple: Triple,
+        usercalls: Vec<(UserExternalName, Signature)>,
+    ) -> Result<Function> {
+        let sig = self.generate_signature(target_triple.architecture)?;
 
         let func = FunctionGenerator::new(
             &mut self.u,
             &self.config,
             target_triple,
-            fname,
+            name,
             sig,
             usercalls,
             ALLOWED_LIBCALLS.to_vec(),
@@ -375,20 +392,46 @@ where
     }
 
     pub fn generate_host_test(mut self) -> Result<TestCase> {
-        // If we're generating test inputs as well as a function, then we're planning to execute
-        // this function. That means that any function references in it need to exist. We don't yet
-        // have infrastructure for generating multiple functions, so just don't generate user call
-        // function references.
-        self.config.usercalls = 0..=0;
-
         // TestCase is meant to be consumed by a runner, so we make the assumption here that we're
         // generating a TargetIsa for the host.
         let builder =
             builder_with_options(true).expect("Unable to build a TargetIsa for the current host");
         let flags = self.generate_flags(builder.triple().architecture)?;
         let isa = builder.finish(flags)?;
-        let func = self.generate_func(isa.triple().clone())?;
-        let inputs = self.generate_test_inputs(&func.signature)?;
-        Ok(TestCase { isa, func, inputs })
+
+        // When generating functions, we allow each function to call any function that has
+        // already been generated. This guarantees that we never have loops in the call graph.
+        // We generate these backwards, and then reverse them so that the main function is at
+        // the start.
+        let func_count = self.u.int_in_range(self.config.testcase_funcs.clone())?;
+        let mut functions: Vec<Function> = Vec::with_capacity(func_count);
+        for i in (0..func_count).rev() {
+            // Function name must be in a different namespace than TESTFILE_NAMESPACE (0)
+            let fname = UserFuncName::user(1, i as u32);
+
+            let usercalls: Vec<(UserExternalName, Signature)> = functions
+                .iter()
+                .map(|f| {
+                    (
+                        f.name.get_user().unwrap().clone(),
+                        f.stencil.signature.clone(),
+                    )
+                })
+                .collect();
+
+            let func = self.generate_func(fname, isa.triple().clone(), usercalls)?;
+            functions.push(func);
+        }
+        // Now reverse the functions so that the main function is at the start.
+        functions.reverse();
+
+        let main = &functions[0];
+        let inputs = self.generate_test_inputs(&main.signature)?;
+
+        Ok(TestCase {
+            isa,
+            functions,
+            inputs,
+        })
     }
 }
diff --git a/fuzz/fuzz_targets/cranelift-fuzzgen.rs b/fuzz/fuzz_targets/cranelift-fuzzgen.rs
index e2d687496..171a8248e 100644
--- a/fuzz/fuzz_targets/cranelift-fuzzgen.rs
+++ b/fuzz/fuzz_targets/cranelift-fuzzgen.rs
@@ -7,7 +7,7 @@ use std::sync::atomic::AtomicU64;
 use std::sync::atomic::Ordering;
 
 use cranelift_codegen::data_value::DataValue;
-use cranelift_codegen::ir::{Function, LibCall, TrapCode};
+use cranelift_codegen::ir::{LibCall, TrapCode};
 use cranelift_filetests::function_runner::{TestFileCompiler, Trampoline};
 use cranelift_fuzzgen::*;
 use cranelift_interpreter::environment::FuncIndex;
@@ -139,9 +139,11 @@ fn run_in_host(trampoline: &Trampoline, args: &[DataValue]) -> RunResult {
     RunResult::Success(res)
 }
 
-fn build_interpreter(func: &Function) -> Interpreter {
+fn build_interpreter(testcase: &TestCase) -> Interpreter {
     let mut env = FunctionStore::default();
-    env.add(func.name.to_string(), &func);
+    for func in testcase.functions.iter() {
+        env.add(func.name.to_string(), &func);
+    }
 
     let state = InterpreterState::default()
         .with_function_store(env)
@@ -174,21 +176,17 @@ fuzz_target!(|testcase: TestCase| {
         STATISTICS.print(valid_inputs);
     }
 
-    let mut compiler = TestFileCompiler::new(testcase.isa);
-    compiler.declare_function(&testcase.func).unwrap();
-    compiler.define_function(testcase.func.clone()).unwrap();
-    compiler
-        .create_trampoline_for_function(&testcase.func)
-        .unwrap();
+    let mut compiler = TestFileCompiler::new(testcase.isa.clone());
+    compiler.add_functions(&testcase.functions[..]).unwrap();
     let compiled = compiler.compile().unwrap();
-    let trampoline = compiled.get_trampoline(&testcase.func).unwrap();
+    let trampoline = compiled.get_trampoline(testcase.main()).unwrap();
 
     for args in &testcase.inputs {
         STATISTICS.total_runs.fetch_add(1, Ordering::SeqCst);
 
         // We rebuild the interpreter every run so that we don't accidentally carry over any state
         // between runs, such as fuel remaining.
-        let mut interpreter = build_interpreter(&testcase.func);
+        let mut interpreter = build_interpreter(&testcase);
         let int_res = run_in_interpreter(&mut interpreter, args);
         match int_res {
             RunResult::Success(_) => {
diff --git a/supply-chain/audits.toml b/supply-chain/audits.toml
index 30b8ccd14..b9a3dbf77 100644
--- a/supply-chain/audits.toml
+++ b/supply-chain/audits.toml
@@ -771,6 +771,12 @@ This is a small crate which doesn't deviate outside of its intended purpose and
 additionally contains no `unsafe` code.
 """
 
+[[audits.wasm-coredump-builder]]
+who = "Alex Crichton <alex@alexcrichton.com>"
+criteria = "safe-to-deploy"
+delta = "0.1.10 -> 0.1.11"
+notes = "This is a minor update which only adds a small bit of functionality."
+
 [[audits.wasm-coredump-encoder]]
 who = "Alex Crichton <alex@alexcrichton.com>"
 criteria = "safe-to-deploy"
@@ -780,6 +786,15 @@ This small crate contains no `unsafe` code and does no more than what it says on
 the tin.
 """
 
+[[audits.wasm-coredump-encoder]]
+who = "Alex Crichton <alex@alexcrichton.com>"
+criteria = "safe-to-deploy"
+delta = "0.1.10 -> 0.1.11"
+notes = """
+This is a small update which accounts for a function offset in frames and
+doesn't add in too much extra.
+"""
+
 [[audits.wasm-coredump-types]]
 who = "Alex Crichton <alex@alexcrichton.com>"
 criteria = "safe-to-deploy"
@@ -789,6 +804,12 @@ This small crate contains no `unsafe` code and only contains type definitions
 used for wasm core dumps and trivially stays within its bounds.
 """
 
+[[audits.wasm-coredump-types]]
+who = "Alex Crichton <alex@alexcrichton.com>"
+criteria = "safe-to-deploy"
+delta = "0.1.10 -> 0.1.11"
+notes = "This is quite a small update which only adds a small bit of offset-related functionality."
+
 [[audits.wasm-encoder]]
 who = "Alex Crichton <alex@alexcrichton.com>"
 criteria = "safe-to-deploy"


Test patch:
diff --git a/cranelift/filetests/src/function_runner.rs b/cranelift/filetests/src/function_runner.rs
index 8df5814ef..1bc159451 100644
--- a/cranelift/filetests/src/function_runner.rs
+++ b/cranelift/filetests/src/function_runner.rs
@@ -114,16 +114,16 @@ impl TestFileCompiler {
         Self::with_host_isa(flags)
     }
 
-    /// Registers all functions in a [TestFile]. Additionally creates a trampoline for each one
-    /// of them.
-    pub fn add_testfile(&mut self, testfile: &TestFile) -> Result<()> {
+    /// Declares and compiles all functions in `functions`. Additionally creates a trampoline for
+    /// each one of them.
+    pub fn add_functions(&mut self, functions: &[Function]) -> Result<()> {
         // Declare all functions in the file, so that they may refer to each other.
-        for (func, _) in &testfile.functions {
+        for func in functions {
             self.declare_function(func)?;
         }
 
         // Define all functions and trampolines
-        for (func, _) in &testfile.functions {
+        for func in functions {
             self.define_function(func.clone())?;
             self.create_trampoline_for_function(func)?;
         }
@@ -131,6 +131,20 @@ impl TestFileCompiler {
         Ok(())
     }
 
+    /// Registers all functions in a [TestFile]. Additionally creates a trampoline for each one
+    /// of them.
+    pub fn add_testfile(&mut self, testfile: &TestFile) -> Result<()> {
+        let functions = testfile
+            .functions
+            .iter()
+            .map(|(f, _)| f)
+            .cloned()
+            .collect::<Vec<_>>();
+
+        self.add_functions(&functions[..])?;
+        Ok(())
+    }
+
     /// Declares a function an registers it as a linkable and callable target internally
     pub fn declare_function(&mut self, func: &Function) -> Result<()> {
         let next_id = self.defined_functions.len() as u32;