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.
Why not just use wasi::random_get?
There was a problem hiding this comment.
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.
But it won't actually be zero if the function returns Err, so it seems fine.
There was a problem hiding this comment.
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.
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 <joerichey@google.com>
* 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>
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.