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.

Sign up for GitHub

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

Jump to bottom

exports for wasi_snapshot_preview1::fd_write in memory64 has 32 bit pointers #3594

Open
yowl opened this issue Dec 9, 2021 · 3 comments
Open

exports for wasi_snapshot_preview1::fd_write in memory64 has 32 bit pointers #3594

yowl opened this issue Dec 9, 2021 · 3 comments
Labels
wasm-proposal:memory64 Issues with the implementation of the memory64 wasm proposal

Comments

@yowl
Copy link
Contributor

yowl commented Dec 9, 2021

Thanks for filing a bug report! Please fill out the TODOs below.

Note: if you want to report a security issue, please read our security policy!

Test Case

Wasm file uploaded

Steps to Reproduce

Run with wasmtime --wasm-features memory64

Expected Results

It runs

Actual Results

Fails with

Error: failed to run main module `C:\Users\ScottWaye\OneDrive\wasm64\1.wasm`

Caused by:
    0: failed to instantiate "C:\\Users\\ScottWaye\\OneDrive\\wasm64\\1.wasm"
    1: incompatible import type for `wasi_snapshot_preview1::fd_write`
    2: function types incompatible: expected func of type `(i32, i64, i64, i64) -> (i32)`, found func of type `(i32, i32, i32, i32) -> (i32)`

Versions and Environment

Wasmtime version or commit: wasmtime 0.31.0

Operating system: Win10

Architecture: x64?

Extra Info

Looks like the wasi export is using 32 bit pointers when for memory64 shouldn't it be 64 bit?

Anything else you'd like to add?
1.zip
@yowl yowl added the bug Incorrect behavior in the current implementation that needs fixing label Dec 9, 2021
@yowl
Copy link
Contributor Author

yowl commented Dec 9, 2021 •
edited

The error message is not clear and it could be that wasmtime is right and my import is wrong. Unfortuantely wabt's wasm2wat doesn't work for memory64 so not sure how to check that.

@alexcrichton
Copy link
Member

The support for memory64 is not complete and it's still experimental, this is one of the reasons why. Our WASI support currently does not work on memory64, not because it's a fundamental issue but rather because it's just not supported yet, it's something we need to implement and design.

@yowl
Copy link
Contributor Author

yowl commented Dec 13, 2021

Thanks, its not a big deal for me. I was just looking for a way to test my own 64 support!

@alexcrichton alexcrichton added wasm-proposal:memory64 Issues with the implementation of the memory64 wasm proposal and removed bug Incorrect behavior in the current implementation that needs fixing labels Dec 13, 2021
josephlr added a commit to rust-random/getrandom that referenced this issue Oct 23, 2022
@josephlr
Cleanup wasm32-wasi target
99077a3 
This change ensures that we only compile our WASI implementation for
32-bit targets. The interaction between the WASI proposal and the
memory64 proposal is not yet clear, [wasmtime does not yet support](
bytecodealliance/wasmtime#3594 (comment))
using WASI with memory64, and many of the interfaces use 32-bit values
for pointers.

This change also reduces the use of `unsafe` from the wasi
implementation. As noted in #253, changes to `Errno` mean that we can't
get the error message from the raw error code, but we can avoid using
unsafe when converting this code to a NonZeroU32. This handling also
makes WASI behave more like our other targets, which also manually check
that errno is non-zero.

Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr josephlr mentioned this issue Oct 23, 2022
Merged
josephlr added a commit to rust-random/getrandom that referenced this issue Oct 23, 2022
@josephlr
Cleanup wasm32-wasi target (#306)
ad08dd9 
* Cleanup wasm32-wasi target

This change ensures that we only compile our WASI implementation for
32-bit targets. The interaction between the WASI proposal and the
memory64 proposal is not yet clear, [wasmtime does not yet support](
bytecodealliance/wasmtime#3594 (comment))
using WASI with memory64, and many of the interfaces use 32-bit values
for pointers.

This change also reduces the use of `unsafe` from the wasi
implementation. As noted in #253, changes to `Errno` mean that we can't
get the error message from the raw error code, but we can avoid using
unsafe when converting this code to a NonZeroU32. This handling also
makes WASI behave more like our other targets, which also manually check
that errno is non-zero.

Signed-off-by: Joe Richey <joerichey@google.com>

* Disable default features for WASI crate

Similar to this crate, the `wasi` crate just uses a `std` feature to
implement `std::error::Errno`, which we don't use.

Signed-off-by: Joe Richey <joerichey@google.com>

Signed-off-by: Joe Richey <joerichey@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
wasm-proposal:memory64 Issues with the implementation of the memory64 wasm proposal
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@alexcrichton @yowl