Skip to content

Commit

Permalink
Disable closures as host functions for now + docs + tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Mark McCaskey committed Nov 24, 2020
1 parent 5e2dc65 commit 29d50a5
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

### Fixed

- [#1841](https://github.com/wasmerio/wasmer/pull/1841) We will now panic when attempting to use a 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
Expand Down
24 changes: 24 additions & 0 deletions lib/api/src/externals/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ 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.
/// Attempting to create a `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,
Expand Down Expand Up @@ -79,6 +84,9 @@ impl Function {
where
F: Fn(&[Val]) -> Result<Vec<Val>, RuntimeError> + 'static,
{
if std::mem::size_of::<F>() != 0 {
Self::closures_unsupported_panic();
}
let dynamic_ctx = VMDynamicFunctionContext::from_context(VMDynamicFunctionWithoutEnv {
func: Box::new(func),
function_type: ty.clone(),
Expand Down Expand Up @@ -130,6 +138,9 @@ impl Function {
F: Fn(&Env, &[Val]) -> Result<Vec<Val>, RuntimeError> + 'static,
Env: Sized + 'static,
{
if std::mem::size_of::<F>() != 0 {
Self::closures_unsupported_panic();
}
let dynamic_ctx = VMDynamicFunctionContext::from_context(VMDynamicFunctionWithEnv {
env: Box::new(env),
func: Box::new(func),
Expand Down Expand Up @@ -180,6 +191,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 {
Expand Down Expand Up @@ -229,6 +243,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();

Expand Down Expand Up @@ -275,6 +292,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();

Expand Down Expand Up @@ -620,6 +640,10 @@ impl Function {
self.definition.clone(),
))
}

fn closures_unsupported_panic() -> ! {
unimplemented!("Closures (functions with captured environments) are currently unsupported. See: https://github.com/wasmerio/wasmer/issues/1840")
}
}

impl<'a> Exportable<'a> for Function {
Expand Down
112 changes: 112 additions & 0 deletions tests/compilers/native_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,118 @@ fn native_function_works_for_wasm() -> Result<()> {
Ok(())
}

#[test]
#[should_panic(
expected = "Closures (functions with captured environments) are currently unsupported. See: https://github.com/wasmerio/wasmer/issues/1840"
)]
fn host_function_closure_panics() {
let store = get_store(false);
let state = 3;
let ty = FunctionType::new(vec![], vec![]);
Function::new(&store, &ty, move |_args| {
println!("{}", state);
Ok(vec![])
});
}

#[test]
#[should_panic(
expected = "Closures (functions with captured environments) are currently unsupported. See: https://github.com/wasmerio/wasmer/issues/1840"
)]
fn env_host_function_closure_panics() {
let store = get_store(false);
let state = 3;
let ty = FunctionType::new(vec![], vec![]);
let env = 4;
Function::new_with_env(&store, &ty, env, move |_env, _args| {
println!("{}", state);
Ok(vec![])
});
}

#[test]
#[should_panic(
expected = "Closures (functions with captured environments) are currently unsupported. 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. 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 lambdas_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 import_object = imports! {
"env" => {
"multiply1" => Function::new(&store, &ty, |args| {
if let (Value::I32(v1), Value::I32(v2)) = (&args[0], &args[1]) {
Ok(vec![Value::I32(v1 * v2)])
} else {
panic!("Invalid arguments");
}
}),
"multiply2" => Function::new_with_env(&store, &ty, env, |&env, args| {
if let (Value::I32(v1), Value::I32(v2)) = (&args[0], &args[1]) {
Ok(vec![Value::I32(v1 * v2 * 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) * 10)) * 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
Expand Down

0 comments on commit 29d50a5

Please sign in to comment.