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

Externref/Reference types throwing error in Node #2274

Closed
Tarnadas opened this issue Aug 7, 2020 · 15 comments
Closed

Externref/Reference types throwing error in Node #2274

Tarnadas opened this issue Aug 7, 2020 · 15 comments
Labels

Comments

@Tarnadas
Copy link
Contributor

Tarnadas commented Aug 7, 2020

Building with --reference-types --target nodejs and importing the result in Node results in the following error:

❯ node --experimental-wasm-reftypes .
/home/marior/projects/wasm-node-externref/dist/wasm_node_externref.js:145
const wasmModule = new WebAssembly.Module(bytes);
                   ^

CompileError: WebAssembly.Module(): Compiling function #17:"wasm_bindgen::externref::Slab::alloc::ha6442f5e..." failed: i32.rem_s[1] expected type i32, found ref.null of type nullref @+7561
    at Object.<anonymous> (/home/marior/projects/wasm-node-externref/dist/wasm_node_externref.js:145:20)
    at Module._compile (internal/modules/cjs/loader.js:1256:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1277:10)
    at Module.load (internal/modules/cjs/loader.js:1105:32)
    at Function.Module._load (internal/modules/cjs/loader.js:967:14)
    at Module.require (internal/modules/cjs/loader.js:1145:19)
    at require (internal/modules/cjs/helpers.js:75:18)
    at Object.<anonymous> (/home/marior/projects/wasm-node-externref/index.js:1:14)
    at Module._compile (internal/modules/cjs/loader.js:1256:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1277:10)

Repository

Tested with Node 14.17.0 and 12.18.3

Interestingly this bug doesn't happen with Deno.
Is this bug related to Node, v8 or wasm-bindgen?
Node 14.17.0 uses v8 8.4.371.19-node.12 and Deno 1.2.2 uses v8 8.5.216

@Tarnadas Tarnadas added the bug label Aug 7, 2020
@alexcrichton
Copy link
Contributor

Thanks for the report! I think this is because node hasn't updated to the latest version of reference types, so we'll need to wait for them to update v8.

@Tarnadas
Copy link
Contributor Author

Tarnadas commented Aug 9, 2020

Thanks for clarifying!
However I find it a bit weird that there is an option to enable anyref, but it doesn’t work.
Are you sure that this is related to v8 version?

@Tarnadas
Copy link
Contributor Author

Here is the PR, which will update Node v8 to 8.5:
nodejs/node#34337

@alexcrichton
Copy link
Contributor

Thanks for the pointer, looks like things should work as expected once that propagates!

@Tarnadas
Copy link
Contributor Author

Node 15 will ship with v8 8.6, so I'll give this another try

@Tarnadas
Copy link
Contributor Author

Confirming that it now works with Node 15 and v8 8.6

@Tarnadas
Copy link
Contributor Author

Tarnadas commented Dec 11, 2020

Unfortunately, I now got another error with Node 15.4.0 and wasm-bindgen 0.2.69 with a more advanced example and reference types enabled.

The error looks a bit different:

const wasmModule = new WebAssembly.Module(bytes);
                   ^

CompileError: WebAssembly.Module(): Compiling function #99:"__externref_table_alloc" failed: local.tee[0] expected type i32, found ref.null of type externref @+39165

The generated JS code at that position calling the __externref_table_alloc function looks like this:

function addToExternrefTable0(obj) {
    const idx = wasm.__externref_table_alloc();
    wasm.__wbindgen_export_2.set(idx, obj);
    return idx;
}

I hope this information is enough since I'm not sure what causes the issue and my code is not yet ready for being released publicly.
I can also confirm, that the browser version works in the latest Chrome and Firefox.

@alexcrichton
Copy link
Contributor

Hm that may be a mismatch in node's expected encoding of a wasm module and what Rust is doing? I forget the precise verisons at which everything starts to align.

@Tarnadas
Copy link
Contributor Author

I’m gonna retry this once v8 8.7 lands in Nodejs with this PR:
nodejs/node#36182

@Tarnadas
Copy link
Contributor Author

So I tried out the Nodejs version of this specific PR with v8 8.7, but I get the same error.
I then also tried out another branch with a v8 8.8 version right here:
https://github.com/targos/node/tree/v8-88
but I also get the same error, so it seems that this might be related to the v8 version that Nodejs is using?
At least it seems like it is slightly modified, because if you print the v8 version with node -p process.versions.v8 it has a trailing -node.x.

@Tarnadas
Copy link
Contributor Author

I wanted to open an issue in the Nodejs repository, if anyone has an idea and I wanted to send there the relevant code as wat, so I tried to compile my wasm to a wat file with the tools from:
https://github.com/WebAssembly/wabt

However I get a similar error:

❯ ../wabt/build/wasm2wat ./pkg/server_bg.wasm --enable-all
./pkg/server_bg.wasm:0009901: error: type mismatch in local.tee, expected [i32] but got [... externref]
./pkg/server_bg.wasm:0009904: error: type mismatch in table.grow, expected [externref, i32] but got [i32, i32]
./pkg/server_bg.wasm:0009a8d: error: type mismatch in function, expected [] but got [i32, i32]
./pkg/server_bg.wasm:0009da3: error: type mismatch in function, expected [] but got [i32]
./pkg/server_bg.wasm:000ac58: error: type mismatch in function, expected [] but got [i32, i32, i32]

@alexcrichton do you still think this is an issue from the specs not yet being aligned?

@alexcrichton
Copy link
Contributor

Unfortunately I'm not entirely sure. If other runtimes accept the same wasm file then it seems like this is node-specific though?

@Tarnadas
Copy link
Contributor Author

I quadruple checked this again and it definitely seems as if the generated wasm code works in Chrome and FF.
I generated the code with wasm-pack and --target web, then in my index.html, this successfully prints:

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-type" content="text/html; charset=utf-8" />
    <title>My awesome Rust, WebAssembly, and Webpack application</title>
  </head>
  <body>
    <div id="root"></div>
    <script>
      import("./index.js").then((m) => {
        console.log("wasm", m);
      });
    </script>
  </body>
</html>

I get other awesome errors with Webpack with --target bundler, because it also fails to parse the wasm file, but I'm still on Webpack v4, so this might already be fixed.

    ERROR in .../pkg/index_bg.wasm
    Module parse failed: Internal failure: parseVec could not cast the value
    You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
    Error: Internal failure: parseVec could not cast the value

@alexcrichton
Copy link
Contributor

It may be that webpack's wasm parser doesn't support the reference types proposal perhaps? But otherwise if browsers accept the wasm module then it seems like it may be an issue on node's end? I would have to dig in more to be certain though.

@Tarnadas
Copy link
Contributor Author

I just wanted to confirm, that updating wasm-bindgen from 0.2.70 to 0.2.74 fixed the issue. wasm2wat can now convert the file and Nodejs can now run it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants