diff --git a/CHANGELOG.md b/CHANGELOG.md index 71c945933bf..2ea497fe669 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ ### Fixed +- [#1841](https://github.com/wasmerio/wasmer/pull/1841) We will now panic when attempting to use a native function with a captured env as a host function. Previously this would silently do the wrong thing. See [#1840](https://github.com/wasmerio/wasmer/pull/1840) for info about Wasmer's support of closures as host functions. - [#1764](https://github.com/wasmerio/wasmer/pull/1764) Fix bug in WASI `path_rename` allowing renamed files to be 1 directory below a preopened directory. ## 1.0.0-alpha5 - 2020-11-06 diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs index b65462e3af3..0142fec4ad5 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -51,6 +51,12 @@ pub enum FunctionDefinition { /// during execution of the function. /// /// Spec: https://webassembly.github.io/spec/core/exec/runtime.html#function-instances +/// +/// # Panics +/// - Closures (functions with captured environments) are not currently supported +/// with native functions. Attempting to create a native `Function` with one will +/// result in a panic. +/// [Closures as host functions tracking issue](https://github.com/wasmerio/wasmer/issues/1840) #[derive(Clone, PartialEq)] pub struct Function { pub(crate) store: Store, @@ -180,6 +186,9 @@ impl Function { Rets: WasmTypeList, Env: Sized + 'static, { + if std::mem::size_of::() != 0 { + Self::closures_unsupported_panic(); + } let function = inner::Function::::new(func); let address = function.address() as *const VMFunctionBody; let vmctx = VMFunctionEnvironment { @@ -229,6 +238,9 @@ impl Function { Rets: WasmTypeList, Env: Sized + 'static, { + if std::mem::size_of::() != 0 { + Self::closures_unsupported_panic(); + } let function = inner::Function::::new(func); let address = function.address(); @@ -275,6 +287,9 @@ impl Function { Rets: WasmTypeList, Env: UnsafeMutableEnv + 'static, { + if std::mem::size_of::() != 0 { + Self::closures_unsupported_panic(); + } let function = inner::Function::::new(func); let address = function.address(); @@ -620,6 +635,10 @@ impl Function { self.definition.clone(), )) } + + fn closures_unsupported_panic() -> ! { + unimplemented!("Closures (functions with captured environments) are currently unsupported with native functions. See: https://github.com/wasmerio/wasmer/issues/1840") + } } impl<'a> Exportable<'a> for Function { diff --git a/lib/deprecated/runtime-core/src/functional_api.rs b/lib/deprecated/runtime-core/src/functional_api.rs index 793991b2168..0b47e023a16 100644 --- a/lib/deprecated/runtime-core/src/functional_api.rs +++ b/lib/deprecated/runtime-core/src/functional_api.rs @@ -78,9 +78,7 @@ pub fn validate(bytes: &[u8]) -> bool { /// /// # Usage /// -/// Function pointers or closures are supported. Closures can capture -/// their environment (with `move`). The first parameter is of kind -/// `vm::Ctx`. +/// Function pointers are supported. The first parameter is of kind `vm::Ctx`. /// /// ``` /// # use wasmer_runtime_core::{imports, func, vm}; @@ -90,15 +88,12 @@ pub fn validate(bytes: &[u8]) -> bool { /// n /// } /// -/// let i = 7; -/// /// let import_object = imports! { /// "env" => { /// "foo" => func!(func), -/// // A closure with a captured environment. -/// "bar" => func!(move |_: &mut vm::Ctx, n: i32| -> i32 { -/// n + i -/// }), +/// "bar" => func!(|_: &mut vm::Ctx, n: i32| -> i32 { +/// n + 7 +/// }), /// }, /// }; /// ``` diff --git a/tests/compilers/native_functions.rs b/tests/compilers/native_functions.rs index cd2600eac1f..91eac643513 100644 --- a/tests/compilers/native_functions.rs +++ b/tests/compilers/native_functions.rs @@ -78,6 +78,90 @@ fn native_function_works_for_wasm() -> Result<()> { Ok(()) } +#[test] +#[should_panic( + expected = "Closures (functions with captured environments) are currently unsupported with native functions. See: https://github.com/wasmerio/wasmer/issues/1840" +)] +fn native_host_function_closure_panics() { + let store = get_store(false); + let state = 3; + Function::new_native(&store, move |_: i32| { + println!("{}", state); + }); +} + +#[test] +#[should_panic( + expected = "Closures (functions with captured environments) are currently unsupported with native functions. See: https://github.com/wasmerio/wasmer/issues/1840" +)] +fn native_with_env_host_function_closure_panics() { + let store = get_store(false); + let state = 3; + let env = 4; + Function::new_native_with_env(&store, env, move |_env: &i32, _: i32| { + println!("{}", state); + }); +} + +#[test] +fn non_native_functions_and_closures_with_no_env_work() -> Result<()> { + let store = get_store(false); + let wat = r#"(module + (func $multiply1 (import "env" "multiply1") (param i32 i32) (result i32)) + (func $multiply2 (import "env" "multiply2") (param i32 i32) (result i32)) + (func $multiply3 (import "env" "multiply3") (param i32 i32) (result i32)) + (func $multiply4 (import "env" "multiply4") (param i32 i32) (result i32)) + + (func (export "test") (param i32 i32 i32 i32 i32) (result i32) + (call $multiply4 + (call $multiply3 + (call $multiply2 + (call $multiply1 + (local.get 0) + (local.get 1)) + (local.get 2)) + (local.get 3)) + (local.get 4))) +)"#; + let module = Module::new(&store, wat).unwrap(); + + let ty = FunctionType::new(vec![Type::I32, Type::I32], vec![Type::I32]); + let env = 10; + let captured_by_closure = 20; + let import_object = imports! { + "env" => { + "multiply1" => Function::new(&store, &ty, move |args| { + if let (Value::I32(v1), Value::I32(v2)) = (&args[0], &args[1]) { + Ok(vec![Value::I32(v1 * v2 * captured_by_closure)]) + } else { + panic!("Invalid arguments"); + } + }), + "multiply2" => Function::new_with_env(&store, &ty, env, move |&env, args| { + if let (Value::I32(v1), Value::I32(v2)) = (&args[0], &args[1]) { + Ok(vec![Value::I32(v1 * v2 * captured_by_closure * env)]) + } else { + panic!("Invalid arguments"); + } + }), + "multiply3" => Function::new_native(&store, |arg1: i32, arg2: i32| -> i32 + {arg1 * arg2 }), + "multiply4" => Function::new_native_with_env(&store, env, |&env: &i32, arg1: i32, arg2: i32| -> i32 + {arg1 * arg2 * env }), + }, + }; + + let instance = Instance::new(&module, &import_object)?; + + let test: NativeFunc<(i32, i32, i32, i32, i32), i32> = + instance.exports.get_native_function("test")?; + + let result = test.call(2, 3, 4, 5, 6)?; + let manually_computed_result = 6 * (5 * (4 * (3 * 2 * 20) * 10 * 20)) * 10; + assert_eq!(result, manually_computed_result); + Ok(()) +} + // The native ABI for functions fails when defining a function natively in // macos (Darwin) with the Apple Silicon ARM chip // TODO: Cranelift should have a good ABI for the ABI