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

wai-bindgen-wasmer::wasmer doesn't work on the web #3870

Closed
kajacx opened this issue May 16, 2023 · 1 comment · Fixed by #3881
Closed

wai-bindgen-wasmer::wasmer doesn't work on the web #3870

kajacx opened this issue May 16, 2023 · 1 comment · Fixed by #3881

Comments

@kajacx
Copy link
Contributor

kajacx commented May 16, 2023

EDIT:

This "bug" is not caused by the difference between wasmer and wai-bindgen-rust::wasmer, instead, it was caused by not including the wasm-types-polyfill feature. See the next comment.

Original post:

Describe the bug

Using wai-bindgen-wasmer::wasmer on the web (with the js feature) doesn't work correctly. When loading a function, wasmer thinks that the function has no arguments. Using wasmer directly, on the other hand, works.

Steps to reproduce

  1. Clone this repo at the wasmer-works-but-not-in-wai tag.
  2. Run the run-wasm-only-forked.sh script and open it in browser to see the error in JS console.
  3. Run the run-no-wai-web.sh script and open it in browser to see it work correctly.

Expected behavior

Using wai-bindgen-wasmer::wasmer should work exactly the same as using wasmer directly, since both folders use wasmer 3.3:

  • wasm-only-forked is using wai-bindgen-wasmer 0.4.0, which is using wasmer 3.3.0
  • no-wai-web is using wasmer 3.3.0 directly

Actual behavior

Getting a typed function in wasm-only-forked gives this error:

should cast to typed: RuntimeError { source: Trap { inner: User(RuntimeStringError { details: "given types ([I32]) for the function arguments don't match the actual types ([])" }) }, wasm_trace: [] }

Additional context

The problem originates in file wasm-only-forked/src/lib.rs, lile 32:

    let add_three = instance
        .exports
        .get_function("add_three")
        .expect("should get function")
        .typed::<i32, i32>(&store)
        .expect("should cast to typed");

Here is the code in wasmer, for convenience:

    pub fn typed<Args, Rets>(
        &self,
        store: &impl AsStoreRef,
    ) -> Result<TypedFunction<Args, Rets>, RuntimeError>
    where
        Args: WasmTypeList,
        Rets: WasmTypeList,
    {
        let ty = self.ty(store);

        // type check
        {
            let expected = ty.params();
            let given = Args::wasm_types();

            if expected != given {
                return Err(RuntimeError::new(format!(
                    "given types (`{:?}`) for the function arguments don't match the actual types (`{:?}`)",
                    given,
                    expected,
                )));
            }
        }
        
        // ...
    }

Here is the WASM plugin in WAT format, decompiled by wasm2wat demo:

(module
  (type $t0 (func (param i32) (result i32)))
  (func $add_three (export "add_three") (type $t0) (param $p0 i32) (result i32)
    (i32.add
      (local.get $p0)
      (i32.const 3)))
  (table $T0 1 1 funcref)
  (memory $memory (export "memory") 16)
  (global $__stack_pointer (mut i32) (i32.const 1048576))
  (global $__data_end (export "__data_end") i32 (i32.const 1048576))
  (global $__heap_base (export "__heap_base") i32 (i32.const 1048576)))

Note:
When using the sys feature (instead of js), it works both when using wai-bindgen-wasmer::wasmer and in wasmer directly.

I have tried deleting the target folder and my Cargo.lock file, but with the same result.

@kajacx kajacx changed the title wai-bindgen-wasmer::wasmer doesn't work, but standalone wasmer does wai-bindgen-wasmer::wasmer doesn't work on the web May 16, 2023
@kajacx
Copy link
Contributor Author

kajacx commented May 18, 2023

Ok, found the problem. wasmer/js-default enables, js, std, and wasm-types-polyfill, but wai-bindgen-wasmer/js enables only js and std. Relevant code:

    /// Creates a new WebAssembly module skipping any kind of validation from a javascript module
    ///
    pub(crate) unsafe fn from_js_module(
        module: WebAssembly::Module,
        binary: impl IntoBytes,
    ) -> Self {
        let binary = binary.into_bytes();
        // The module is now validated, so we can safely parse it's types
        #[cfg(feature = "wasm-types-polyfill")]
        let (type_hints, name) = {
            let info = crate::js::module_info_polyfill::translate_module(&binary[..]).unwrap();

            (
                Some(ModuleTypeHints {
                    imports: info
                        .info
                        .imports()
                        .map(|import| import.ty().clone())
                        .collect::<Vec<_>>(),
                    exports: info
                        .info
                        .exports()
                        .map(|export| export.ty().clone())
                        .collect::<Vec<_>>(),
                }),
                info.info.name,
            )
        };
        #[cfg(not(feature = "wasm-types-polyfill"))]
        let (type_hints, name) = (None, None);

        Self {
            module,
            type_hints,
            name,
            #[cfg(feature = "js-serializable-module")]
            raw_bytes: Some(binary.into_bytes()),
        }
    }

Pull request coming soon.

kajacx added a commit to kajacx/wasmer that referenced this issue May 18, 2023
Fixes wasmerio#3870

`wasm-types-polyfill` feature, which is enabled by `js-default`, is needed for type annotations.
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

Successfully merging a pull request may close this issue.

1 participant