-
Notifications
You must be signed in to change notification settings - Fork 194
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
Update to wasi 0.11 #253
Update to wasi 0.11 #253
Conversation
Personally, I would prefer for |
While I'd also prefer the This also seems to be what the standard library does. |
The main breaking change between v0.10 and v0.11 is that Error is removed in favour of Errno. Unfortunately we can't create an Errno from outside the wasi create so we're loosing some debug information for errors. I've opened an issue to add back such a constructor, see <bytecodealliance/wasi-rs#64>.
Since wasi v0.11 doesn't (yet) provided a way to create Errno, see <bytecodealliance/wasi-rs#64>.
I've changed over to use |
@@ -9,15 +9,11 @@ | |||
//! Implementation for WASI | |||
use crate::Error; | |||
use core::num::NonZeroU32; | |||
use wasi::random_get; | |||
use wasi::wasi_snapshot_preview1::random_get; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just use wasi::random_get
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the new Errno
from wasi, which replaced Error
, can actually hold 0. Errno::as_raw
returns u16
, which may be zero, thus invalidating the previous code calling NonZeroU32::new_unchecked
. However handling zero ourselves ensures we can call NonZeroU32::new_unchecked
again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it won't actually be zero if the function returns Err
, so it seems fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would mean that soundness of our code relies on an implementation detail of wasi
. I am not sure we can rely on it, even though it's a very reasonable assumption.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would mean that soundness of our code relies on an implementation detail of
wasi
. I am not sure we can rely on it, even though it's a very reasonable assumption.
This is the exact reason I decided to switch.
Adding |
So do you think we should just not have error messages for |
Yes, before the |
So I should drop the libc dependency and remove the error message? |
That sounds great! |
This does mean that we won't get an error message for the error type.
@josephlr done. |
Any update on this? |
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 <[email protected]>
* 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 <[email protected]> * 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 <[email protected]> Signed-off-by: Joe Richey <[email protected]>
The main breaking change between v0.10 and v0.11 is that Error is
removed in favour of Errno. Unfortunately we can't create an Errno from
outside the wasi create so we're loosing some debug information for
errors.
I've opened an issue to add back such a constructor, see
bytecodealliance/wasi-rs#64.