From 8d3750ac0cb5122251b7c4526c5b9082ceeb7994 Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Sat, 24 Sep 2022 07:56:32 +0100 Subject: [PATCH 1/8] fuzzgen: Test compiler flags --- cranelift/fuzzgen/src/lib.rs | 87 ++++++++++++++++++++++---- fuzz/fuzz_targets/cranelift-fuzzgen.rs | 18 ++---- 2 files changed, 79 insertions(+), 26 deletions(-) diff --git a/cranelift/fuzzgen/src/lib.rs b/cranelift/fuzzgen/src/lib.rs index 0a44d09c585d..da944f3c4a90 100644 --- a/cranelift/fuzzgen/src/lib.rs +++ b/cranelift/fuzzgen/src/lib.rs @@ -1,5 +1,6 @@ use crate::config::Config; use crate::function_generator::FunctionGenerator; +use crate::settings::{Flags, OptLevel}; use anyhow::Result; use arbitrary::{Arbitrary, Unstructured}; use cranelift::codegen::data_value::DataValue; @@ -30,6 +31,9 @@ impl<'a> Arbitrary<'a> for SingleFunction { } pub struct TestCase { + /// [Flags] to use when compiling this test case + pub flags: Flags, + /// Function under test pub func: Function, /// Generate multiple test inputs for each test case. /// This allows us to get more coverage per compilation, which may be somewhat expensive. @@ -38,19 +42,23 @@ pub struct TestCase { impl fmt::Debug for TestCase { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - r#";; Fuzzgen test case + writeln!(f, ";; Fuzzgen test case\n")?; + writeln!(f, "test interpret")?; + writeln!(f, "test run")?; -test interpret -test run -set enable_llvm_abi_extensions -target aarch64 -target s390x -target x86_64 + // Print only non default flags + let default_flags = Flags::new(settings::builder()); + for (default, flag) in default_flags.iter().zip(self.flags.iter()) { + assert_eq!(default.name, flag.name); -"# - )?; + if default.value_string() != flag.value_string() { + writeln!(f, "set {}={}", flag.name, flag.value_string())?; + } + } + + writeln!(f, "target aarch64")?; + writeln!(f, "target s390x")?; + writeln!(f, "target x86_64\n")?; writeln!(f, "{}", self.func)?; @@ -217,14 +225,69 @@ where self.run_func_passes(func) } + /// Generate a random set of cranelift flags. + /// Only semantics preserving flags are considered + fn generate_flags(&mut self) -> Result { + let mut builder = settings::builder(); + + let opt_levels = [OptLevel::None, OptLevel::Speed, OptLevel::SpeedAndSize]; + let opt = self.u.choose(&opt_levels)?; + builder.set("opt_level", &format!("{}", opt)[..])?; + + // Boolean flags + // TODO: probestack is semantics preserving, but only works inline and on x64 + // TODO: enable_pinned_reg does not work with our current trampolines. See: #4376 + // TODO: is_pic has issues: + // x86: https://github.com/bytecodealliance/wasmtime/issues/5005 + // aarch64: https://github.com/bytecodealliance/wasmtime/issues/2735 + let bool_settings = [ + "enable_alias_analysis", + "enable_simd", + "enable_float", + "enable_atomics", + "enable_safepoints", + "unwind_info", + "preserve_frame_pointers", + "machine_code_cfg_info", + "enable_jump_tables", + "enable_heap_access_spectre_mitigation", + "enable_table_access_spectre_mitigation", + "enable_incremental_compilation_cache_checks", + "regalloc_checker", + "enable_llvm_abi_extensions", + ]; + for flag_name in bool_settings { + let value = format!("{}", bool::arbitrary(self.u)?); + builder.set(flag_name, value.as_str())?; + } + + // Fixed settings + + // We need llvm ABI extensions for i128 values on x86, so enable it regardless of + // what we picked above. + if cfg!(target_arch = "x86_64") { + builder.enable("enable_llvm_abi_extensions")?; + } + + // This is the default, but we should ensure that it wasn't accidentally turned off anywhere. + builder.enable("enable_verifier")?; + + Ok(Flags::new(builder)) + } + pub fn generate_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 funcrefs. self.config.funcrefs_per_function = 0..=0; + let flags = self.generate_flags()?; let func = self.generate_func()?; let inputs = self.generate_test_inputs(&func.signature)?; - Ok(TestCase { func, inputs }) + Ok(TestCase { + flags, + func, + inputs, + }) } } diff --git a/fuzz/fuzz_targets/cranelift-fuzzgen.rs b/fuzz/fuzz_targets/cranelift-fuzzgen.rs index e063f21ecd64..355bcfa9516e 100644 --- a/fuzz/fuzz_targets/cranelift-fuzzgen.rs +++ b/fuzz/fuzz_targets/cranelift-fuzzgen.rs @@ -8,8 +8,6 @@ use std::sync::atomic::Ordering; use cranelift_codegen::data_value::DataValue; use cranelift_codegen::ir::{LibCall, TrapCode}; -use cranelift_codegen::settings; -use cranelift_codegen::settings::Configurable; use cranelift_filetests::function_runner::{TestFileCompiler, Trampoline}; use cranelift_fuzzgen::*; use cranelift_interpreter::environment::FuncIndex; @@ -167,24 +165,16 @@ fn build_interpreter(testcase: &TestCase) -> Interpreter { static STATISTICS: Lazy = Lazy::new(Statistics::default); fuzz_target!(|testcase: TestCase| { + // This is the default, but we should ensure that it wasn't accidentally turned off anywhere. + assert!(testcase.flags.enable_verifier()); + // Periodically print statistics let valid_inputs = STATISTICS.valid_inputs.fetch_add(1, Ordering::SeqCst); if valid_inputs != 0 && valid_inputs % 10000 == 0 { STATISTICS.print(valid_inputs); } - // Native fn - let flags = { - let mut builder = settings::builder(); - // We need llvm ABI extensions for i128 values on x86 - builder.set("enable_llvm_abi_extensions", "true").unwrap(); - - // This is the default, but we should ensure that it wasn't accidentally turned off anywhere. - builder.set("enable_verifier", "true").unwrap(); - - settings::Flags::new(builder) - }; - let mut compiler = TestFileCompiler::with_host_isa(flags).unwrap(); + let mut compiler = TestFileCompiler::with_host_isa(testcase.flags.clone()).unwrap(); compiler.declare_function(&testcase.func).unwrap(); compiler.define_function(testcase.func.clone()).unwrap(); compiler From f1ad9b06660c6bf8a908217b07d8eb06531e03f4 Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Wed, 5 Oct 2022 12:19:46 +0100 Subject: [PATCH 2/8] cranelift: Generate `all()` function for all enum flags This allows a user to iterate all flags that exist. --- cranelift/codegen/meta/src/gen_settings.rs | 26 ++++++++++++++++++++++ cranelift/fuzzgen/src/lib.rs | 3 +-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/cranelift/codegen/meta/src/gen_settings.rs b/cranelift/codegen/meta/src/gen_settings.rs index 3c1de7cd6496..01a4f731a772 100644 --- a/cranelift/codegen/meta/src/gen_settings.rs +++ b/cranelift/codegen/meta/src/gen_settings.rs @@ -98,6 +98,26 @@ fn gen_iterator(group: &SettingGroup, fmt: &mut Formatter) { fmtln!(fmt, "}"); } +/// Generates a `all()` function with all options for this enum +fn gen_enum_all(name: &str, values: &[&'static str], fmt: &mut Formatter) { + fmtln!( + fmt, + "/// Returns a slice with all possible [{}] values.", + name + ); + fmtln!(fmt, "pub fn all() -> &'static [{}] {{", name); + fmt.indent(|fmt| { + fmtln!(fmt, "&["); + fmt.indent(|fmt| { + for v in values.iter() { + fmtln!(fmt, "Self::{},", camel_case(v)); + } + }); + fmtln!(fmt, "]"); + }); + fmtln!(fmt, "}"); +} + /// Emit Display and FromStr implementations for enum settings. fn gen_to_and_from_str(name: &str, values: &[&'static str], fmt: &mut Formatter) { fmtln!(fmt, "impl fmt::Display for {} {{", name); @@ -158,6 +178,12 @@ fn gen_enum_types(group: &SettingGroup, fmt: &mut Formatter) { }); fmtln!(fmt, "}"); + fmtln!(fmt, "impl {} {{", name); + fmt.indent(|fmt| { + gen_enum_all(&name, values, fmt); + }); + fmtln!(fmt, "}"); + gen_to_and_from_str(&name, values, fmt); } } diff --git a/cranelift/fuzzgen/src/lib.rs b/cranelift/fuzzgen/src/lib.rs index da944f3c4a90..a38b19cc59cd 100644 --- a/cranelift/fuzzgen/src/lib.rs +++ b/cranelift/fuzzgen/src/lib.rs @@ -230,8 +230,7 @@ where fn generate_flags(&mut self) -> Result { let mut builder = settings::builder(); - let opt_levels = [OptLevel::None, OptLevel::Speed, OptLevel::SpeedAndSize]; - let opt = self.u.choose(&opt_levels)?; + let opt = self.u.choose(OptLevel::all())?; builder.set("opt_level", &format!("{}", opt)[..])?; // Boolean flags From 28e844ba96161ac0beef98a9cdfaee12e9fec8fd Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Sat, 8 Oct 2022 22:12:33 +0100 Subject: [PATCH 3/8] fuzzgen: Minimize regalloc_checker compiles --- cranelift/fuzzgen/src/config.rs | 7 +++++++ cranelift/fuzzgen/src/lib.rs | 9 ++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/cranelift/fuzzgen/src/config.rs b/cranelift/fuzzgen/src/config.rs index 6f1103b0510c..daae25d4eb54 100644 --- a/cranelift/fuzzgen/src/config.rs +++ b/cranelift/fuzzgen/src/config.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::ops::RangeInclusive; /// Holds the range of acceptable values to use during the generation of testcases @@ -51,6 +52,11 @@ pub struct Config { /// We insert a checking sequence to guarantee that those inputs never make /// it to the instruction, but sometimes we want to allow them. pub allowed_fcvt_traps_ratio: (usize, usize), + + /// Some flags really impact compile performance, we still want to test + /// them, but probably at a lower rate, so that overall execution time isn't + /// impacted as much + pub compile_flag_ratio: HashMap<&'static str, (usize, usize)>, } impl Default for Config { @@ -75,6 +81,7 @@ impl Default for Config { backwards_branch_ratio: (1, 1000), allowed_int_divz_ratio: (1, 1_000_000), allowed_fcvt_traps_ratio: (1, 1_000_000), + compile_flag_ratio: [("regalloc_checker", (1usize, 1000))].into_iter().collect(), } } } diff --git a/cranelift/fuzzgen/src/lib.rs b/cranelift/fuzzgen/src/lib.rs index a38b19cc59cd..0150e209be93 100644 --- a/cranelift/fuzzgen/src/lib.rs +++ b/cranelift/fuzzgen/src/lib.rs @@ -256,7 +256,14 @@ where "enable_llvm_abi_extensions", ]; for flag_name in bool_settings { - let value = format!("{}", bool::arbitrary(self.u)?); + let enabled = self + .config + .compile_flag_ratio + .get(&flag_name) + .map(|&(num, denum)| self.u.ratio(num, denum)) + .unwrap_or_else(|| bool::arbitrary(self.u))?; + + let value = format!("{}", enabled); builder.set(flag_name, value.as_str())?; } From b3e29f9c9312f4f44429421fd5c7b927904e0016 Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Wed, 5 Oct 2022 20:44:27 +0100 Subject: [PATCH 4/8] fuzzgen: Limit the amount of test case inputs --- cranelift/fuzzgen/src/config.rs | 8 ++++++-- cranelift/fuzzgen/src/lib.rs | 5 ++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/cranelift/fuzzgen/src/config.rs b/cranelift/fuzzgen/src/config.rs index daae25d4eb54..d0247f59045b 100644 --- a/cranelift/fuzzgen/src/config.rs +++ b/cranelift/fuzzgen/src/config.rs @@ -3,7 +3,11 @@ use std::ops::RangeInclusive; /// Holds the range of acceptable values to use during the generation of testcases pub struct Config { - pub test_case_inputs: RangeInclusive, + /// Maximum allowed test case inputs. + /// We build test case inputs from the rest of the bytes that the fuzzer provides us + /// 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, pub signature_params: RangeInclusive, pub signature_rets: RangeInclusive, pub instructions_per_block: RangeInclusive, @@ -62,7 +66,7 @@ pub struct Config { impl Default for Config { fn default() -> Self { Config { - test_case_inputs: 1..=10, + max_test_case_inputs: 100, signature_params: 0..=16, signature_rets: 0..=16, instructions_per_block: 0..=64, diff --git a/cranelift/fuzzgen/src/lib.rs b/cranelift/fuzzgen/src/lib.rs index 0150e209be93..7eba44082b2d 100644 --- a/cranelift/fuzzgen/src/lib.rs +++ b/cranelift/fuzzgen/src/lib.rs @@ -148,7 +148,10 @@ where fn generate_test_inputs(mut self, signature: &Signature) -> Result> { let mut inputs = Vec::new(); - loop { + // Generate up to "max_test_case_inputs" inputs, we need an upper bound here since + // the fuzzer at some point starts trying to feed us way too many inputs. (I found one + // test case with 130k inputs!) + for _ in 0..self.config.max_test_case_inputs { let last_len = self.u.len(); let test_args = signature From 771dca161d0d72844c8342368214726d0b3e7e12 Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Wed, 12 Oct 2022 09:20:51 +0100 Subject: [PATCH 5/8] fuzzgen: Add egraphs flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's finally here! 🥳 --- cranelift/fuzzgen/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/cranelift/fuzzgen/src/lib.rs b/cranelift/fuzzgen/src/lib.rs index 7eba44082b2d..b7a3f3d80da0 100644 --- a/cranelift/fuzzgen/src/lib.rs +++ b/cranelift/fuzzgen/src/lib.rs @@ -257,6 +257,7 @@ where "enable_incremental_compilation_cache_checks", "regalloc_checker", "enable_llvm_abi_extensions", + "use_egraphs", ]; for flag_name in bool_settings { let enabled = self From dcbaff1bae776b72bcd53174cc97616170526a3e Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Thu, 20 Oct 2022 18:13:26 +0100 Subject: [PATCH 6/8] cranelift: Add fuzzing comment to settings --- cranelift/codegen/meta/src/shared/settings.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cranelift/codegen/meta/src/shared/settings.rs b/cranelift/codegen/meta/src/shared/settings.rs index 6b9c241c7d2c..d006d38815c2 100644 --- a/cranelift/codegen/meta/src/shared/settings.rs +++ b/cranelift/codegen/meta/src/shared/settings.rs @@ -364,5 +364,7 @@ pub(crate) fn define() -> SettingGroup { false, ); + // When adding new settings please check if they can also be added + // in cranelift/fuzzgen/src/lib.rs for fuzzing. settings.build() } From 8b222a455ad7b441b09eb5cf6d3ce3189566271f Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Thu, 20 Oct 2022 18:16:37 +0100 Subject: [PATCH 7/8] fuzzgen: Add riscv64 --- cranelift/fuzzgen/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/cranelift/fuzzgen/src/lib.rs b/cranelift/fuzzgen/src/lib.rs index b7a3f3d80da0..7350a0612f48 100644 --- a/cranelift/fuzzgen/src/lib.rs +++ b/cranelift/fuzzgen/src/lib.rs @@ -58,6 +58,7 @@ impl fmt::Debug for TestCase { writeln!(f, "target aarch64")?; writeln!(f, "target s390x")?; + writeln!(f, "target riscv64")?; writeln!(f, "target x86_64\n")?; writeln!(f, "{}", self.func)?; From 711bcff9c07e48c283b7513f02757a3558fe09ba Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Thu, 20 Oct 2022 18:33:19 +0100 Subject: [PATCH 8/8] fuzzgen: Unconditionally enable some flags --- cranelift/fuzzgen/src/lib.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/cranelift/fuzzgen/src/lib.rs b/cranelift/fuzzgen/src/lib.rs index 7350a0612f48..4e9a64c83340 100644 --- a/cranelift/fuzzgen/src/lib.rs +++ b/cranelift/fuzzgen/src/lib.rs @@ -245,13 +245,9 @@ where // aarch64: https://github.com/bytecodealliance/wasmtime/issues/2735 let bool_settings = [ "enable_alias_analysis", - "enable_simd", - "enable_float", - "enable_atomics", "enable_safepoints", "unwind_info", "preserve_frame_pointers", - "machine_code_cfg_info", "enable_jump_tables", "enable_heap_access_spectre_mitigation", "enable_table_access_spectre_mitigation", @@ -283,6 +279,16 @@ where // This is the default, but we should ensure that it wasn't accidentally turned off anywhere. builder.enable("enable_verifier")?; + // These settings just panic when they're not enabled and we try to use their respective functionality + // so they aren't very interesting to be automatically generated. + builder.enable("enable_atomics")?; + builder.enable("enable_float")?; + builder.enable("enable_simd")?; + + // `machine_code_cfg_info` generates additional metadata for the embedder but this doesn't feed back + // into compilation anywhere, we leave it on unconditionally to make sure the generation doesn't panic. + builder.enable("machine_code_cfg_info")?; + Ok(Flags::new(builder)) }