Disable closures as host functions for now + docs + tests

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

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::<F>() != 0 {
+            Self::closures_unsupported_panic();
+        }
         let function = inner::Function::<Args, Rets>::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::<F>() != 0 {
+            Self::closures_unsupported_panic();
+        }
         let function = inner::Function::<Args, Rets>::new(func);
         let address = function.address();
 
@@ -275,6 +287,9 @@ impl Function {
         Rets: WasmTypeList,
         Env: UnsafeMutableEnv + 'static,
     {
+        if std::mem::size_of::<F>() != 0 {
+            Self::closures_unsupported_panic();
+        }
         let function = inner::Function::<Args, Rets>::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 {

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
+///          }),
 ///     },
 /// };
 /// ```

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