From 22e13fee1d883acb0d28e45626736a2bb5017f4f Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 14 Jun 2022 09:50:41 -0700 Subject: [PATCH] fuzz: allow generating shared memories (#4266) `wasm-smith` v0.11 has support for generating shared memories when the `threads_enabled` configuration flag is set. This change turns on that flag occasionally. This also upgrades `wasm-smith` to v0.11.1 to always generate shared memory with a known maximum. --- Cargo.lock | 4 +- crates/fuzzing/Cargo.toml | 2 +- crates/fuzzing/src/generators.rs | 56 ++++++++++++++++++++-------- fuzz/README.md | 2 +- fuzz/fuzz_targets/differential_v8.rs | 9 +++-- 5 files changed, 50 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index df29bac98c..ad7a5e3264 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3237,9 +3237,9 @@ dependencies = [ [[package]] name = "wasm-smith" -version = "0.11.0" +version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4f0ed86e26ccdda3c178e557f8f2874e277c8d4f29c88e3c3320c58db12840ed" +checksum = "c85cf25be85aac46356216b4662eb5768347046449a45c938ae1443b788665bb" dependencies = [ "arbitrary", "flagset", diff --git a/crates/fuzzing/Cargo.toml b/crates/fuzzing/Cargo.toml index 7f4d758e0f..817fbaacff 100644 --- a/crates/fuzzing/Cargo.toml +++ b/crates/fuzzing/Cargo.toml @@ -20,7 +20,7 @@ wasmprinter = "0.2.36" wasmtime = { path = "../wasmtime" } wasmtime-wast = { path = "../wast" } wasm-encoder = "0.13.0" -wasm-smith = "0.11.0" +wasm-smith = "0.11.1" wasm-mutate = "0.2.4" wasm-spec-interpreter = { path = "./wasm-spec-interpreter", optional = true } wasmi = "0.7.0" diff --git a/crates/fuzzing/src/generators.rs b/crates/fuzzing/src/generators.rs index ee0e0f5091..19c82b2654 100644 --- a/crates/fuzzing/src/generators.rs +++ b/crates/fuzzing/src/generators.rs @@ -163,6 +163,11 @@ impl<'a> Arbitrary<'a> for Config { .. } = &config.wasmtime.strategy { + // If the pooling allocator is used, do not allow shared memory to + // be created. FIXME: see + // https://github.com/bytecodealliance/wasmtime/issues/4244. + config.module_config.config.threads_enabled = false; + // Force the use of a normal memory config when using the pooling allocator and // limit the static memory maximum to be the same as the pooling allocator's memory // page limit. @@ -296,6 +301,7 @@ impl Config { config.reference_types_enabled = false; config.simd_enabled = false; config.memory64_enabled = false; + config.threads_enabled = false; // If using the pooling allocator, update the instance limits too if let InstanceAllocationStrategy::Pooling { @@ -343,11 +349,11 @@ impl Config { pub fn set_spectest_compliant(&mut self) { let config = &mut self.module_config.config; config.memory64_enabled = false; - config.simd_enabled = false; config.bulk_memory_enabled = true; config.reference_types_enabled = true; config.multi_value_enabled = true; config.simd_enabled = true; + config.threads_enabled = false; config.max_memories = 1; config.max_tables = 5; @@ -388,6 +394,7 @@ impl Config { .wasm_multi_memory(self.module_config.config.max_memories > 1) .wasm_simd(self.module_config.config.simd_enabled) .wasm_memory64(self.module_config.config.memory64_enabled) + .wasm_threads(self.module_config.config.threads_enabled) .wasm_backtrace(self.wasmtime.wasm_backtraces) .cranelift_nan_canonicalization(self.wasmtime.canonicalize_nans) .cranelift_opt_level(self.wasmtime.opt_level.to_wasmtime()) @@ -434,21 +441,33 @@ impl Config { } } - match &self.wasmtime.memory_config { - MemoryConfig::Normal(memory_config) => { - cfg.static_memory_maximum_size( - memory_config.static_memory_maximum_size.unwrap_or(0), - ) - .static_memory_guard_size(memory_config.static_memory_guard_size.unwrap_or(0)) - .dynamic_memory_guard_size(memory_config.dynamic_memory_guard_size.unwrap_or(0)) - .guard_before_linear_memory(memory_config.guard_before_linear_memory); - } - MemoryConfig::CustomUnaligned => { - cfg.with_host_memory(Arc::new(UnalignedMemoryCreator)) - .static_memory_maximum_size(0) - .dynamic_memory_guard_size(0) - .static_memory_guard_size(0) - .guard_before_linear_memory(false); + // Vary the memory configuration, but only if threads are not enabled. + // When the threads proposal is enabled we might generate shared memory, + // which is less amenable to different memory configurations: + // - shared memories are required to be "static" so fuzzing the various + // memory configurations will mostly result in uninteresting errors. + // The interesting part about shared memories is the runtime so we + // don't fuzz non-default settings. + // - shared memories are required to be aligned which means that the + // `CustomUnaligned` variant isn't actually safe to use with a shared + // memory. + if !self.module_config.config.threads_enabled { + match &self.wasmtime.memory_config { + MemoryConfig::Normal(memory_config) => { + cfg.static_memory_maximum_size( + memory_config.static_memory_maximum_size.unwrap_or(0), + ) + .static_memory_guard_size(memory_config.static_memory_guard_size.unwrap_or(0)) + .dynamic_memory_guard_size(memory_config.dynamic_memory_guard_size.unwrap_or(0)) + .guard_before_linear_memory(memory_config.guard_before_linear_memory); + } + MemoryConfig::CustomUnaligned => { + cfg.with_host_memory(Arc::new(UnalignedMemoryCreator)) + .static_memory_maximum_size(0) + .dynamic_memory_guard_size(0) + .static_memory_guard_size(0) + .guard_before_linear_memory(false); + } } } @@ -634,6 +653,11 @@ impl<'a> Arbitrary<'a> for ModuleConfig { // `SwarmConfig::arbitrary`. config.memory64_enabled = u.arbitrary()?; + // Allow the threads proposal if memory64 is not already enabled. FIXME: + // to allow threads and memory64 to coexist, see + // https://github.com/bytecodealliance/wasmtime/issues/4267. + config.threads_enabled = !config.memory64_enabled && u.arbitrary()?; + Ok(ModuleConfig { config }) } } diff --git a/fuzz/README.md b/fuzz/README.md index d9fec0dfa1..b2e51170c3 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -43,7 +43,7 @@ At the time of writing, we have the following fuzz targets: the same results as the `wasmi` interpreter. * `instantiate`: Generate a Wasm module and Wasmtime configuration and attempt to compile and instantiate with them. -* `instantiate`: Generate many Wasm modules and attempt to compile and +* `instantiate-many`: Generate many Wasm modules and attempt to compile and instantiate them concurrently. * `spectests`: Pick a random spec test and run it with a generated configuration. diff --git a/fuzz/fuzz_targets/differential_v8.rs b/fuzz/fuzz_targets/differential_v8.rs index 440ac9956f..882ae6628e 100644 --- a/fuzz/fuzz_targets/differential_v8.rs +++ b/fuzz/fuzz_targets/differential_v8.rs @@ -17,9 +17,12 @@ fn run(data: &[u8]) -> Result<()> { config.set_differential_config(); // Enable features that v8 has implemented - config.module_config.config.simd_enabled = true; - config.module_config.config.bulk_memory_enabled = true; - config.module_config.config.reference_types_enabled = true; + config.module_config.config.simd_enabled = u.arbitrary()?; + config.module_config.config.bulk_memory_enabled = u.arbitrary()?; + config.module_config.config.reference_types_enabled = u.arbitrary()?; + // FIXME: to enable fuzzing with the threads proposal, see + // https://github.com/bytecodealliance/wasmtime/issues/4268. + // config.module_config.config.threads_enabled = u.arbitrary()?; // Allow multiple tables, as set_differential_config() assumes reference // types are disabled and therefore sets max_tables to 1