feat(runtime-core) Host function without a `vm::Ctx` argument #917

Hywan · 2019-10-30T14:15:17Z

Extracted from #882.
~~Includes #916. Must not be merged until #916 lands in master.~~
If you want to compare only the different commits, see https://github.com/Hywan/wasmer/compare/feat-runtime-core-tests...Hywan:feat-runtime-core-host-function-without-vmctx?expand=1.

This patch updates the ExternalFunction implementation to support host functions (aka imported functions) without having an explicit vm::Ctx argument.

It is thus possible to write:

fn add_one(x: i32) -> i32 {
    x + 1
}

let import_object = imports! {
    "env" => {
        "add_one" => func!(add_one);
    },
};

The previous form is still working though:

fn add_one(_: &mut vm::Ctx, x: i32) -> i32 {
    x + 1
}

The backends aren't modified. It means that the pointer to vm::Ctx is still inserted in the stack, but it is not used when the function doesn't need it.

We thought this is an acceptable trade-off.

About the C API. It has not check to ensure the type signature. Since the pointer to vm::Ctx is always inserted, the C API can digest host functions with or without a vm::Ctx argument. The C API uses the Export + FuncPointer API instead of going through the Func API (which is modified by this API), which is only possible since the C API only receives an opaque function pointer. Conclusion is: We can't ensure anything on the C side, but it will work with or without the vm::Ctx argument in theory.

Hywan · 2019-10-30T14:16:32Z

bors try

Hywan · 2019-10-30T14:22:38Z

lib/runtime-core/src/typed_func.rs

@@ -438,33 +472,91 @@ macro_rules! impl_traits {
            }
        }

-        impl< $( $x, )* Rets, Trap, FN > ExternalFunction<( $( $x ),* ), Rets> for FN
+        impl< $( $x, )* Rets, Trap, FN > ExternalFunction<ExplicitVmCtx, ( $( $x ),* ), Rets> for FN


Implementation of ExternalFunction<ExplicitVmCtx, …>, where FN must be Fn(&mut vm::Ctx, …). The wrap function has a &mut vm::Ctx argument.

Note for the ones who aren't familiar with this detail: The wrap function will be dereferenced to *mut vm::Func (opaque function pointer), and will be used as FuncPointer by the Export API. So wrap acts as a “wrapper” around the user-given function.

Hywan · 2019-10-30T14:24:22Z

lib/runtime-core/src/typed_func.rs

+            }
+        }
+
+        impl< $( $x, )* Rets, Trap, FN > ExternalFunction<ImplicitVmCtx, ( $( $x ),* ), Rets> for FN


Implementation of ExternalFunction<ImplicitVmCtx, …>, where FN must be Fn(…) (without vm::Ctx). The wrap function still has a &mut vm::Ctx argument! Why? Because the backend always inserts a pointer to vm::Ctx in the stack, so wrap can read it.

Why grabbing vm::Ctx then? It is mandatory to trap error with do_early_trap.

bors · 2019-10-30T14:47:58Z

try

Build succeeded

wasmerio.wasmer

bors · 2019-10-30T15:18:14Z

try

Build succeeded

wasmerio.wasmer

Just to be consistent with the rest of the code.

… argument. This patch allows host functions to get a signature without an explicit `vm::Ctx` argument. It is for Rust only. The C API receives a function pointer and has no clue whether a `vm::Ctx` argument is present or not, so it assumes it is always declared. From the backend point of view, the pointer to `vm::Ctx` is always inserted in the stack, but it is not used by the function when the argument is absent.

MarkMcCaskey

Looks pretty good to me, I'm not completely confident in the transmute_copys, those are easy to mess up. Approving, but I'd like to talk with you a bit about some of the specifics here

lib/runtime-core/src/typed_func.rs

MarkMcCaskey · 2019-10-30T22:29:53Z

lib/runtime-core/src/typed_func.rs

+        {
+            #[allow(non_snake_case)]
+            fn to_raw(&self) -> NonNull<vm::Func> {
+                if mem::size_of::<Self>() == 0 {


Presumably Rust wasn't happy with you using the Sized bound?

Self refers to FN so to the Fn trait. Writing mem::size_of::<Self>() == 0 is a way to tell: FN is a function pointer, or a closure with no captured environment.

This code will change in the next PR to support closures as host functions. So far, this PR only copy-paste the existing code by definingvm::Ctx as an optional argument.

Hywan · 2019-10-31T09:32:24Z

bors r+

bors · 2019-10-31T09:49:11Z

Merge conflict

Hywan · 2019-10-31T10:00:32Z

bors r+

917: feat(runtime-core) Host function without a `vm::Ctx` argument r=Hywan a=Hywan Extracted from #882. ~Includes #916. Must not be merged until #916 lands in `master`.~ ~If you want to compare only the different commits, see https://github.com/Hywan/wasmer/compare/feat-runtime-core-tests...Hywan:feat-runtime-core-host-function-without-vmctx?expand=1.~ This patch updates the `ExternalFunction` implementation to support host functions (aka imported functions) without having an explicit `vm::Ctx` argument. It is thus possible to write: ```rust fn add_one(x: i32) -> i32 { x + 1 } let import_object = imports! { "env" => { "add_one" => func!(add_one); }, }; ``` The previous form is still working though: ```rust fn add_one(_: &mut vm::Ctx, x: i32) -> i32 { x + 1 } ``` The backends aren't modified. It means that the pointer to `vm::Ctx` is still inserted in the stack, but it is not used when the function doesn't need it. We thought this is an acceptable trade-off. About the C API. It has not check to ensure the type signature. Since the pointer to `vm::Ctx` is always inserted, the C API can digest host functions with or without a `vm::Ctx` argument. The C API uses [the `Export` + `FuncPointer` API](https://github.com/wasmerio/wasmer/blob/cf5b3cc09eff149ce415bd81846d84b2d2cdf3a3/lib/runtime-c-api/src/import/mod.rs#L630-L653) instead of going through the `Func` API (which is modified by this API), which is only possible since the C API only receives an opaque function pointer. Conclusion is: We can't ensure anything on the C side, but it will work with or without the `vm::Ctx` argument in theory. Co-authored-by: Ivan Enderlin <[email protected]>

bors · 2019-10-31T10:32:10Z

Build succeeded

wasmerio.wasmer

925: feat(runtime-core) Support closures with a captured environment as host functions r=Hywan a=Hywan Reboot of #882 and #219. For the moment, host functions (aka imported functions) can be regular function pointers, or (as a side-effect) closures without a captured environment. This PR extends the support of host functions to closures with a captured environment. This is required for many other features (incl. the Python integration, the Ruby integration, WebAssembly Interface Types [see #787], and so on). This PR is the culmination of previous ones, notably #915, #916 and #917. ### General idea The user-defined host function is wrapped inside a `wrap` function. This wrapper function initially receives a `vm::Ctx` as its first argument, which is passed to the host function when necessary. The patch keeps this behavior but it comes from `vm::FuncCtx`, which is a new structure. A `vm::FuncCtx` is held by `vm::ImportedFunc` such as: ```rust #[repr(C)] pub struct ImportedFunc { pub(crate) func: *const Func, pub(crate) func_ctx: NonNull<FuncCtx>, } ``` where `vm::FuncCtx` is: ```rust #[repr(C)] pub struct FuncCtx { pub(crate) vmctx: NonNull<Ctx>, pub(crate) func_env: Option<NonNull<FuncEnv>>, } ``` where `vm::FuncEnv` is: ```rust #[repr(transparent)] pub struct FuncEnv(pub(self) *mut c_void); ``` i.e. a raw opaque pointer. So the wrapper function of a host function receives a `vm::Ctx`, which is used to find out the associated `FuncCtx` (by using the import backing), which holds `vm::FuncEnv`. It holds a pointer to the closure captured environment. ### Implementation details #### How to get a pointer to a closure captured environment A closure with a captured environment has a memory size greater than zero. This is how we detect it: ```rust if mem::size_of::<Self>() != 0 { … } ``` To get a pointer to its captured environment, we use this statement: ```rust NonNull::new(Box::into_raw(Box::new(self))).map(NonNull::cast) ``` (in `typed_func.rs`, in the `wrap` functions). To reconstruct the closure based on the pointer, we use this statement: ```rust let func: &FN = { let func: NonNull<FN> = func_env.cast(); &*func.as_ptr() }; ``` That's basically how it works. And that's the core idea of this patch. As a side effect, we have removed an undefined behavior (UB) in 2 places: The `mem::transmute(&())` has been removed (it was used to get the function pointer of `FN`). The transmute is replaced by `FuncEnv`, which provides a unified API, erasing the difference between host functions as closures with a captured environment, and host functions as function pointer. For a reason I ignore, the UB wasn't showing himself until this PR and a modification in the Singlepass backend. But now it's fixed. #### Impact on `Backing` After the modification on the `typed_func` and the `vm` modules, this is the other core idea of this patch: Updating the `backing` module so that `vm::ImportedFunc` replaces `vm::Ctx` by `vm::FuncCtx`. When creating `vm::ImportedFunc`, a new `vm::FuncCtx` is created and its pointer is used. We are purposely leaking `vm::FuncCtx` so that the pointer is always valid. Hence the specific `Drop` implementation on `ImportBacking` to dereference the pointer, and to drop it properly. #### Impact on the backends Since the structure of `vm::ImportedFunc` has changed, backends must be updated. We must deref `FuncCtx` to reach its `vmctx` field. #### Impact on `Instance` Because `vm::ImportedFunc` has changed, it has a minor impact on the `instance` module, nothing crazy though. Co-authored-by: Ivan Enderlin <[email protected]>

Hywan added 📦 lib-deprecated About the deprecated crates 🧪 tests I love tests 📚 documentation Do you like to read? labels Oct 30, 2019

Hywan requested review from bjfish, MarkMcCaskey and syrusakbary as code owners October 30, 2019 14:15

bors bot added a commit that referenced this pull request Oct 30, 2019

Try #917:

0ead96f

Hywan commented Oct 30, 2019

View reviewed changes

Hywan added 5 commits October 30, 2019 19:34

chore(runtime-core) Use vm::Ctx to access it.

77e01f0

Just to be consistent with the rest of the code.

test(runtime-core) Add tests for the Func structure.

bd9d4d9

test(runtime-core) Test host functions without a vm::Ctx argument.

7eef49b

doc(runtimecore) Explain ExternalFunctionKind with more details.

6a1d490

Hywan force-pushed the feat-runtime-core-host-function-without-vmctx branch from b6c2a0f to 6a1d490 Compare October 30, 2019 18:34

Hywan changed the title ~~[blocked by #916] feat(runtime-core) Host function without a vm::Ctx argument~~ feat(runtime-core) Host function without a vm::Ctx argument Oct 30, 2019

MarkMcCaskey approved these changes Oct 30, 2019

View reviewed changes

Hywan added 2 commits October 31, 2019 10:59

Merge branch 'master' into feat-runtime-core-host-function-without-vmctx

2703d48

chore(cargo) Update Cargo.lock.

912eb32

bors bot merged commit 912eb32 into wasmerio:master Oct 31, 2019

Hywan mentioned this pull request Nov 4, 2019

feat(runtime-core) Support closures with a captured environment as host functions #925

Merged

Hywan added a commit to Hywan/wasmer that referenced this pull request Nov 6, 2019

doc(changelog) Add wasmerio#915, wasmerio#917 and wasmerio#925.

ac8aece

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(runtime-core) Host function without a `vm::Ctx` argument #917

feat(runtime-core) Host function without a `vm::Ctx` argument #917

Hywan commented Oct 30, 2019 •

edited

Loading

Hywan commented Oct 30, 2019

Hywan Oct 30, 2019

Hywan Oct 30, 2019

bors bot commented Oct 30, 2019

bors bot commented Oct 30, 2019

MarkMcCaskey left a comment

MarkMcCaskey Oct 30, 2019

Hywan Oct 31, 2019

Hywan commented Oct 31, 2019

bors bot commented Oct 31, 2019

Hywan commented Oct 31, 2019

bors bot commented Oct 31, 2019

feat(runtime-core) Host function without a vm::Ctx argument #917

feat(runtime-core) Host function without a vm::Ctx argument #917

Conversation

Hywan commented Oct 30, 2019 • edited Loading

Hywan commented Oct 30, 2019

Hywan Oct 30, 2019

Choose a reason for hiding this comment

Hywan Oct 30, 2019

Choose a reason for hiding this comment

bors bot commented Oct 30, 2019

try

Build succeeded

bors bot commented Oct 30, 2019

try

Build succeeded

MarkMcCaskey left a comment

Choose a reason for hiding this comment

MarkMcCaskey Oct 30, 2019

Choose a reason for hiding this comment

Hywan Oct 31, 2019

Choose a reason for hiding this comment

Hywan commented Oct 31, 2019

bors bot commented Oct 31, 2019

Merge conflict

Hywan commented Oct 31, 2019

bors bot commented Oct 31, 2019

Build succeeded

feat(runtime-core) Host function without a `vm::Ctx` argument #917

feat(runtime-core) Host function without a `vm::Ctx` argument #917

Hywan commented Oct 30, 2019 •

edited

Loading