diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ed4162267e..1fafe64428 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -357,8 +357,6 @@ jobs: - run: cargo fuzz build --dev -s none # Check that the ISLE fuzz targets build too. - run: cargo fuzz build --dev -s none --fuzz-dir ./cranelift/isle/fuzz - # Check that winch fuzzing builds too. - - run: cargo fuzz build --dev -s none --features fuzz-winch # common logic to cancel the entire run if this job fails - run: gh run cancel ${{ github.run_id }} diff --git a/crates/fuzzing/Cargo.toml b/crates/fuzzing/Cargo.toml index 5ead20aca9..e712834e07 100644 --- a/crates/fuzzing/Cargo.toml +++ b/crates/fuzzing/Cargo.toml @@ -19,7 +19,7 @@ target-lexicon = { workspace = true } tempfile = "3.3.0" wasmparser = { workspace = true } wasmprinter = { workspace = true } -wasmtime = { workspace = true, features = ['default'] } +wasmtime = { workspace = true, features = ['default', 'winch'] } wasmtime-wast = { workspace = true } wasm-encoder = { workspace = true } wasm-smith = { workspace = true } @@ -46,4 +46,3 @@ wasm-spec-interpreter = { path = "./wasm-spec-interpreter", optional = true, fea [features] fuzz-spec-interpreter = ['wasm-spec-interpreter'] -winch = ["wasmtime/winch"] diff --git a/crates/fuzzing/src/generators.rs b/crates/fuzzing/src/generators.rs index 8bc11b1a64..d7703631d8 100644 --- a/crates/fuzzing/src/generators.rs +++ b/crates/fuzzing/src/generators.rs @@ -23,7 +23,6 @@ pub mod table_ops; mod value; pub use codegen_settings::CodegenSettings; -#[cfg(feature = "winch")] pub use config::CompilerStrategy; pub use config::{Config, WasmtimeConfig}; pub use instance_allocation_strategy::InstanceAllocationStrategy; diff --git a/crates/fuzzing/src/generators/config.rs b/crates/fuzzing/src/generators/config.rs index 048bb0d82a..5ea4f6ec7e 100644 --- a/crates/fuzzing/src/generators/config.rs +++ b/crates/fuzzing/src/generators/config.rs @@ -168,37 +168,42 @@ impl Config { .allocation_strategy(self.wasmtime.strategy.to_wasmtime()) .generate_address_map(self.wasmtime.generate_address_map); - #[cfg(feature = "winch")] + let compiler_strategy = &self.wasmtime.compiler_strategy; + let cranelift_strategy = *compiler_strategy == CompilerStrategy::Cranelift; cfg.strategy(self.wasmtime.compiler_strategy.to_wasmtime()); self.wasmtime.codegen.configure(&mut cfg); - // If the wasm-smith-generated module use nan canonicalization then we - // don't need to enable it, but if it doesn't enable it already then we - // enable this codegen option. - cfg.cranelift_nan_canonicalization(!self.module_config.config.canonicalize_nans); - - // Enabling the verifier will at-least-double compilation time, which - // with a 20-30x slowdown in fuzzing can cause issues related to - // timeouts. If generated modules can have more than a small handful of - // functions then disable the verifier when fuzzing to try to lessen the - // impact of timeouts. - if self.module_config.config.max_funcs > 10 { - cfg.cranelift_debug_verifier(false); - } + // Only set cranelift specific flags when the Cranelift strategy is + // chosen. + if cranelift_strategy { + // If the wasm-smith-generated module use nan canonicalization then we + // don't need to enable it, but if it doesn't enable it already then we + // enable this codegen option. + cfg.cranelift_nan_canonicalization(!self.module_config.config.canonicalize_nans); + + // Enabling the verifier will at-least-double compilation time, which + // with a 20-30x slowdown in fuzzing can cause issues related to + // timeouts. If generated modules can have more than a small handful of + // functions then disable the verifier when fuzzing to try to lessen the + // impact of timeouts. + if self.module_config.config.max_funcs > 10 { + cfg.cranelift_debug_verifier(false); + } - if self.wasmtime.force_jump_veneers { - unsafe { - cfg.cranelift_flag_set("wasmtime_linkopt_force_jump_veneer", "true"); + if self.wasmtime.force_jump_veneers { + unsafe { + cfg.cranelift_flag_set("wasmtime_linkopt_force_jump_veneer", "true"); + } } - } - if let Some(pad) = self.wasmtime.padding_between_functions { - unsafe { - cfg.cranelift_flag_set( - "wasmtime_linkopt_padding_between_functions", - &pad.to_string(), - ); + if let Some(pad) = self.wasmtime.padding_between_functions { + unsafe { + cfg.cranelift_flag_set( + "wasmtime_linkopt_padding_between_functions", + &pad.to_string(), + ); + } } } @@ -406,7 +411,6 @@ pub struct WasmtimeConfig { padding_between_functions: Option, generate_address_map: bool, native_unwind_info: bool, - #[cfg(feature = "winch")] /// Configuration for the compiler to use. pub compiler_strategy: CompilerStrategy, bb_padding_log2: u8, @@ -454,8 +458,7 @@ impl OptLevel { } } -#[cfg(feature = "winch")] -#[derive(Arbitrary, Clone, Debug, PartialEq, Eq, Hash)] +#[derive(Clone, Debug, PartialEq, Eq, Hash)] /// Compiler to use. pub enum CompilerStrategy { /// Cranelift compiler. @@ -464,7 +467,6 @@ pub enum CompilerStrategy { Winch, } -#[cfg(feature = "winch")] impl CompilerStrategy { fn to_wasmtime(&self) -> wasmtime::Strategy { match self { @@ -473,3 +475,13 @@ impl CompilerStrategy { } } } + +// Unconditionally return `Cranelift` given that Winch is not ready to be +// enabled by default in all the fuzzing targets. Each fuzzing target is +// expected to explicitly override the strategy as needed. Currently only the +// differential target overrides the compiler strategy. +impl Arbitrary<'_> for CompilerStrategy { + fn arbitrary(_: &mut Unstructured<'_>) -> arbitrary::Result { + Ok(Self::Cranelift) + } +} diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 46e47c58b6..ef6f2ded09 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -21,8 +21,8 @@ cranelift-control = { workspace = true } libfuzzer-sys = { workspace = true, features = ["arbitrary-derive"] } target-lexicon = { workspace = true } smallvec = { workspace = true } -wasmparser = { workspace = true, optional = true } -wasmtime = { workspace = true } +wasmparser = { workspace = true } +wasmtime = { workspace = true, features = ["winch"] } wasmtime-fuzzing = { workspace = true } component-test-util = { workspace = true } component-fuzz-util = { workspace = true } @@ -36,7 +36,6 @@ quote = "1.0" component-fuzz-util = { workspace = true } [features] -fuzz-winch = ["wasmtime/winch", "wasmtime-fuzzing/winch", "dep:wasmparser"] default = ['fuzz-spec-interpreter'] fuzz-spec-interpreter = ['wasmtime-fuzzing/fuzz-spec-interpreter'] chaos = ["cranelift-control/chaos"] diff --git a/fuzz/fuzz_targets/differential.rs b/fuzz/fuzz_targets/differential.rs index 799d8ac409..c20942b79b 100644 --- a/fuzz/fuzz_targets/differential.rs +++ b/fuzz/fuzz_targets/differential.rs @@ -5,14 +5,11 @@ use libfuzzer_sys::fuzz_target; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering::SeqCst; use std::sync::Once; -#[cfg(feature = "fuzz-winch")] use wasmtime_fuzzing::generators::CompilerStrategy; use wasmtime_fuzzing::generators::{Config, DiffValue, DiffValueType, SingleInstModule}; use wasmtime_fuzzing::oracles::diff_wasmtime::WasmtimeInstance; use wasmtime_fuzzing::oracles::engine::{build_allowed_env_list, parse_env_list}; use wasmtime_fuzzing::oracles::{differential, engine, log_wasm, DiffEqResult}; -#[cfg(feature = "fuzz-winch")] -use wasmtime_fuzzing::wasm_smith::{InstructionKind, InstructionKinds}; // Upper limit on the number of invocations for each WebAssembly function // executed by this fuzz target. @@ -26,8 +23,10 @@ static SETUP: Once = Once::new(); // - ALLOWED_ENGINES=wasmi,spec cargo +nightly fuzz run ... // - ALLOWED_ENGINES=-v8 cargo +nightly fuzz run ... // - ALLOWED_MODULES=single-inst cargo +nightly fuzz run ... +// - FUZZ_WINCH=1 cargo +nightly fuzz run ... static mut ALLOWED_ENGINES: Vec<&str> = vec![]; static mut ALLOWED_MODULES: Vec<&str> = vec![]; +static mut FUZZ_WINCH: bool = false; // Statistics about what's actually getting executed during fuzzing static STATS: RuntimeStats = RuntimeStats::new(); @@ -38,25 +37,26 @@ fuzz_target!(|data: &[u8]| { // `setup_ocaml_runtime`. engine::setup_engine_runtimes(); - let (default_engines, default_modules) = if cfg!(feature = "fuzz-winch") { - (vec!["wasmi"], vec!["wasm-smith", "single-inst"]) - } else { - ( - vec!["wasmtime", "wasmi", "spec", "v8"], - vec!["wasm-smith", "single-inst"], - ) - }; - // Retrieve the configuration for this fuzz target from `ALLOWED_*` // environment variables. - let allowed_engines = - build_allowed_env_list(parse_env_list("ALLOWED_ENGINES"), &default_engines); - let allowed_modules = - build_allowed_env_list(parse_env_list("ALLOWED_MODULES"), &default_modules); + let allowed_engines = build_allowed_env_list( + parse_env_list("ALLOWED_ENGINES"), + &["wasmtime", "wasmi", "spec", "v8"], + ); + let allowed_modules = build_allowed_env_list( + parse_env_list("ALLOWED_MODULES"), + &["wasm-smith", "single-inst"], + ); + + let fuzz_winch = match std::env::var("FUZZ_WINCH").map(|v| v == "1") { + Ok(v) => v, + _ => false, + }; unsafe { ALLOWED_ENGINES = allowed_engines; ALLOWED_MODULES = allowed_modules; + FUZZ_WINCH = fuzz_winch; } }); @@ -69,6 +69,7 @@ fn execute_one(data: &[u8]) -> Result<()> { STATS.bump_attempts(); let mut u = Unstructured::new(data); + let fuzz_winch = unsafe { FUZZ_WINCH }; // Generate a Wasmtime and module configuration and update its settings // initially to be suitable for differential execution where the generated @@ -77,14 +78,11 @@ fn execute_one(data: &[u8]) -> Result<()> { let mut config: Config = u.arbitrary()?; config.set_differential_config(); - #[cfg(feature = "fuzz-winch")] - { - // When fuzzing Winch: - // 1. Explicitly override the compiler strategy. - // 2. Explicitly set the allowed instructions for `wasm-smith`. + // When fuzzing Winch, explicitly override the compiler strategy, which by + // default its arbitrary implementation unconditionally returns + // `Cranelift`. + if fuzz_winch { config.wasmtime.compiler_strategy = CompilerStrategy::Winch; - config.module_config.config.allowed_instructions = - InstructionKinds::new(&[InstructionKind::Numeric, InstructionKind::Variable]); } // Choose an engine that Wasmtime will be differentially executed against. @@ -120,8 +118,7 @@ fn execute_one(data: &[u8]) -> Result<()> { _ => unreachable!(), }; - #[cfg(feature = "fuzz-winch")] - if !winch_supports_module(&wasm) { + if fuzz_winch && !winch_supports_module(&wasm) { return Ok(()); } @@ -277,7 +274,6 @@ impl RuntimeStats { } } -#[cfg(feature = "fuzz-winch")] // Returns true if the module only contains operators supported by // Winch. Winch's x86_64 target has broader support for Wasm operators // than the aarch64 target. This list assumes fuzzing on the x86_64