koute / stdweb

A standard library for the client-side Web
Apache License 2.0
3.45k stars 177 forks source link

Mutable version of UnsafeTypedArray #360

Open daboross opened 5 years ago

daboross commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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?