Stream: git-wasmtime

Topic: wasmtime / issue #5229 wiggle: adapt Wiggle guest slices ...

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2022 at 21:41):

bjorn3 commented on issue #5229:

Would it make sense to have a method that returns &UnsafeCell<[T]> or &Cell<[T]>?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2022 at 23:03):

github-actions[bot] commented on issue #5229:

Subscribe to Label Action

cc @kubkon

<details>
This issue or pull request has been labeled: "wasi"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2022 at 23:12):

abrown commented on issue #5229:

Would it make sense to have a method that returns &UnsafeCell<[T]> or &Cell<[T]>?

I'm open to it; not sure if all of the functions exposed are necessary, though. Let's see what @alexcrichton and @pchickey think?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 09 2022 at 16:42):

abrown commented on issue #5229:

Otherwise I think it should be able to implement:

rust impl<T> Deref for UnsafeGuestSlice<'_, T> { type Target = [UnsafeCell<T>]; // ... }

which could actually be relatively convenient to work with.

Thinking about this more, I don't think accessing T through an UnsafeCell is right: UnsafeCell::get_mut would give us a &mut T and the documentation of that method claims that "this call borrows the UnsafeCell mutably (at compile-time) which guarantees that we possess the only reference," which is incorrect. It's incorrect because UnsafeGuestSlice is pointing to shared memory which could be modified at any time by any WASI thread, outside of the Rust borrowing world. @alexcrichton, what do you think?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 09 2022 at 16:57):

alexcrichton commented on issue #5229:

The get_mut method requires &mut self so we won't have to worry about that here since we'll never hand out &mut UnsafeCell<T>. By more-or-less handing out &[UnsafeCell<T>] we're basically saying "here's a 100% valid region of memory in terms of in-bounds, you have access to it, and it's all aligned correctly and whatnot -- but the contents of memory are ruled by other mechanisms"

The only way to interact with the contents of &UnsafeCell<T> is to have unsafe, which is the goal here, so I think it'll work out well. For unshared cases the wrapper methods to go to safe slices will work, and for shared cases it's clear that atomics/raw intrinsics/etc need to get used (and is something we'll document).

view this post on Zulip Wasmtime GitHub notifications bot (Nov 09 2022 at 18:15):

abrown commented on issue #5229:

The get_mut method requires &mut self

Ah, yeah, I see what you mean; this makes more sense. I'm not sure how/when this would be used, though; I guess this would be yet another way to modify a slice besides the unsafe UnsafeGuestSlice::get_mut? (I guess we could also expose an unsafe `UnsafeGuestSlice::get which would return a &[T]). If this isn't going to be used just yet, should we implement this in this PR or wait until it is needed?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 09 2022 at 18:15):

abrown edited a comment on issue #5229:

The get_mut method requires &mut self

Ah, yeah, I see what you mean; this makes more sense. I'm not sure how/when this would be used, though; I guess this would be yet another way to modify a slice besides the unsafe UnsafeGuestSlice::get_mut? (I guess we could also expose an unsafe UnsafeGuestSlice::get which would return a &[T]). If this isn't going to be used just yet, should we implement this in this PR or wait until it is needed?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 09 2022 at 18:16):

abrown edited a comment on issue #5229:

The get_mut method requires &mut self

Ah, yeah, I see what you mean; this makes more sense. I'm not sure how/when this would be used, though; I guess this would be yet another way to modify a slice besides the unsafe UnsafeGuestSlice::get_mut? (I guess for completeness we could also expose an unsafe UnsafeGuestSlice::get which would return a &[T]). If this isn't going to be used just yet, should we implement this in this PR or wait until it is needed?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 09 2022 at 19:09):

alexcrichton commented on issue #5229:

Oh I don't think the Deref impl is really all that important to have, it's just a "might as well" sort of thing. I think though having an interim UnsafeGuestSlice get constructed on the way to a GuestSlice{,Mut} will exercise it and make it useful. Only for niche cases will UnsafeGuestSlice be useful. Note though we could have UnsafeGuestSlice::copy_from_slice as a safe method which only fails due to active borrows.

Last updated: Nov 22 2024 at 16:03 UTC