rusty_v8
rusty_v8 copied to clipboard
Using the deref value of a shared BackingStore might be undefined behavior
In the C++ v8 API, BackingStore
simply exposes a pointer to the chunk of memory that belongs to the backing store and leaves it to the user to use it responsibly. In order to offer a safe API, rusty_v8
has BackingStore
deref to a slice of Cell<u8>
. But although this seems to be safe if the backing store isn't shared, for stores corresponding to SharedArrayBuffer
s, using this deref value seems to be undefined behavior.
The Javascript memory model allows two types of orderings on (Shared
)ArrayBuffer
s: Unordered
and SeqCst
. The former is the ordering used with raw reads or writes, while the latter is the one you get by using the methods in the Atomics
global. Although I'm not 100% sure that the SeqCst
ordering is equivalent to using C++20/Rust atomics with Ordering::SeqCst
, that seems likely to be the case.
However, the unordered
ordering provides no guarantees other than a lack of UB – my understanding is that unordered
access don't even have to be atomic. Since raw memory accesses in C++ or Rust would cause UB, it might be safe to use atomics with Ordering::Relaxed
, since it's always okay to use a stronger ordering than needed – but it wasn't clear to me that that wouldn't interact with the JIT code in unexpected ways. However, I found this v8 issue which fixed a case of UB in accessing a SAB's memory by replacing it with relaxed atomics, so using that is presumably fine.
I propose to fix this by getting rid of the Deref
implementation and instead add the following method:
impl BackingStore {
pub fn as_slice<T: BackingStoreElement>(&self) -> BackingStoreSlice<'_, T> {
todo!()
}
}
enum BackingStoreSlice<'a, T: BackingStoreElement> {
SingleThreaded(&'a [Cell<T>]),
Shared(&'a [T::Atomic]),
}
trait BackingStoreElement {
type Atomic;
}
impl BackingStoreElement for u8 {
type Atomic = AtomicU8;
}
impl BackingStoreElement for i8 {
type Atomic = AtomicI8;
}
impl BackingStoreElement for u16 {
type Atomic = AtomicU16;
}
// ...
This API wouldn't force users to choose either Ordering::Relaxed
or Ordering::SeqCst
, but my understanding is that using other orderings might cause unexpected results but wouldn't be UB no matter what the JIT does.
cc @piscisaureus
I found another case where the Deref
of a BackingStore
might be UB, even in the non-shared case: for length-zero BackingStore
s, it seems like the pointer returned by bs.data()
is always the null pointer – and creating a slice from the null pointer is UB, even for empty slices. This can be fixed ahead of the rest of this issue by creating a slice from NonNull::dangling()
in those cases.