rustwasm / wasm-bindgen

Facilitating high-level interactions between Wasm modules and JavaScript
https://rustwasm.github.io/docs/wasm-bindgen/
Apache License 2.0
7.81k stars 1.08k forks source link

`wasm-bindgen-wasm-interpreter` can't handle extra block #3315

Open Niedzwiedzw opened 1 year ago

Niedzwiedzw commented 1 year ago
Finished release [optimized] target(s) in 0.08s
[INFO]: License key is set in Cargo.toml but no LICENSE file(s) were found; Please add the LICENSE file(s) to your project directory
[INFO]: Installing wasm-bindgen...
thread 'main' panicked at 'unknown instruction Block(Block { seq: Id { idx: 1 } })', crates/wasm-interpreter/src/lib.rs:367:18
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: wasm_bindgen_wasm_interpreter::Interpreter::call
   3: wasm_bindgen_wasm_interpreter::Interpreter::interpret_descriptor
   4: wasm_bindgen_cli_support::descriptors::execute
   5: wasm_bindgen_cli_support::Bindgen::generate_output
   6: wasm_bindgen_cli_support::Bindgen::generate
   7: wasm_bindgen::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Error: Running the wasm-bindgen CLI
Caused by: failed to execute `wasm-bindgen`: exited with exit status: 101
  full command: "/home/niedzwiedz/.cache/.wasm-pack/wasm-bindgen-5f26acfc988649a3/wasm-bindgen" "/home/niedzwiedz/crionis/crio-app/target/wasm32-unknown-unknown/release/editor.wasm" "--out-dir" "/home/niedzwiedz/crionis/crio-app/pkg" "--typescript" "--target" "bundler" "--out-name" "index"

EDIT: https://github.com/rustwasm/wasm-pack/issues/1229 <- I've opened this issue here to, cause I have no idea what causes the bug

EDIT: https://github.com/rustwasm/wasm-bindgen/blob/629a623adfe27c67a21408c120d3466965575e45/crates/wasm-interpreter/src/lib.rs#L367 this is the line that crashes

csnover commented 1 year ago

I encountered the unknown instruction Block panic as well trying to compile a library to wasm32-wasi and discovered that this is because the bindgen descriptor interpreter is trying to call through __wasm_call_ctors and __wasm_call_dtors, at least the former of which contains complex functions that the interpreter isn’t designed to handle. According to this ticket, LLVM 13 changed its default output to include these in exported functions by default. Looking at the output of the compiler, it seems that the -Zwasi-exec-model flag in a nightly compiler may suppress these extra calls, but in any case wasm-bindgen probably should just be updated to avoid calling them since they aren’t relevant to what it is doing.

This is probably a duplicate of #2673.

Liamolucko commented 1 year ago

I encountered the unknown instruction Block panic as well trying to compile a library to wasm32-wasi and discovered that this is because the bindgen descriptor interpreter is trying to call through __wasm_call_ctors and __wasm_call_dtors, at least the former of which contains complex functions that the interpreter isn’t designed to handle. According to this ticket, LLVM 13 changed its default output to include these in exported functions by default. Looking at the output of the compiler, it seems that the -Zwasi-exec-model flag in a nightly compiler may suppress these extra calls, but in any case wasm-bindgen probably should just be updated to avoid calling them since they aren’t relevant to what it is doing.

This is probably a duplicate of #2673.

Yeah, I think your issue is the same as #2673 (and #2969), but I'm not so sure about @Niedzwiedzw's. They're using wasm-pack, and wasm-pack compiles for wasm32-unknown-unknown, not wasm32-wasi, so I don't see how they could be running into that issue.

@Niedzwiedzw, could you give some instructions for how to reproduce this?

daxpedda commented 1 year ago

Closing in favor of #3421.

Michael-F-Bryan commented 1 year ago

@daxpedda can you re-open this ticket? The OP was actually compiling to wasm32-unknown-unknown (you can see crio-app/target/wasm32-unknown-unknown/release in their error message), so this issue isn't related to wasm32-wasi.

I'm running into the exact same issue compiling wasmer/wasmer-js to wasm32-unknown-unknown using wasm-pack.

Michael-F-Bryan commented 1 year ago

I did some more troubleshooting for this and found that the underlying cause was the #[tracing::instrument] attribute I added to my #[wasm_bindgen] function.

When compiling without optimisations mode, this introduces extra Instr::Block and Instr::BrIf instructions that the minimal wasm interpreter used by wasm-bindgen can't handle.

In my case, all I needed to do was remove the #[tracing::instrument] calls and start the span inside the functions.

 #[wasm_bindgen]
 impl Instance {
     /// Wait for the process to exit.
-    #[tracing::instrument(level = "debug", skip_all)]
     pub async fn wait(self) -> Result<JsOutput, Error> {
+        let _span = tracing::debug_span!("wait").entered();
+
         let Instance {
             stdin,
             stdout,
daxpedda commented 1 year ago

Thank you for looking into it!

seancribbs commented 10 months ago

To add additional context since I just investigated this myself (also related to tracing::instrument), the codegen unconditionally emits the attributes attached to the original function onto the generated bindgen and descriptor functions. Is the rationale for this behavior to support conditional compilation?

Would it be useful to have an option for the wasm_bindgen macro that lets you filter out attributes with specific names? I could see that being straightforward to add and would be willing to try.

daxpedda commented 10 months ago

Would it be useful to have an option for the wasm_bindgen macro that lets you filter out attributes with specific names? I could see that being straightforward to add and would be willing to try.

It would be nice to do it the other way around: only pass down specific attributes and then optionally include user-specified ones. But I guess that would be a breaking change.

I'm happy to review a PR though.

HarukaMa commented 10 months ago

In my case, it's wasm-opt which generates block instructions (-O2 and above) instead of directly generated by the compiler. Only happens with certain code, can't find any pattern for that.

daxpedda commented 10 months ago

If understand that correctly you are using wasm-bindgen-cli after processing the file with wasm-opt. But you have to use wasm-opt after processing the file with wasm-bindgen!

HarukaMa commented 10 months ago

didn't know the order mattered as it was working fine for me. guess I just got super lucky or something like that. Thanks for the tip!