Refactor the internals of Func to remove layers of indirection (#1363)

* Remove `WrappedCallable` indirection

At this point `Func` has evolved quite a bit since inception and the
`WrappedCallable` trait I don't believe is needed any longer. This
should help clean up a few entry points by having fewer traits in play.

* Remove the `Callable` trait

This commit removes the `wasmtime::Callable` trait, changing the
signature of `Func::new` to take an appropriately typed `Fn`.
Additionally the function now always takes `&Caller` like `Func::wrap`
optionally can, to empower `Func::new` to have the same capabilities of
`Func::wrap`.

* Add a test for an already-fixed issue

Closes #849

* rustfmt

* Update more locations for `Callable`

* rustfmt

* Remove a stray leading borrow

* Review feedback

* Remove unneeded `wasmtime_call_trampoline` shim
This commit is contained in:
Alex Crichton
2020-03-19 14:21:45 -05:00
committed by GitHub
parent 39ba281bc7
commit afd980b4f6
16 changed files with 354 additions and 682 deletions

View File

@@ -1,5 +1,4 @@
use anyhow::Result;
use std::rc::Rc;
use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
use wasmtime::*;
@@ -238,21 +237,15 @@ fn get_from_wrapper() {
#[test]
fn get_from_signature() {
struct Foo;
impl Callable for Foo {
fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> {
panic!()
}
}
let store = Store::default();
let ty = FuncType::new(Box::new([]), Box::new([]));
let f = Func::new(&store, ty, Rc::new(Foo));
let f = Func::new(&store, ty, |_, _, _| panic!());
assert!(f.get0::<()>().is_ok());
assert!(f.get0::<i32>().is_err());
assert!(f.get1::<i32, ()>().is_err());
let ty = FuncType::new(Box::new([ValType::I32]), Box::new([ValType::F64]));
let f = Func::new(&store, ty, Rc::new(Foo));
let f = Func::new(&store, ty, |_, _, _| panic!());
assert!(f.get0::<()>().is_err());
assert!(f.get0::<i32>().is_err());
assert!(f.get1::<i32, ()>().is_err());
@@ -392,3 +385,16 @@ fn caller_memory() -> anyhow::Result<()> {
Instance::new(&module, &[f.into()])?;
Ok(())
}
#[test]
fn func_write_nothing() -> anyhow::Result<()> {
let store = Store::default();
let ty = FuncType::new(Box::new([]), Box::new([ValType::I32]));
let f = Func::new(&store, ty, |_, _, _| Ok(()));
let err = f.call(&[]).unwrap_err();
assert_eq!(
err.message(),
"function attempted to return an incompatible value"
);
Ok(())
}

View File

@@ -1,4 +1,3 @@
use std::rc::Rc;
use wasmtime::*;
#[test]
@@ -15,28 +14,6 @@ fn same_import_names_still_distinct() -> anyhow::Result<()> {
)
"#;
struct Ret1;
impl Callable for Ret1 {
fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap> {
assert!(params.is_empty());
assert_eq!(results.len(), 1);
results[0] = 1i32.into();
Ok(())
}
}
struct Ret2;
impl Callable for Ret2 {
fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap> {
assert!(params.is_empty());
assert_eq!(results.len(), 1);
results[0] = 2.0f32.into();
Ok(())
}
}
let store = Store::default();
let module = Module::new(&store, WAT)?;
@@ -44,13 +21,23 @@ fn same_import_names_still_distinct() -> anyhow::Result<()> {
Func::new(
&store,
FuncType::new(Box::new([]), Box::new([ValType::I32])),
Rc::new(Ret1),
|_, params, results| {
assert!(params.is_empty());
assert_eq!(results.len(), 1);
results[0] = 1i32.into();
Ok(())
},
)
.into(),
Func::new(
&store,
FuncType::new(Box::new([]), Box::new([ValType::F32])),
Rc::new(Ret2),
|_, params, results| {
assert!(params.is_empty());
assert_eq!(results.len(), 1);
results[0] = 2.0f32.into();
Ok(())
},
)
.into(),
];

View File

@@ -15,33 +15,24 @@ fn test_import_calling_export() {
)
"#;
struct Callback {
pub other: RefCell<Option<Func>>,
}
let store = Store::default();
let module = Module::new(&store, WAT).expect("failed to create module");
impl Callable for Callback {
fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> {
self.other
let other = Rc::new(RefCell::new(None::<Func>));
let other2 = other.clone();
let callback_func = Func::new(
&store,
FuncType::new(Box::new([]), Box::new([])),
move |_, _, _| {
other2
.borrow()
.as_ref()
.expect("expected a function ref")
.call(&[])
.expect("expected function not to trap");
Ok(())
}
}
let store = Store::default();
let module = Module::new(&store, WAT).expect("failed to create module");
let callback = Rc::new(Callback {
other: RefCell::new(None),
});
let callback_func = Func::new(
&store,
FuncType::new(Box::new([]), Box::new([])),
callback.clone(),
},
);
let imports = vec![callback_func.into()];
@@ -55,7 +46,7 @@ fn test_import_calling_export() {
.func()
.expect("expected a run func in the module");
*callback.other.borrow_mut() = Some(
*other.borrow_mut() = Some(
exports[1]
.func()
.expect("expected an other func in the module")
@@ -76,25 +67,17 @@ fn test_returns_incorrect_type() {
)
"#;
struct EvilCallback;
impl Callable for EvilCallback {
fn call(&self, _params: &[Val], results: &mut [Val]) -> Result<(), Trap> {
// Evil! Returns I64 here instead of promised in the signature I32.
results[0] = Val::I64(228);
Ok(())
}
}
let store = Store::default();
let module = Module::new(&store, WAT).expect("failed to create module");
let callback = Rc::new(EvilCallback);
let callback_func = Func::new(
&store,
FuncType::new(Box::new([]), Box::new([ValType::I32])),
callback.clone(),
|_, _, results| {
// Evil! Returns I64 here instead of promised in the signature I32.
results[0] = Val::I64(228);
Ok(())
},
);
let imports = vec![callback_func.into()];
@@ -111,6 +94,6 @@ fn test_returns_incorrect_type() {
let trap = run_func.call(&[]).expect_err("the execution should fail");
assert_eq!(
trap.message(),
"`Callable` attempted to return an incompatible value"
"function attempted to return an incompatible value"
);
}

View File

@@ -1,18 +1,9 @@
use anyhow::Result;
use std::panic::{self, AssertUnwindSafe};
use std::rc::Rc;
use wasmtime::*;
#[test]
fn test_trap_return() -> Result<()> {
struct HelloCallback;
impl Callable for HelloCallback {
fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> {
Err(Trap::new("test 123"))
}
}
let store = Store::default();
let wat = r#"
(module
@@ -23,7 +14,7 @@ fn test_trap_return() -> Result<()> {
let module = Module::new(&store, wat)?;
let hello_type = FuncType::new(Box::new([]), Box::new([]));
let hello_func = Func::new(&store, hello_type, Rc::new(HelloCallback));
let hello_func = Func::new(&store, hello_type, |_, _, _| Err(Trap::new("test 123")));
let instance = Instance::new(&module, &[hello_func.into()])?;
let run_func = instance.exports()[0]
@@ -74,14 +65,6 @@ fn test_trap_trace() -> Result<()> {
#[test]
fn test_trap_trace_cb() -> Result<()> {
struct ThrowCallback;
impl Callable for ThrowCallback {
fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> {
Err(Trap::new("cb throw"))
}
}
let store = Store::default();
let wat = r#"
(module $hello_mod
@@ -92,7 +75,7 @@ fn test_trap_trace_cb() -> Result<()> {
"#;
let fn_type = FuncType::new(Box::new([]), Box::new([]));
let fn_func = Func::new(&store, fn_type, Rc::new(ThrowCallback));
let fn_func = Func::new(&store, fn_type, |_, _, _| Err(Trap::new("cb throw")));
let module = Module::new(&store, wat)?;
let instance = Instance::new(&module, &[fn_func.into()])?;
@@ -223,14 +206,6 @@ wasm backtrace:
#[test]
fn trap_start_function_import() -> Result<()> {
struct ReturnTrap;
impl Callable for ReturnTrap {
fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> {
Err(Trap::new("user trap"))
}
}
let store = Store::default();
let binary = wat::parse_str(
r#"
@@ -243,7 +218,7 @@ fn trap_start_function_import() -> Result<()> {
let module = Module::new(&store, &binary)?;
let sig = FuncType::new(Box::new([]), Box::new([]));
let func = Func::new(&store, sig, Rc::new(ReturnTrap));
let func = Func::new(&store, sig, |_, _, _| Err(Trap::new("user trap")));
let err = Instance::new(&module, &[func.into()]).err().unwrap();
assert_eq!(err.downcast_ref::<Trap>().unwrap().message(), "user trap");
Ok(())
@@ -251,14 +226,6 @@ fn trap_start_function_import() -> Result<()> {
#[test]
fn rust_panic_import() -> Result<()> {
struct Panic;
impl Callable for Panic {
fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> {
panic!("this is a panic");
}
}
let store = Store::default();
let binary = wat::parse_str(
r#"
@@ -273,7 +240,7 @@ fn rust_panic_import() -> Result<()> {
let module = Module::new(&store, &binary)?;
let sig = FuncType::new(Box::new([]), Box::new([]));
let func = Func::new(&store, sig, Rc::new(Panic));
let func = Func::new(&store, sig, |_, _, _| panic!("this is a panic"));
let instance = Instance::new(
&module,
&[
@@ -302,14 +269,6 @@ fn rust_panic_import() -> Result<()> {
#[test]
fn rust_panic_start_function() -> Result<()> {
struct Panic;
impl Callable for Panic {
fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> {
panic!("this is a panic");
}
}
let store = Store::default();
let binary = wat::parse_str(
r#"
@@ -322,7 +281,7 @@ fn rust_panic_start_function() -> Result<()> {
let module = Module::new(&store, &binary)?;
let sig = FuncType::new(Box::new([]), Box::new([]));
let func = Func::new(&store, sig, Rc::new(Panic));
let func = Func::new(&store, sig, |_, _, _| panic!("this is a panic"));
let err = panic::catch_unwind(AssertUnwindSafe(|| {
drop(Instance::new(&module, &[func.into()]));
}))