Update to wasi 0.11 by Thomasdezeeuw · Pull Request #253 · rust-random/getrandom

Thomasdezeeuw · 2022-03-19T15:36:05Z

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.

newpavlov · 2022-03-25T22:22:12Z

Personally, I would prefer for wasi developers to fix the Error issue first before moving from v0.10.

josephlr · 2022-03-25T23:00:16Z

While I'd also prefer the wasi types to have the error information, it looks like wasi's libcprovides strerror_r which contains the errno information. So we might be able to get the debug info just from the call to libc, and handle things on wasi like we do on unix.

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>.

Thomasdezeeuw · 2022-03-26T13:14:40Z

I've changed over to use libc::stderrorand needed to force push to resolve the merge conflict.

josephlr · 2022-03-26T21:19:44Z

 use crate::Error;
 use core::num::NonZeroU32;
-use wasi::random_get;
+use wasi::wasi_snapshot_preview1::random_get;


Why not just use wasi::random_get?

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.

But it won't actually be zero if the function returns Err, so it seems fine.

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.

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.

newpavlov · 2022-03-26T21:27:00Z

Adding libc simply for error messages is not great... Especially considering that random_get errors are extremely unlikely and there are no random_get-specific error codes.

josephlr · 2022-03-26T21:34:44Z

Adding libc simply for error messages is not great... Especially considering that random_get errors are extremely unlikely and there are no random_get-specific error codes.

So do you think we should just not have error messages for wasi errors then? I would be fine with that.

newpavlov · 2022-03-26T21:46:38Z

Yes, before the wasi crate improves handling of errors, I think it's fine to simply do not have any error messages for WASI.

Thomasdezeeuw · 2022-03-28T10:49:46Z

So I should drop the libc dependency and remove the error message?

josephlr · 2022-03-29T03:59:11Z

That sounds great!

This does mean that we won't get an error message for the error type.

Thomasdezeeuw · 2022-03-29T11:48:43Z

@josephlr done.

Thomasdezeeuw · 2022-04-15T08:40:40Z

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>

* Update to wasi 0.11 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>. * Use libc::strerror to get the error message on wasi Since wasi v0.11 doesn't (yet) provided a way to create Errno, see <bytecodealliance/wasi-rs#64>. * Remove libc dependency for WASI This does mean that we won't get an error message for the error type.

* 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 rust-random#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>

josephlr requested changes Mar 25, 2022
View reviewed changes

Comment threadCargo.toml Outdated
Comment threadsrc/error.rs Outdated

Thomasdezeeuw added 2 commits March 26, 2022 14:13

Use libc::strerror to get the error message on wasi
3487d64
Since wasi v0.11 doesn't (yet) provided a way to create Errno, see <bytecodealliance/wasi-rs#64>.

josephlr reviewed Mar 26, 2022
View reviewed changes

Remove libc dependency for WASI
523b443
This does mean that we won't get an error message for the error type.

josephlr approved these changes Apr 15, 2022
View reviewed changes

josephlr merged commit 2d65a40 into rust-random:masterApr 15, 2022

Thomasdezeeuw deleted the wasi-v0.11 branch April 15, 2022 15:30

josephlr mentioned this pull request Oct 23, 2022
Cleanup wasm32-wasi target #306
Merged

Uh oh!

Conversation

Thomasdezeeuw commented Mar 19, 2022

Uh oh!

newpavlov commented Mar 25, 2022

Uh oh!

josephlr commented Mar 25, 2022

Uh oh!

Uh oh!

Uh oh!

Thomasdezeeuw commented Mar 26, 2022

Uh oh!

josephlrMar 26, 2022

Choose a reason for hiding this comment

Uh oh!

ThomasdezeeuwMar 26, 2022

Choose a reason for hiding this comment

Uh oh!

josephlrMar 26, 2022

Choose a reason for hiding this comment

Uh oh!

newpavlovMar 26, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomasdezeeuwMar 26, 2022

Choose a reason for hiding this comment

Uh oh!

newpavlov commented Mar 26, 2022

Uh oh!

josephlr commented Mar 26, 2022

Uh oh!

newpavlov commented Mar 26, 2022

Uh oh!

Thomasdezeeuw commented Mar 28, 2022

Uh oh!

josephlr commented Mar 29, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Thomasdezeeuw commented Mar 29, 2022

Uh oh!

Thomasdezeeuw commented Apr 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

newpavlovMar 26, 2022•
edited
Loading

josephlr commented Mar 29, 2022•
edited
Loading