From a6d62aac3bf31797970a8bfbc563d19b5221fe9b Mon Sep 17 00:00:00 2001 From: Taylor Hogge Date: Mon, 20 Jan 2025 18:04:38 -0700 Subject: [PATCH 1/4] wasmtime_test: Rename "Cranelift" strategy to "CraneliftNative" --- crates/test-macros/src/lib.rs | 4 ++-- tests/all/fuel.rs | 2 +- tests/all/winch_engine_features.rs | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/test-macros/src/lib.rs b/crates/test-macros/src/lib.rs index 43b21855c8b1..90037dc77f4d 100644 --- a/crates/test-macros/src/lib.rs +++ b/crates/test-macros/src/lib.rs @@ -54,7 +54,7 @@ impl TestConfig { if meta.path.is_ident("Winch") { self.strategies.retain(|s| *s != Compiler::Winch); Ok(()) - } else if meta.path.is_ident("Cranelift") { + } else if meta.path.is_ident("CraneliftNative") { self.strategies.retain(|s| *s != Compiler::CraneliftNative); Ok(()) } else { @@ -193,7 +193,7 @@ fn expand(test_config: &TestConfig, func: Fn) -> Result { let mut tests = if test_config.strategies == [Compiler::Winch] { vec![quote! { // This prevents dead code warning when the macro is invoked as: - // #[wasmtime_test(strategies(not(Cranelift))] + // #[wasmtime_test(strategies(not(CraneliftNative))] // Given that Winch only fully supports x86_64. #[allow(dead_code)] #func diff --git a/tests/all/fuel.rs b/tests/all/fuel.rs index 3cb200f8e39b..15f8b218d6f8 100644 --- a/tests/all/fuel.rs +++ b/tests/all/fuel.rs @@ -259,7 +259,7 @@ fn get_fuel_clamps_at_zero(config: &mut Config) -> Result<()> { Ok(()) } -#[wasmtime_test(strategies(not(Cranelift)))] +#[wasmtime_test(strategies(not(CraneliftNative)))] #[cfg_attr(miri, ignore)] fn ensure_stack_alignment(config: &mut Config) -> Result<()> { config.consume_fuel(true); diff --git a/tests/all/winch_engine_features.rs b/tests/all/winch_engine_features.rs index 99ba51702f4e..2f4efa347e4e 100644 --- a/tests/all/winch_engine_features.rs +++ b/tests/all/winch_engine_features.rs @@ -1,7 +1,7 @@ use wasmtime::*; use wasmtime_test_macros::wasmtime_test; -#[wasmtime_test(strategies(not(Cranelift)))] +#[wasmtime_test(strategies(not(CraneliftNative)))] #[cfg_attr(miri, ignore)] fn ensure_compatibility_between_winch_and_table_lazy_init(config: &mut Config) -> Result<()> { config.table_lazy_init(false); @@ -21,7 +21,7 @@ fn ensure_compatibility_between_winch_and_table_lazy_init(config: &mut Config) - Ok(()) } -#[wasmtime_test(strategies(not(Cranelift)))] +#[wasmtime_test(strategies(not(CraneliftNative)))] #[cfg_attr(miri, ignore)] fn ensure_compatibility_between_winch_and_signals_based_traps(config: &mut Config) -> Result<()> { config.signals_based_traps(false); @@ -43,7 +43,7 @@ fn ensure_compatibility_between_winch_and_signals_based_traps(config: &mut Confi Ok(()) } -#[wasmtime_test(strategies(not(Cranelift)))] +#[wasmtime_test(strategies(not(CraneliftNative)))] #[cfg_attr(miri, ignore)] fn ensure_compatibility_between_winch_and_generate_native_debuginfo( config: &mut Config, From 876486cbedcb13e296b8a83ea5fbaf4e87e25bb0 Mon Sep 17 00:00:00 2001 From: Taylor Hogge Date: Mon, 20 Jan 2025 18:13:49 -0700 Subject: [PATCH 2/4] wasmtime_test: Add CraneliftPulley to default test strategies --- crates/test-macros/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/test-macros/src/lib.rs b/crates/test-macros/src/lib.rs index 90037dc77f4d..be229674642a 100644 --- a/crates/test-macros/src/lib.rs +++ b/crates/test-macros/src/lib.rs @@ -57,6 +57,9 @@ impl TestConfig { } else if meta.path.is_ident("CraneliftNative") { self.strategies.retain(|s| *s != Compiler::CraneliftNative); Ok(()) + } else if meta.path.is_ident("CraneliftPulley") { + self.strategies.retain(|s| *s != Compiler::CraneliftPulley); + Ok(()) } else { Err(meta.error("Unknown strategy")) } @@ -97,7 +100,11 @@ impl TestConfig { impl Default for TestConfig { fn default() -> Self { Self { - strategies: vec![Compiler::CraneliftNative, Compiler::Winch], + strategies: vec![ + Compiler::CraneliftNative, + Compiler::Winch, + Compiler::CraneliftPulley, + ], flags: Default::default(), test_attribute: None, } From a97bc706af6732a0e274c5eb1257d699b7a315e9 Mon Sep 17 00:00:00 2001 From: Taylor Hogge Date: Mon, 20 Jan 2025 18:29:08 -0700 Subject: [PATCH 3/4] wasmtime_test: Use one specific compilation strategy with `only` specifier. Tests in `tests/all/fuel.rs` and `tests/all/winch_engine_features.rs` were using `#[wasmtime_test(strategies(not(Cranelift)))]` to gate their Winch specific tests. Now that there is a third compilation strategy, those tests were failing against Pulley. I've replaced those with `#[wasmtime_test(strategies(only(Winch)))]` to be more clear that they are targeted specifically at Winch. --- crates/test-macros/src/lib.rs | 26 +++++++++++++++++++++++++- tests/all/fuel.rs | 2 +- tests/all/winch_engine_features.rs | 6 +++--- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/crates/test-macros/src/lib.rs b/crates/test-macros/src/lib.rs index be229674642a..7056ce391bd7 100644 --- a/crates/test-macros/src/lib.rs +++ b/crates/test-macros/src/lib.rs @@ -15,6 +15,15 @@ //! } //! ``` //! +//! To use just one specific compiler strategy: +//! +//! ```rust +//! #[wasmtime_test(strategies(only(Winch)))] +//! fn my_test(config: &mut Config) -> Result<()> { +//! Ok(()) +//! } +//! ``` +//! //! To explicitly indicate that a wasm features is needed //! ``` //! #[wasmtime_test(wasm_features(gc))] @@ -64,6 +73,21 @@ impl TestConfig { Err(meta.error("Unknown strategy")) } }) + } else if meta.path.is_ident("only") { + meta.parse_nested_meta(|meta| { + if meta.path.is_ident("Winch") { + self.strategies.retain(|s| *s == Compiler::Winch); + Ok(()) + } else if meta.path.is_ident("CraneliftNative") { + self.strategies.retain(|s| *s == Compiler::CraneliftNative); + Ok(()) + } else if meta.path.is_ident("CraneliftPulley") { + self.strategies.retain(|s| *s == Compiler::CraneliftPulley); + Ok(()) + } else { + Err(meta.error("Unknown strategy")) + } + }) } else { Err(meta.error("Unknown identifier")) } @@ -200,7 +224,7 @@ fn expand(test_config: &TestConfig, func: Fn) -> Result { let mut tests = if test_config.strategies == [Compiler::Winch] { vec![quote! { // This prevents dead code warning when the macro is invoked as: - // #[wasmtime_test(strategies(not(CraneliftNative))] + // #[wasmtime_test(strategies(only(Winch))] // Given that Winch only fully supports x86_64. #[allow(dead_code)] #func diff --git a/tests/all/fuel.rs b/tests/all/fuel.rs index 15f8b218d6f8..aa84bab09f39 100644 --- a/tests/all/fuel.rs +++ b/tests/all/fuel.rs @@ -259,7 +259,7 @@ fn get_fuel_clamps_at_zero(config: &mut Config) -> Result<()> { Ok(()) } -#[wasmtime_test(strategies(not(CraneliftNative)))] +#[wasmtime_test(strategies(only(Winch)))] #[cfg_attr(miri, ignore)] fn ensure_stack_alignment(config: &mut Config) -> Result<()> { config.consume_fuel(true); diff --git a/tests/all/winch_engine_features.rs b/tests/all/winch_engine_features.rs index 2f4efa347e4e..5f4e975f6c3a 100644 --- a/tests/all/winch_engine_features.rs +++ b/tests/all/winch_engine_features.rs @@ -1,7 +1,7 @@ use wasmtime::*; use wasmtime_test_macros::wasmtime_test; -#[wasmtime_test(strategies(not(CraneliftNative)))] +#[wasmtime_test(strategies(only(Winch)))] #[cfg_attr(miri, ignore)] fn ensure_compatibility_between_winch_and_table_lazy_init(config: &mut Config) -> Result<()> { config.table_lazy_init(false); @@ -21,7 +21,7 @@ fn ensure_compatibility_between_winch_and_table_lazy_init(config: &mut Config) - Ok(()) } -#[wasmtime_test(strategies(not(CraneliftNative)))] +#[wasmtime_test(strategies(only(Winch)))] #[cfg_attr(miri, ignore)] fn ensure_compatibility_between_winch_and_signals_based_traps(config: &mut Config) -> Result<()> { config.signals_based_traps(false); @@ -43,7 +43,7 @@ fn ensure_compatibility_between_winch_and_signals_based_traps(config: &mut Confi Ok(()) } -#[wasmtime_test(strategies(not(CraneliftNative)))] +#[wasmtime_test(strategies(only(Winch)))] #[cfg_attr(miri, ignore)] fn ensure_compatibility_between_winch_and_generate_native_debuginfo( config: &mut Config, From 998e75434b54147e51e6da707ec60c678225f1e1 Mon Sep 17 00:00:00 2001 From: Taylor Hogge Date: Mon, 20 Jan 2025 21:13:51 -0700 Subject: [PATCH 4/4] wasmtime_test: Fix flaky `wrap_and_typed_i31ref` test in tests/all/func.rs The `static HITS` variable was sharing state between tests, causing them to clobber each other when ran together. --- tests/all/func.rs | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/tests/all/func.rs b/tests/all/func.rs index 574fb8964c58..375c7f02a1f8 100644 --- a/tests/all/func.rs +++ b/tests/all/func.rs @@ -2085,18 +2085,25 @@ fn typed_v128_imports(config: &mut Config) -> anyhow::Result<()> { #[cfg_attr(miri, ignore)] fn wrap_and_typed_i31ref(config: &mut Config) -> Result<()> { let engine = Engine::new(&config)?; - let mut store = Store::new(&engine, ()); + let mut store = Store::::new(&engine, AtomicUsize::new(0)); - static HITS: AtomicUsize = AtomicUsize::new(0); let mut linker = Linker::new(&engine); - linker.func_wrap("env", "i31ref", |x: Option| -> Option { - assert_eq!(HITS.fetch_add(1, Ordering::SeqCst), 0); - x - })?; - linker.func_wrap("env", "ref-i31", |x: I31| -> I31 { - assert_eq!(HITS.fetch_add(1, Ordering::SeqCst), 1); - x - })?; + linker.func_wrap( + "env", + "i31ref", + |caller: Caller<'_, AtomicUsize>, x: Option| -> Option { + assert_eq!(caller.data().fetch_add(1, Ordering::SeqCst), 0); + x + }, + )?; + linker.func_wrap( + "env", + "ref-i31", + |caller: Caller<'_, AtomicUsize>, x: I31| -> I31 { + assert_eq!(caller.data().fetch_add(1, Ordering::SeqCst), 1); + x + }, + )?; let module = Module::new( &engine, @@ -2128,7 +2135,7 @@ fn wrap_and_typed_i31ref(config: &mut Config) -> Result<()> { let x = ref_i31.call(&mut store, I31::wrapping_u32(0x1234))?; assert_eq!(x, I31::wrapping_u32(0x1234)); - assert_eq!(HITS.load(Ordering::SeqCst), 2); + assert_eq!(store.data().load(Ordering::SeqCst), 2); Ok(()) }