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 panics at local.tee instruction #2969

Closed
ranile opened this issue Jun 28, 2022 · 4 comments
Closed

wasm-bindgen panics at local.tee instruction #2969

ranile opened this issue Jun 28, 2022 · 4 comments
Labels

Comments

@ranile
Copy link
Collaborator

ranile commented Jun 28, 2022

Describe the Bug

When LocalTee is encountered, wasm-bindgen panics. This happens because this instruction is not handled proeprly. The panic occurs at

// All other instructions shouldn't be used by our various
// descriptor functions. LLVM optimizations may mean that some
// of the above instructions aren't actually needed either, but
// the above instructions have empirically been required when
// executing our own test suite in wasm-bindgen.
//
// Note that LLVM may change over time to generate new
// instructions in debug mode, and we'll have to react to those
// sorts of changes as they arise.
s => panic!("unknown instruction {:?}", s),

Unlike what the comment suggests, this also occurs in release mode

Steps to Reproduce

  1. Git clone https://github.com/hamza1311/nushell-demo/
  2. cargo build --target wasm32-wasi --release
  3. wasm-bindgen target/wasm32-wasi/release/nushell-demo-2.wasm --out-dir dist

Backtrace

〉RUST_BACKTRACE=full wasm-bindgen target/wasm32-wasi/release/nushell-demo-2.wasm --out-dir dist
thread 'main' panicked at 'unknown instruction LocalTee(LocalTee { local: Id { idx: 67501 } })', /home/hamza/.cargo/registry/src/github.com-1ecc6299db9ec823/wasm-bindgen-wasm-interpreter-0.2.80/src/lib.rs:363:18
stack backtrace:
   0:     0x55a1cfa274ed - std::backtrace_rs::backtrace::libunwind::trace::hee598835bc88d35b
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x55a1cfa274ed - std::backtrace_rs::backtrace::trace_unsynchronized::h9cdc730ba5cf5d72
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x55a1cfa274ed - std::sys_common::backtrace::_print_fmt::h75aeaf7ed30e43fa
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys_common/backtrace.rs:66:5
   3:     0x55a1cfa274ed - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h606862f787600875
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys_common/backtrace.rs:45:22
   4:     0x55a1cfa4c15c - core::fmt::write::he803f0f418caf762
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/fmt/mod.rs:1190:17
   5:     0x55a1cfa23848 - std::io::Write::write_fmt::h70bc45872f37e7bb
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/io/mod.rs:1657:15
   6:     0x55a1cfa29577 - std::sys_common::backtrace::_print::h64d038cf8ac3e13e
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys_common/backtrace.rs:48:5
   7:     0x55a1cfa29577 - std::sys_common::backtrace::print::h359300b4a7fccf65
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys_common/backtrace.rs:35:9
   8:     0x55a1cfa29577 - std::panicking::default_hook::{{closure}}::hf51be35e2f510149
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:295:22
   9:     0x55a1cfa29240 - std::panicking::default_hook::h03ca0f22e1d2d25e
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:314:9
  10:     0x55a1cfa29cc9 - std::panicking::rust_panic_with_hook::h3b7380e99b825b63
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:698:17
  11:     0x55a1cfa299b7 - std::panicking::begin_panic_handler::{{closure}}::h8e849d0710154ce0
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:588:13
  12:     0x55a1cfa279b4 - std::sys_common::backtrace::__rust_end_short_backtrace::hedcdaddbd4c46cc5
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys_common/backtrace.rs:138:18
  13:     0x55a1cfa296c9 - rust_begin_unwind
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:584:5
  14:     0x55a1cf797073 - core::panicking::panic_fmt::he1bbc7336d49a357
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/panicking.rs:143:14
  15:     0x55a1cf8efc04 - wasm_bindgen_wasm_interpreter::Interpreter::call::h3bc948dddbc23ce1
  16:     0x55a1cf8ef95d - wasm_bindgen_wasm_interpreter::Interpreter::call::h3bc948dddbc23ce1
  17:     0x55a1cf8ef95d - wasm_bindgen_wasm_interpreter::Interpreter::call::h3bc948dddbc23ce1
  18:     0x55a1cf8ef95d - wasm_bindgen_wasm_interpreter::Interpreter::call::h3bc948dddbc23ce1
  19:     0x55a1cf8eec97 - wasm_bindgen_wasm_interpreter::Interpreter::interpret_descriptor::ha7133e2d18b55d4b
  20:     0x55a1cf7f0a9d - wasm_bindgen_cli_support::descriptors::execute::hf2bff1d3d9d87f99
  21:     0x55a1cf7af31a - wasm_bindgen_cli_support::Bindgen::generate_output::haf96afc2dd8aca41
  22:     0x55a1cf79e5ce - wasm_bindgen_cli_support::Bindgen::generate::h7815928f94a5c31f
  23:     0x55a1cf79aa60 - wasm_bindgen::main::hfcc8dcf29010eb17
  24:     0x55a1cf7987c3 - std::sys_common::backtrace::__rust_begin_short_backtrace::h34fa2df626bd2e0d
  25:     0x55a1cf798d39 - std::rt::lang_start::{{closure}}::h2bf96a5883d86a41
  26:     0x55a1cfa26b41 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hb7014f43484a8b4e
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/ops/function.rs:259:13
  27:     0x55a1cfa26b41 - std::panicking::try::do_call::h7bc9dc436daeb8c7
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:492:40
  28:     0x55a1cfa26b41 - std::panicking::try::h653d68a27ff5f175
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:456:19
  29:     0x55a1cfa26b41 - std::panic::catch_unwind::h9d739f9f59895e68
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panic.rs:137:14
  30:     0x55a1cfa26b41 - std::rt::lang_start_internal::{{closure}}::hf006f2bc7ce22bbe
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/rt.rs:128:48
  31:     0x55a1cfa26b41 - std::panicking::try::do_call::hfb39d6df61a2e69f
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:492:40
  32:     0x55a1cfa26b41 - std::panicking::try::h13e2d225134958ac
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:456:19
  33:     0x55a1cfa26b41 - std::panic::catch_unwind::h3bd49b5a5dfb1a50
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panic.rs:137:14
  34:     0x55a1cfa26b41 - std::rt::lang_start_internal::h2ba92edce36c035e
                               at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/rt.rs:128:20
  35:     0x55a1cf79af52 - main
  36:     0x7fdd2b429290 - <unknown>
  37:     0x7fdd2b42934a - __libc_start_main
  38:     0x55a1cf7978a5 - _start
  39:                0x0 - <unknown>
@ranile ranile added the bug label Jun 28, 2022
@Liamolucko
Copy link
Collaborator

It looks like the WASI target is injecting some extra runtime stuff around the function wasm-bindgen is trying to call, which contains that instruction:

(func $__wbindgen_describe___wbg_self_dc2f808420e70e36.command_export (type 1)
    call $__wasm_call_ctors
    call $__wbindgen_describe___wbg_self_dc2f808420e70e36
    call $__wasm_call_dtors)

Liamolucko added a commit to Liamolucko/wasm-bindgen that referenced this issue Aug 13, 2022
Fixes rustwasm#2969

This changes `wasm-bindgen-wasm-interpreter` to ignore a function rather than panicking if it contains an unsupported instruction. This works around some runtime glue that gets added to our descriptor functions on the WASI target by more-or-less ignoring them.

I also put some work into making sure a helpful error message is given if the actual describe function contains unsupported instructions, by keeping track of the instructions that caused us to skip and including them in the error message if the descriptor is invalid.
@tv42
Copy link

tv42 commented Jan 7, 2023

It seems I can trigger this without WASI:

#[wasm_bindgen(module = "...")]
extern "C" {
        pub type Foo;

        #[wasm_bindgen(method)]
        pub fn bar(this: &Foo, callback: &Closure<dyn FnMut()>);
}

...
	let my_closure = Closure::new(move || {
                // nothing
        });
        my_foo.callback(&my_closure);
        my_foo.this_call_uses_the_callback().await;
  thread 'main' panicked at 'unknown instruction LocalTee(LocalTee { local: Id { idx: 810 } })', /blah/.cargo/registry/src/github.com-1ecc6299db9ec823/wasm-bindgen-wasm-interpreter-0.2.83/src/lib.rs:363:18

Sorry, I don't have a minimal reproducer at this time, that's copy-pasted from a larger code base.

I think I need a heap-allocated closure because it only gets used in the very next function call, which is async. My current workaround is to put both of those calls in a single async JS helper function, and then a stack-allocated closure works...

Liamolucko added a commit to Liamolucko/wasm-bindgen that referenced this issue Jan 12, 2023
This doesn't solve rustwasm#2969, since WASI uses plenty of instructions we don't support aside from `local.tee`, but it hopefully might solve [@tv42's issue](rustwasm#2969 (comment)).
@Liamolucko
Copy link
Collaborator

@tv42 I've opened #3232 which adds support for the local.tee instruction. It won't solve the issue with WASI, since that also uses lots of other instructions we don't support, but let me know if it solves your issue!

alexcrichton pushed a commit that referenced this issue Jan 12, 2023
This doesn't solve #2969, since WASI uses plenty of instructions we don't support aside from `local.tee`, but it hopefully might solve [@tv42's issue](#2969 (comment)).
@daxpedda
Copy link
Collaborator

This is fixed by #3232, WASI support can be discussed in #3421.

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

Successfully merging a pull request may close this issue.

4 participants