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.

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

Mutable version of UnsafeTypedArray #360

Open
daboross opened this issue Aug 13, 2019 · 3 comments
Open

Mutable version of UnsafeTypedArray #360

daboross opened this issue Aug 13, 2019 · 3 comments

Comments

@daboross
Copy link
Contributor

daboross commented Aug 13, 2019

UnsafeTypedArray takes &[T] in the constructor, and stores that internally. It's nice for reading values efficiently, but as I understand it rustc will assume that values behind a reference are never mutated, and thus it's unsound to mutate values using an UnsafeTypedArray.

Could a similar structure be made which exposes a mutable view of an array to JS? For manual initialization code, like

let mut arr = [0u32; 3];
let mut wrapped = MutableUnsafeTypedArray::new(&mut arr);
js! {
    let vals = @{wrapped};
    vals[0] = ...;
    vals[1] = ...;
    vals[2] = ...;
};
arr
@koute
Copy link
Owner

koute commented Aug 13, 2019

Yep, you're right, although I'm not 100% sure how to make a a mutable UnsafeTypedArrayMut which would be guaranteed to never trigger (even in theory) any UB since Rust's unsafe guidelines are not yet fully baked.

The current UnsafeTypedArray wrapper isn't really anything special; here's how it's serialized:

macro_rules! impl_for_unsafe_typed_array {
    ($ty:ty, $kind:expr) => {
        impl< 'r > JsSerialize for UnsafeTypedArray< 'r, $ty > {
            #[doc(hidden)]
            #[inline]
            fn _into_js< 'a >( &'a self ) -> SerializedValue< 'a > {
                SerializedUntaggedUnsafeTypedArray {
                    pointer: self.0.as_ptr() as u32 / mem::size_of::< $ty >() as u32,
                    length: self.0.len() as u32,
                    kind: $kind
                }.into()
            }
        }
                            
        __js_serializable_boilerplate!( impl< 'a > for UnsafeTypedArray< 'a, $ty > );
    }
}

So basically it just pushes raw pointers to the JS side. I wonder, would using as_mut_ptr instead of as_ptr in the mutable version would make it completely UB-free in face of modification from the JS side?

@daboross
Copy link
Contributor Author

daboross commented Aug 14, 2019

Ah - that seems like a pretty reasonable implementation.

Since JsSerialize takes &self, I think we might need to do more than just store an &'a mut [T] or use as_mut_ptr. I'm 90% sure the compiler will look at that &self, and if there are no raw pointers or UnsafeCells stored within, it'll assume that no mutation takes place.

What if we defined a type like this, though?

pub struct MutableUnsafeTypedArray(*mut [T], PhantomData<(&'a mut [T])>)

Then we would turn the reference into a *mut pointer in the constructor, and the compiler would no longer make that assumption. I don't have a full grasp of undefined behavior, but I think we're allowed to mutate things pointed to by pointers as long as no two &mut references exist at the same time. WASM and JS being single threaded should prevent abuse from this.

@daboross
Copy link
Contributor Author

I've been reading up a bit more. While not everything makes since, I think if there's a raw pointer stored in the data structure it should be sound.

Like you said, there are a lot of rules that aren't defined. But https://doc.rust-lang.org/nomicon/aliasing.html mentions that raw pointers have no aliasing requirement, and I think that if we create one then even taking a reference to it should be fine.

I'd be wary of just using as_mut_ptr in the method, since the compiler could still look at the function signature and see that it takes approximately &&[T]. It would be similar to modifying the contents of the array directly, without UnsafeCell, just using as_mut_ptr as an implementation detail?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants