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

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

Open
Niedzwiedzw opened this issue Feb 16, 2023 · 11 comments
Open

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

Niedzwiedzw opened this issue Feb 16, 2023 · 11 comments
Labels

Comments

@Niedzwiedzw
Copy link

Niedzwiedzw commented Feb 16, 2023

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: rustwasm/wasm-pack#1229 <- I've opened this issue here to, cause I have no idea what causes the bug

EDIT:

s => panic!("unknown instruction {:?}", s),
this is the line that crashes

@csnover
Copy link

csnover commented Mar 23, 2023

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
Copy link
Collaborator

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
Copy link
Collaborator

Closing in favor of #3421.

@daxpedda daxpedda closed this as not planned Won't fix, can't repro, duplicate, stale May 11, 2023
@Michael-F-Bryan
Copy link

@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
Copy link

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 daxpedda reopened this Aug 22, 2023
@daxpedda
Copy link
Collaborator

Thank you for looking into it!

@daxpedda daxpedda changed the title wasm-pack build panics on Installing wasm-bindgen... step wasm-bindgen-wasm-interpreter can't handle extra block Aug 22, 2023
@seancribbs
Copy link

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
Copy link
Collaborator

daxpedda commented Jan 5, 2024

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
Copy link

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
Copy link
Collaborator

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
Copy link

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!

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

7 participants