From e9c0a612a8e331fb4e3417894a6ce99e045e2832 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 9 Jul 2024 14:09:19 -0400 Subject: [PATCH] fix: disable multi-memory when config.only_stable_features is true (#269) --- crates/fuzz-utils/Cargo.toml | 4 ++-- crates/tests/tests/spec-tests.rs | 14 +++----------- src/module/config.rs | 31 ++++++++++++++++++++++++++++++- src/module/mod.rs | 9 ++++++--- 4 files changed, 41 insertions(+), 17 deletions(-) diff --git a/crates/fuzz-utils/Cargo.toml b/crates/fuzz-utils/Cargo.toml index e945261f..e88f9815 100644 --- a/crates/fuzz-utils/Cargo.toml +++ b/crates/fuzz-utils/Cargo.toml @@ -7,10 +7,10 @@ publish = false [dependencies] anyhow = "1.0" -env_logger = "0.8.1" +env_logger = "0.11.3" rand = { version = "0.7.0", features = ['small_rng'] } tempfile = "3.1.0" -wasmparser = "0.80.2" +wasmparser = "0.212.0" wat = "1.0" [dependencies.walrus] diff --git a/crates/tests/tests/spec-tests.rs b/crates/tests/tests/spec-tests.rs index 82cdb084..64f8e1f9 100644 --- a/crates/tests/tests/spec-tests.rs +++ b/crates/tests/tests/spec-tests.rs @@ -54,7 +54,9 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { let mut files = Vec::new(); let mut config = walrus::ModuleConfig::new(); - if extra_args.len() == 0 { + if proposal.is_none() { + // For non-proposals tests, we only enable the stable features. + // For proposals tests, we enable all supported features. config.only_stable_features(true); } @@ -71,16 +73,6 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { let path = tempdir.path().join(filename); match command["type"].as_str().unwrap() { "assert_invalid" | "assert_malformed" => { - // The multiple-memory feature is on (from wasmparser::WasmFeatures::default()). - // In imports.wast and memory.wast, following cases will be parsed which should not. - if proposal.is_none() && command["text"] == "multiple memories" { - continue; - } - // In binary.wast, following cases will be parsed which should not. - if proposal.is_none() && command["text"] == "zero byte expected" { - continue; - } - let wasm = fs::read(&path)?; if config.parse(&wasm).is_ok() { should_not_parse.push(line); diff --git a/src/module/config.rs b/src/module/config.rs index 66470a65..45ba8bbf 100644 --- a/src/module/config.rs +++ b/src/module/config.rs @@ -4,6 +4,7 @@ use crate::module::Module; use crate::parse::IndicesToIds; use std::fmt; use std::path::Path; +use wasmparser::WasmFeatures; /// Configuration for a `Module` which currently affects parsing. #[derive(Default)] @@ -153,12 +154,40 @@ impl ModuleConfig { /// the codebase, even if set to `true` some unstable features may still be /// allowed. /// - /// By default this flag is `false` + /// By default this flag is `false`. pub fn only_stable_features(&mut self, only: bool) -> &mut ModuleConfig { self.only_stable_features = only; self } + /// Returns a `wasmparser::WasmFeatures` based on the enabled proposals + /// which should be used for `wasmparser::Parser`` and `wasmparser::Validator`. + pub(crate) fn get_wasmparser_wasm_features(&self) -> WasmFeatures { + // Start from empty so that we explicitly control what is enabled. + let mut features = WasmFeatures::empty(); + // This is not a proposal. + features.insert(WasmFeatures::FLOATS); + // Always enable [finished proposals](https://github.com/WebAssembly/proposals/blob/main/finished-proposals.md). + features.insert(WasmFeatures::MUTABLE_GLOBAL); + features.insert(WasmFeatures::SATURATING_FLOAT_TO_INT); + features.insert(WasmFeatures::SIGN_EXTENSION); + features.insert(WasmFeatures::MULTI_VALUE); + features.insert(WasmFeatures::REFERENCE_TYPES); + features.insert(WasmFeatures::BULK_MEMORY); + features.insert(WasmFeatures::SIMD); + // Enable supported active proposals. + if !self.only_stable_features { + // # Fully supported proposals. + features.insert(WasmFeatures::MULTI_MEMORY); + // # Partially supported proposals. + // ## threads + // spec-tests/proposals/threads still fail + // round_trip tests already require this feature, so we can't disable it by default. + features.insert(WasmFeatures::THREADS); + } + features + } + /// Provide a function that is invoked after successfully parsing a module, /// and gets access to data structures that only exist at parse time, such /// as the map from indices in the original Wasm to the new walrus IDs. diff --git a/src/module/mod.rs b/src/module/mod.rs index 7411d82b..4aaf6bcc 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -45,7 +45,7 @@ use std::fs; use std::mem; use std::ops::Range; use std::path::Path; -use wasmparser::{BinaryReader, Parser, Payload, Validator, WasmFeatures}; +use wasmparser::{BinaryReader, Parser, Payload, Validator}; pub use self::config::ModuleConfig; @@ -137,14 +137,17 @@ impl Module { // For now we have the same set of wasm features // regardless of config.only_stable_features. New unstable features // may be enabled under `only_stable_features: false` in future. - let wasm_features = WasmFeatures::default(); + let wasm_features = config.get_wasmparser_wasm_features(); let mut validator = Validator::new_with_features(wasm_features); let mut local_functions = Vec::new(); let mut debug_sections = Vec::new(); - for payload in Parser::new(0).parse_all(wasm) { + let mut parser = Parser::new(0); + parser.set_features(wasm_features); + + for payload in parser.parse_all(wasm) { match payload? { Payload::Version { num,