Skip to content

Commit 3f53021

Browse files
authored
Generalize VMGcObjectDataMut for immutable GC object accesses too (bytecodealliance#10462)
* Generalize `VMGcObjectDataMut` for immutable GC object accesses too This makes it generic over a `T` and then shared+immutable accesses are bound by `T: AsRef<[u8]>` and exclusive+mutable accesses are bound by `T: AsMut<[u8]>`. This allows reusing these accessors in more places in the future, and means there are fewer places to bounds check accesses and remember to do little-endian conversion and all that. * Make `VMGcObjectData` a fancy unsized newtype * fix no-gc builds
1 parent 00a1780 commit 3f53021

File tree

7 files changed

+82
-76
lines changed

7 files changed

+82
-76
lines changed

crates/wasmtime/src/runtime/vm/gc.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,8 @@ impl GcStore {
220220
/// Get the data for the given object reference.
221221
///
222222
/// Panics when the structref and its size is out of the GC heap bounds.
223-
pub fn gc_object_data(&mut self, gc_ref: &VMGcRef) -> VMGcObjectDataMut<'_> {
224-
self.gc_heap.gc_object_data(gc_ref)
223+
pub fn gc_object_data(&mut self, gc_ref: &VMGcRef) -> &mut VMGcObjectData {
224+
self.gc_heap.gc_object_data_mut(gc_ref)
225225
}
226226

227227
/// Get the object datas for the given pair of object references.
@@ -231,7 +231,7 @@ impl GcStore {
231231
&mut self,
232232
a: &VMGcRef,
233233
b: &VMGcRef,
234-
) -> (VMGcObjectDataMut<'_>, VMGcObjectDataMut<'_>) {
234+
) -> (&mut VMGcObjectData, &mut VMGcObjectData) {
235235
assert_ne!(a, b);
236236
self.gc_heap.gc_object_data_pair(a, b)
237237
}
Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
11
//! Dummy GC types for when the `gc` cargo feature is disabled.
2-
//!
3-
//! To reduce `#[cfg(...)]`s, this provides all the same methods as the real
4-
//! `VMExternRef` except for constructors.
52
63
#![allow(missing_docs)]
74

@@ -11,13 +8,19 @@ pub enum VMStructRef {}
118

129
pub enum VMArrayRef {}
1310

14-
pub struct VMGcObjectDataMut<'a> {
11+
pub struct VMGcObjectData {
1512
_inner: VMStructRef,
16-
_phantom: core::marker::PhantomData<&'a mut ()>,
13+
_phantom: core::marker::PhantomData<[u8]>,
1714
}
1815

19-
impl VMGcObjectDataMut<'_> {
20-
pub fn new(_data: &mut [u8]) -> Self {
16+
impl<'a> From<&'a [u8]> for &'a VMGcObjectData {
17+
fn from(_: &'a [u8]) -> Self {
18+
unreachable!()
19+
}
20+
}
21+
22+
impl<'a> From<&'a mut [u8]> for &'a mut VMGcObjectData {
23+
fn from(_: &'a mut [u8]) -> Self {
2124
unreachable!()
2225
}
2326
}

crates/wasmtime/src/runtime/vm/gc/enabled/arrayref.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ impl VMArrayRef {
202202
debug_assert!(val._matches_ty(&store, &ty.unpack())?);
203203

204204
let offset = layout.elem_offset(index);
205-
let mut data = store.unwrap_gc_store_mut().gc_object_data(self.as_gc_ref());
205+
let data = store.unwrap_gc_store_mut().gc_object_data(self.as_gc_ref());
206206
match val {
207207
Val::I32(i) if ty.is_i8() => data.write_i8(offset, truncate_i32_to_i8(i)),
208208
Val::I32(i) if ty.is_i16() => data.write_i16(offset, truncate_i32_to_i16(i)),
@@ -232,7 +232,7 @@ impl VMArrayRef {
232232
None => None,
233233
};
234234
store.gc_store_mut()?.write_gc_ref(&mut gc_ref, e.as_ref());
235-
let mut data = store.gc_store_mut()?.gc_object_data(self.as_gc_ref());
235+
let data = store.gc_store_mut()?.gc_object_data(self.as_gc_ref());
236236
data.write_u32(offset, gc_ref.map_or(0, |r| r.as_raw_u32()));
237237
}
238238
Val::AnyRef(a) => {
@@ -243,7 +243,7 @@ impl VMArrayRef {
243243
None => None,
244244
};
245245
store.gc_store_mut()?.write_gc_ref(&mut gc_ref, a.as_ref());
246-
let mut data = store.gc_store_mut()?.gc_object_data(self.as_gc_ref());
246+
let data = store.gc_store_mut()?.gc_object_data(self.as_gc_ref());
247247
data.write_u32(offset, gc_ref.map_or(0, |r| r.as_raw_u32()));
248248
}
249249

crates/wasmtime/src/runtime/vm/gc/enabled/data.rs

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -60,41 +60,64 @@ impl PodValType<{ mem::size_of::<V128>() }> for V128 {
6060
/// for preserving the memory safety of indexed GC heaps in the face of (for
6161
/// example) collector bugs, but the latter is just a defensive technique to
6262
/// catch bugs early and prevent action at a distance as much as possible.
63-
pub struct VMGcObjectDataMut<'a> {
64-
data: &'a mut [u8],
63+
#[repr(transparent)]
64+
pub struct VMGcObjectData {
65+
data: [u8],
6566
}
6667

6768
macro_rules! impl_pod_methods {
68-
( $( $t:ty, $read:ident, $write:ident; )* ) => {
69+
( $( $T:ty, $read:ident, $write:ident; )* ) => {
6970
$(
7071
/// Read a `
71-
#[doc = stringify!($t)]
72+
#[doc = stringify!($T)]
7273
/// ` field this object.
7374
///
7475
/// Panics on out-of-bounds accesses.
7576
#[inline]
76-
pub fn $read(&self, offset: u32) -> $t {
77-
self.read_pod::<{ mem::size_of::<$t>() }, $t>(offset)
77+
pub fn $read(&self, offset: u32) -> $T
78+
{
79+
self.read_pod::<{ mem::size_of::<$T>() }, $T>(offset)
7880
}
7981

8082
/// Write a `
81-
#[doc = stringify!($t)]
83+
#[doc = stringify!($T)]
8284
/// ` into this object.
8385
///
8486
/// Panics on out-of-bounds accesses.
8587
#[inline]
86-
pub fn $write(&mut self, offset: u32, val: $t) {
87-
self.write_pod::<{ mem::size_of::<$t>() }, $t>(offset, val);
88+
pub fn $write(&mut self, offset: u32, val: $T)
89+
{
90+
self.write_pod::<{ mem::size_of::<$T>() }, $T>(offset, val);
8891
}
8992
)*
9093
};
9194
}
9295

93-
impl<'a> VMGcObjectDataMut<'a> {
94-
/// Construct a `VMStructDataMut` from the given slice of bytes.
96+
impl<'a> From<&'a [u8]> for &'a VMGcObjectData {
9597
#[inline]
96-
pub fn new(data: &'a mut [u8]) -> Self {
97-
Self { data }
98+
fn from(data: &'a [u8]) -> Self {
99+
&VMGcObjectData::from_slice(data)
100+
}
101+
}
102+
103+
impl<'a> From<&'a mut [u8]> for &'a mut VMGcObjectData {
104+
#[inline]
105+
fn from(data: &'a mut [u8]) -> Self {
106+
VMGcObjectData::from_slice_mut(data)
107+
}
108+
}
109+
110+
impl VMGcObjectData {
111+
/// Construct a `VMGcObjectData` from the given slice of bytes.
112+
#[inline]
113+
pub fn from_slice(data: &[u8]) -> &Self {
114+
unsafe { mem::transmute(data) }
115+
}
116+
117+
/// Construct a `VMGcObjectData` from the given slice of bytes.
118+
#[inline]
119+
pub fn from_slice_mut(data: &mut [u8]) -> &mut Self {
120+
unsafe { mem::transmute(data) }
98121
}
99122

100123
/// Read a POD field out of this object.
@@ -133,14 +156,17 @@ impl<'a> VMGcObjectDataMut<'a> {
133156
Some(into) => into,
134157
None => panic!(
135158
"out of bounds field! field range = {offset:#x}..{end:#x}; object len = {:#x}",
136-
self.data.len(),
159+
self.data.as_mut().len(),
137160
),
138161
};
139162
val.write_le(into.try_into().unwrap());
140163
}
141164

142165
/// Get a slice of this object's data.
143166
///
167+
/// Note that GC data is always stored in little-endian order, and this
168+
/// method does not do any conversions to/from host endianness for you.
169+
///
144170
/// Panics on out-of-bounds accesses.
145171
#[inline]
146172
pub fn slice(&self, offset: u32, len: u32) -> &[u8] {
@@ -152,6 +178,9 @@ impl<'a> VMGcObjectDataMut<'a> {
152178

153179
/// Get a mutable slice of this object's data.
154180
///
181+
/// Note that GC data is always stored in little-endian order, and this
182+
/// method does not do any conversions to/from host endianness for you.
183+
///
155184
/// Panics on out-of-bounds accesses.
156185
#[inline]
157186
pub fn slice_mut(&mut self, offset: u32, len: u32) -> &mut [u8] {
@@ -163,6 +192,9 @@ impl<'a> VMGcObjectDataMut<'a> {
163192

164193
/// Copy the given slice into this object's data at the given offset.
165194
///
195+
/// Note that GC data is always stored in little-endian order, and this
196+
/// method does not do any conversions to/from host endianness for you.
197+
///
166198
/// Panics on out-of-bounds accesses.
167199
#[inline]
168200
pub fn copy_from_slice(&mut self, offset: u32, src: &[u8]) {

crates/wasmtime/src/runtime/vm/gc/enabled/drc.rs

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
//! <https://openresearch-repository.anu.edu.au/bitstream/1885/42030/2/hon-thesis.pdf>
4343
4444
use super::free_list::FreeList;
45-
use super::{VMArrayRef, VMGcObjectDataMut, VMStructRef};
45+
use super::{VMArrayRef, VMStructRef};
4646
use crate::hash_set::HashSet;
4747
use crate::prelude::*;
4848
use crate::runtime::vm::{
@@ -661,43 +661,6 @@ unsafe impl GcHeap for DrcHeap {
661661
self.dealloc(structref.into());
662662
}
663663

664-
fn gc_object_data(&mut self, gc_ref: &VMGcRef) -> VMGcObjectDataMut<'_> {
665-
let range = self.object_range(gc_ref);
666-
let data = &mut self.heap_slice_mut()[range];
667-
VMGcObjectDataMut::new(data)
668-
}
669-
670-
fn gc_object_data_pair(
671-
&mut self,
672-
a: &VMGcRef,
673-
b: &VMGcRef,
674-
) -> (VMGcObjectDataMut<'_>, VMGcObjectDataMut<'_>) {
675-
assert_ne!(a, b);
676-
677-
let a_range = self.object_range(a);
678-
let b_range = self.object_range(b);
679-
680-
// Assert that the two objects do not overlap.
681-
assert!(a_range.start <= a_range.end);
682-
assert!(b_range.start <= b_range.end);
683-
assert!(a_range.end <= b_range.start || b_range.end <= a_range.start);
684-
685-
let (a_data, b_data) = if a_range.start < b_range.start {
686-
let (a_half, b_half) = self.heap_slice_mut().split_at_mut(b_range.start);
687-
let b_len = b_range.end - b_range.start;
688-
(&mut a_half[a_range], &mut b_half[..b_len])
689-
} else {
690-
let (b_half, a_half) = self.heap_slice_mut().split_at_mut(a_range.start);
691-
let a_len = a_range.end - a_range.start;
692-
(&mut a_half[..a_len], &mut b_half[b_range])
693-
};
694-
695-
(
696-
VMGcObjectDataMut::new(a_data),
697-
VMGcObjectDataMut::new(b_data),
698-
)
699-
}
700-
701664
fn alloc_uninit_array(
702665
&mut self,
703666
ty: VMSharedTypeIndex,

crates/wasmtime/src/runtime/vm/gc/enabled/structref.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ impl VMStructRef {
196196
debug_assert!(val._matches_ty(&store, &ty.unpack())?);
197197

198198
let offset = layout.fields[field].offset;
199-
let mut data = store.gc_store_mut()?.gc_object_data(self.as_gc_ref());
199+
let data = store.gc_store_mut()?.gc_object_data(self.as_gc_ref());
200200
match val {
201201
Val::I32(i) if ty.is_i8() => data.write_i8(offset, truncate_i32_to_i8(i)),
202202
Val::I32(i) if ty.is_i16() => data.write_i16(offset, truncate_i32_to_i16(i)),
@@ -226,7 +226,7 @@ impl VMStructRef {
226226
None => None,
227227
};
228228
store.gc_store_mut()?.write_gc_ref(&mut gc_ref, e.as_ref());
229-
let mut data = store.gc_store_mut()?.gc_object_data(self.as_gc_ref());
229+
let data = store.gc_store_mut()?.gc_object_data(self.as_gc_ref());
230230
data.write_u32(offset, gc_ref.map_or(0, |r| r.as_raw_u32()));
231231
}
232232
Val::AnyRef(a) => {
@@ -237,7 +237,7 @@ impl VMStructRef {
237237
None => None,
238238
};
239239
store.gc_store_mut()?.write_gc_ref(&mut gc_ref, a.as_ref());
240-
let mut data = store.gc_store_mut()?.gc_object_data(self.as_gc_ref());
240+
let data = store.gc_store_mut()?.gc_object_data(self.as_gc_ref());
241241
data.write_u32(offset, gc_ref.map_or(0, |r| r.as_raw_u32()));
242242
}
243243

crates/wasmtime/src/runtime/vm/gc/gc_runtime.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
use crate::prelude::*;
44
use crate::runtime::vm::{
55
ExternRefHostDataId, ExternRefHostDataTable, GcHeapObject, SendSyncPtr, TypedGcRef, VMArrayRef,
6-
VMExternRef, VMGcHeader, VMGcObjectDataMut, VMGcRef, VMStructRef,
6+
VMExternRef, VMGcHeader, VMGcObjectData, VMGcRef, VMStructRef,
77
};
88
use core::ptr::NonNull;
99
use core::{alloc::Layout, any::Any, marker, mem, num::NonZeroUsize, ops::Range, ptr};
@@ -457,10 +457,21 @@ pub unsafe trait GcHeap: 'static + Send + Sync {
457457
/// # Panics
458458
///
459459
/// Panics on out-of-bounds accesses or if the `gc_ref` is an `i31ref`.
460-
fn gc_object_data(&mut self, gc_ref: &VMGcRef) -> VMGcObjectDataMut<'_> {
460+
fn gc_object_data(&self, gc_ref: &VMGcRef) -> &VMGcObjectData {
461+
let range = self.object_range(gc_ref);
462+
let data = &self.heap_slice()[range];
463+
data.into()
464+
}
465+
466+
/// Get a mutable borrow of the given object's data.
467+
///
468+
/// # Panics
469+
///
470+
/// Panics on out-of-bounds accesses or if the `gc_ref` is an `i31ref`.
471+
fn gc_object_data_mut(&mut self, gc_ref: &VMGcRef) -> &mut VMGcObjectData {
461472
let range = self.object_range(gc_ref);
462473
let data = &mut self.heap_slice_mut()[range];
463-
VMGcObjectDataMut::new(data)
474+
data.into()
464475
}
465476

466477
/// Get a pair of mutable borrows of the given objects' data.
@@ -473,7 +484,7 @@ pub unsafe trait GcHeap: 'static + Send + Sync {
473484
&mut self,
474485
a: &VMGcRef,
475486
b: &VMGcRef,
476-
) -> (VMGcObjectDataMut<'_>, VMGcObjectDataMut<'_>) {
487+
) -> (&mut VMGcObjectData, &mut VMGcObjectData) {
477488
assert_ne!(a, b);
478489

479490
let a_range = self.object_range(a);
@@ -494,10 +505,7 @@ pub unsafe trait GcHeap: 'static + Send + Sync {
494505
(&mut a_half[..a_len], &mut b_half[b_range])
495506
};
496507

497-
(
498-
VMGcObjectDataMut::new(a_data),
499-
VMGcObjectDataMut::new(b_data),
500-
)
508+
(a_data.into(), b_data.into())
501509
}
502510
}
503511

0 commit comments

Comments
 (0)