Skip to content
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

Passing arguments while calling exported functions #17

Closed
pepyakin opened this issue Sep 4, 2018 · 2 comments
Closed

Passing arguments while calling exported functions #17

pepyakin opened this issue Sep 4, 2018 · 2 comments

Comments

@pepyakin
Copy link
Contributor

pepyakin commented Sep 4, 2018

To call an exported function we need to be able to pass arguments to it. I have a two straw man approaches to implement that.

The first one: pass arguments dynamically. That is, every exported function should have a thunk generated. This thunk reads all arguments from the dynamic list of argument values (a-la wasmi's RuntimeValue) and then call into the original function with the whatever ABI it has. Upon calling the exported function, we check that the argument slice matches the signature and then call we call the thunk with the fixed signature (vmctx, args_ptr). This thunk could also convert and write the return value to the specifc pre-allocated location in the callee. Dumb but easy.

The second one. Generate exported functions with the specific ABI, say system_v. Then introduce an argument wrapper trait.

unsafe trait ArgsWrapper {
  fn signature()WasmSignature;

  fn call(fn_ptr: usize, vmctx: usize, args: Self);
}

This trait's main purpose is to provide a way to get the signature dynamically and unpack the values for the actual call. We can generate impls for this trait by a macro. Here is an example impl for function with arity 2.

unsafe impl<A: HasWasmType, B: HasWasmType> ArgsWrapper for (A, B) {
  fn signature()WasmSignature {
    WasmSignature::new(&[A::wasm_type(), B::wasm_type()])
  }
  unsafe fn call(fn_ptr: usize, vmctx: usize, args: (A, B)) {
    let (a1, a2) = args;
    let f = fn_ptr as extern "C" fn(usize, A, B);
    unsafe {
      f(vmctx, a1, a2)
    }
  }
}

Then actual call can be implemented as following:

fn call<A: ArgsWrapper >(func_name: &str, args: A) {
  let wasm_func = self.funcs.get(func_name).unwrap();
  assert_eq!(wasm_func.signature, A::signature());
  unsafe { A::call(wasm_func.ptr, args) }
}

I think this approach can be scaled to handle returning values as well.

@sunfishcode what do you think? Am I missing something, do you have better proposals?

@sunfishcode
Copy link
Member

Sorry for the late response here! I've been busy and somehow lost track of this issue.

My general preference would be for the second approach. Cranelift has system_v and windows_fastcall specifically to allow it to implement platform-native calling conventions, and I think it makes sense to take advantage of that here. I like the idea of using a helper function like this to wrap up the unsafe code and the vmctx argument.

@sunfishcode
Copy link
Member

I've now implemented an approach where in we use the JIT to generate trampoline functions. We store the argument values onto the stack, the trampoline function loads them and makes the call, and then stores the return values onto the stack. Take a look at trampoline_park.rs for details, or just use InstancePlus and call invoke as desired :-).

kubkon pushed a commit to kubkon/wasmtime that referenced this issue Mar 11, 2020
* atoms in one test unit

* factor out pointers test

* factor structs into separate test unit

* factor out arrays, flags

* finally, separate into strings and ints
grishasobol pushed a commit to grishasobol/wasmtime that referenced this issue Nov 29, 2021
pchickey pushed a commit to pchickey/wasmtime that referenced this issue May 12, 2023
* Update a few aspects of the adapter build

* Use the `wasm32-unknown-unknown` target for the adapter and specify
  flags in `.cargo/config.toml` to avoid having to pass the same flags
  everywhere. This allows using `wasm32-wasi` for tests to ensure the
  flags only apply to the adapter.

* Use `opt-level=s` since speed is not of the utmost concern for this
  wasm but since it's likely to be included in many places size is
  likely more important.

* Use `strip = 'debuginfo'` for the release build to remove the standard
  library's debugging information which isn't necessary.

* Remove `debug = 0` from the `dev` profile to have debugging
  information for development.

* Add a small `README.md` describing what's here for now.

* Move `command` support behind a `command` feature

This commit adds a `command` feature to main crate to avoid importing
the `_start` function when the `command` feature is disabled, making
this adapter useful for non-command WASI programs as well.

For now this still emits the `command` export in the final component but
with `use` in `*.wit` files it should be easier to avoid that export.
pchickey pushed a commit to pchickey/wasmtime that referenced this issue May 16, 2023
* Update a few aspects of the adapter build

* Use the `wasm32-unknown-unknown` target for the adapter and specify
  flags in `.cargo/config.toml` to avoid having to pass the same flags
  everywhere. This allows using `wasm32-wasi` for tests to ensure the
  flags only apply to the adapter.

* Use `opt-level=s` since speed is not of the utmost concern for this
  wasm but since it's likely to be included in many places size is
  likely more important.

* Use `strip = 'debuginfo'` for the release build to remove the standard
  library's debugging information which isn't necessary.

* Remove `debug = 0` from the `dev` profile to have debugging
  information for development.

* Add a small `README.md` describing what's here for now.

* Move `command` support behind a `command` feature

This commit adds a `command` feature to main crate to avoid importing
the `_start` function when the `command` feature is disabled, making
this adapter useful for non-command WASI programs as well.

For now this still emits the `command` export in the final component but
with `use` in `*.wit` files it should be easier to avoid that export.
frank-emrich added a commit to frank-emrich/wasmtime that referenced this issue Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants