From 2dd6064005d75e29573d77299a715a0dfb704e44 Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Tue, 28 Feb 2023 18:47:09 +0000 Subject: [PATCH] fuzzgen: Generate multiple functions per testcase (#5765) * fuzzgen: Generate multiple functions per testcase * fuzzgen: Fix typo Co-authored-by: Jamey Sharp --------- Co-authored-by: Jamey Sharp --- cranelift/codegen/src/ir/extname.rs | 8 ++ cranelift/filetests/src/function_runner.rs | 24 +++- cranelift/fuzzgen/src/config.rs | 7 +- cranelift/fuzzgen/src/lib.rs | 129 ++++++++++++++------- fuzz/fuzz_targets/cranelift-fuzzgen.rs | 20 ++-- 5 files changed, 125 insertions(+), 63 deletions(-) diff --git a/cranelift/codegen/src/ir/extname.rs b/cranelift/codegen/src/ir/extname.rs index 00552bbd6949..f6aac3b979c1 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/filetests/src/function_runner.rs b/cranelift/filetests/src/function_runner.rs index 8df5814ef5ca..1bc159451887 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::>(); + + 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; diff --git a/cranelift/fuzzgen/src/config.rs b/cranelift/fuzzgen/src/config.rs index 2ccdf9ef63a8..9e7a094a9552 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, pub signature_params: RangeInclusive, pub signature_rets: RangeInclusive, pub instructions_per_block: RangeInclusive, @@ -30,9 +32,6 @@ pub struct Config { pub switch_cases: RangeInclusive, pub switch_max_range_size: RangeInclusive, - /// Number of distinct functions in the same testsuite that we allow calling per function. - pub usercalls: RangeInclusive, - /// Stack slots. /// The combination of these two determines stack usage per function pub static_stack_slots_per_function: RangeInclusive, @@ -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 d036650040c0..04d2815ba841 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::>>() + .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, /// 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, @@ -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::>() .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 { + 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> { let mut inputs = Vec::new(); @@ -253,37 +288,19 @@ where Ok(ctx.func) } - fn generate_func(&mut self, target_triple: Triple) -> Result { - 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::>>()?; + fn generate_func( + &mut self, + name: UserFuncName, + target_triple: Triple, + usercalls: Vec<(UserExternalName, Signature)>, + ) -> Result { + 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 { - // 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 = 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 e2d687496a33..171a8248efa4 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(_) => {