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

Wasmtime fails to run a .wasm built with rust-secp256k1, Wasmer can run it #2587

Closed
JeremyRubin opened this issue Jan 15, 2021 · 15 comments · Fixed by #2879
Closed

Wasmtime fails to run a .wasm built with rust-secp256k1, Wasmer can run it #2587

JeremyRubin opened this issue Jan 15, 2021 · 15 comments · Fixed by #2879
Labels
bug Incorrect behavior in the current implementation that needs fixing

Comments

@JeremyRubin
Copy link

This is an issue where wasmtime seems to read a const pointer as a function pointer, when it is actually a pointer to a constant. Wasmer does not have this issue, which suggests it's a defect in wasmtime as opposed to the library being used.

  • What are the steps to reproduce the issue?

Create a new crate bin with main.rs:

 use secp256k1::*;
 fn main() {
     println!("Hello, world! {:?}", Secp256k1::default());
 }

And Cargo.toml

 [dependencies]
 secp256k1 = "0.20.1"

Build in either release or debug mode for --target wasm32-wasi, and run the resulting binary using wasmtime.

  • What do you expect to happen? What does actually happen? Does it panic, and
    if so, with which assertion?

Running under Wasmer, it will print:

Hello, world! <secp256k1 context 0x1100b0, all capabilities>

Running under Wasmtime it will print:

Error: failed to run main module `target/wasm32-wasi/debug/test-wasm-secp.wasm`

Caused by:
    0: failed to instantiate "target/wasm32-wasi/debug/test-wasm-secp.wasm"
    1: command export 'rustsecp256k1_v0_4_0_context_no_precomp' is not a function
  • Which Wasmtime version / commit hash / branch are you using?

wasmtime 0.22.0

  • If relevant, can you include some extra information about your environment?
    (Rust version, operating system, architecture...)

rustc 1.48.0 (7eac88abb 2020-11-16)

Distributor ID: Ubuntu
Description: Ubuntu 20.04.1 LTS
Release: 20.04
Codename: focal

Architecture: x86_64
CPU op-mode(s): 32-bit, 64-bit
Byte Order: Little Endian
Address sizes: 43 bits physical, 48 bits virtual
CPU(s): 16
On-line CPU(s) list: 0-15
Thread(s) per core: 2
Core(s) per socket: 8
Socket(s): 1
NUMA node(s): 1
Vendor ID: AuthenticAMD
CPU family: 23
Model: 1
Model name: AMD Ryzen 7 1800X Eight-Core Processor

@JeremyRubin JeremyRubin added the bug Incorrect behavior in the current implementation that needs fixing label Jan 15, 2021
@bjorn3
Copy link
Contributor

bjorn3 commented Jan 15, 2021

command export 'rustsecp256k1_v0_4_0_context_no_precomp' is not a function

This is the reason. Wasmer doesn't care about this, but I believe the WASI spec doesn't allow it. (not sure though)

@JeremyRubin
Copy link
Author

@bjorn3
Copy link
Contributor

bjorn3 commented Jan 15, 2021

It does allow it, just not exported from the executable itself. It must be private.

@JeremyRubin
Copy link
Author

Interesting -- I might be missing something, but I am not re-exporting the symbol from the executable. I am only linking against a library which does, which should be static linked?

@JeremyRubin
Copy link
Author

@bjorn3 do you have a link to the spec you mentioned? I couldn't find a reference for it.

@bjorn3
Copy link
Contributor

bjorn3 commented Jan 16, 2021

It can be found at https://github.com/webassembly/wasi

@JeremyRubin
Copy link
Author

I couldn't find anything mentioning this restriction there -- do you have a more specific link or text section to point at?

@sunfishcode
Copy link
Member

The closest we have right now is this, though indeed there's no mention of what to do about unexpected exports.

Wasmtime's interpretation here comes from a desire to be conservative, since we expect the WASI ABI to expand and have a much richer set of things that you can do with exports in the future, and we don't want to get into a situation where a program has exports that it expects will just be ignored, where we would otherwise try to do something with them.

Are you using --export-dynamic, --export-all, or any directives to cause these symbols to be exported? I haven't tried it, but I wouldn't expect an extern "C" section to get a wasm export by default. If something like that is in play here, there may be things we can do here to help the toolchain not emit that export.

@JeremyRubin
Copy link
Author

Yeah I think everything is under extern "C" in the secp256k1 crate; I'm not setting any additional flags on my crate.

@fitzgen
Copy link
Member

fitzgen commented Jan 21, 2021

A bunch of wasm toolchains happen to export random extra crap. I understand our desire to be conservative here, but maybe emitting a warning, rather than refusing to execute the module, is a better balance?

@bjorn3
Copy link
Contributor

bjorn3 commented Jan 21, 2021

What about only disallowing exports of unknown __wasi_* functions and globals?

@sunfishcode
Copy link
Member

rustc is adding --export-dynamic. I've now submitted rust-lang/rust#81255 to fix that.

A warning might be ok, however I do think it's worth looking investigating this where it does come up. Some of the cases we've seen in the past of toolchains emitting extra exports were symptoms of bigger problems -- toolchains emitting wasm files which expected their exports to be used in certain ways which don't happen in a pure WASI execution environment.

Wasmtime does have a modest allowlist for exports to ignore, however most of them were added to work around the --export-dynamic issue. Hopefully perhaps after rust-lang/rust#81255 is in wide use we'll eventually be able to tidy that up.

@tschneidereit
Copy link
Member

We could also consider adding a command line flag / config parameter along the lines of --allow-unknown-exports or perhaps --unstable-allow-unknown-exports as an escape hatch: I do share the concern of a warning being ignored, and hard-to-fix de-facto standards being established that tie our hands later on.

@fitzgen
Copy link
Member

fitzgen commented Jan 22, 2021

--allow-unknown-exports

This sounds like a great solution to me 👍

peterhuene added a commit to peterhuene/wasmtime that referenced this issue May 6, 2021
This commit implements the `--allow-unknown-exports` option to the CLI run
command that will ignore unknown exports in a command module rather than
return an error.

Fixes bytecodealliance#2587.
cfallin pushed a commit to cfallin/wasmtime that referenced this issue Jun 21, 2021
This commit implements the `--allow-unknown-exports` option to the CLI run
command that will ignore unknown exports in a command module rather than
return an error.

Fixes bytecodealliance#2587.
ZauberNerd added a commit to ZauberNerd/tinygo that referenced this issue Mar 18, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
ZauberNerd added a commit to ZauberNerd/tinygo that referenced this issue Mar 18, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
ZauberNerd added a commit to ZauberNerd/tinygo that referenced this issue Mar 18, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
aykevl pushed a commit to tinygo-org/tinygo that referenced this issue Mar 19, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
deadprogram pushed a commit to tinygo-org/tinygo that referenced this issue Apr 7, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
deadprogram pushed a commit to tinygo-org/tinygo that referenced this issue Apr 11, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
ardnew pushed a commit to ardnew/tinygo that referenced this issue Jun 21, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
@pannous
Copy link

pannous commented Dec 12, 2022

could you make wasmtime --allow-unknown-exports default?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior in the current implementation that needs fixing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants