-
Notifications
You must be signed in to change notification settings - Fork 824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(runtime-core-tests) Introduce the new wasmer-runtime-core-tests
crate
#916
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…` crate. This non-publishable new crate contains a test suite for the `wasmer-runtime-core` crate. So far, the test suite is rather small, but it aims to be extended in a close future.
Hywan
added
🎉 enhancement
New feature!
📦 lib-deprecated
About the deprecated crates
🧪 tests
I love tests
labels
Oct 30, 2019
bors try |
bors try |
tryAlready running a review |
bors cancel |
tryBuild failed
|
bors try |
tryBuild succeeded
|
MarkMcCaskey
approved these changes
Oct 30, 2019
bors r+ |
bors bot
added a commit
that referenced
this pull request
Oct 30, 2019
916: feat(runtime-core-tests) Introduce the new `wasmer-runtime-core-tests` crate r=Hywan a=Hywan Extracted from #882. This non-publishable new crate contains a test suite for the `wasmer-runtime-core` crate. So far, the test suite is rather small, but it aims to be extended in a close future. Co-authored-by: Ivan Enderlin <[email protected]>
Build succeeded
|
bors bot
added a commit
that referenced
this pull request
Oct 31, 2019
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 bot
added a commit
that referenced
this pull request
Nov 12, 2019
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]>
bors bot
added a commit
that referenced
this pull request
Nov 12, 2019
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]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Extracted from #882.
This non-publishable new crate contains a test suite for the
wasmer-runtime-core
crate. So far, the test suite is rather small,but it aims to be extended in a close future.