Skip to content

Commit 9f81f22

Browse files
authored
Inch towards making tables safer (#11212)
This is a small step towards #11179 which starts off by making `vm::Instance::get_defined_table` a safe function. That required updating one location which required both a `&mut Table` and a `&mut GcStore` at the same time with a new helper on the store. More of these sorts of helpers are likely going to be necessary but IMO it's a small price to pay for safety.
1 parent ab76f64 commit 9f81f22

File tree

3 files changed

+25
-13
lines changed

3 files changed

+25
-13
lines changed

crates/wasmtime/src/runtime/store.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,6 +1296,20 @@ impl StoreOpaque {
12961296
self.instances[id].handle.get_mut()
12971297
}
12981298

1299+
/// Pair of `Self::gc_store_mut` and `Self::instance_mut`
1300+
pub fn gc_store_and_instance_mut(
1301+
&mut self,
1302+
id: InstanceId,
1303+
) -> Result<(&mut GcStore, Pin<&mut vm::Instance>)> {
1304+
// Fill in `self.gc_store`, then proceed below to the point where we
1305+
// convince the borrow checker that we're accessing disjoint fields.
1306+
self.gc_store_mut()?;
1307+
Ok((
1308+
self.gc_store.as_mut().unwrap(),
1309+
self.instances[id].handle.get_mut(),
1310+
))
1311+
}
1312+
12991313
/// Get all instances (ignoring dummy instances) within this store.
13001314
pub fn all_instances<'a>(&'a mut self) -> impl ExactSizeIterator<Item = Instance> + 'a {
13011315
let instances = self

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,10 +1263,8 @@ impl Instance {
12631263
}
12641264

12651265
/// Get a locally-defined table.
1266-
pub(crate) fn get_defined_table(self: Pin<&mut Self>, index: DefinedTableIndex) -> *mut Table {
1267-
// SAFETY: the `unsafe` here is projecting from `*mut (A, B)` to
1268-
// `*mut A`, which should be a safe operation to do.
1269-
unsafe { &raw mut (*self.tables_mut().get_raw_mut(index).unwrap()).1 }
1266+
pub(crate) fn get_defined_table(self: Pin<&mut Self>, index: DefinedTableIndex) -> &mut Table {
1267+
&mut self.tables_mut()[index].1
12701268
}
12711269

12721270
pub(crate) fn with_defined_table_index_and_instance<R>(

crates/wasmtime/src/runtime/vm/instance/allocator.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -613,31 +613,31 @@ fn initialize_tables(
613613
.expect("const expression should be valid")
614614
};
615615
let idx = module.table_index(table);
616-
let table = unsafe {
617-
store
618-
.instance_mut(context.instance)
619-
.get_defined_table(table)
620-
.as_mut()
621-
.unwrap()
622-
};
623616
match module.tables[idx].ref_type.heap_type.top() {
624617
WasmHeapTopType::Extern => {
618+
let (gc_store, instance) =
619+
store.gc_store_and_instance_mut(context.instance)?;
620+
let table = instance.get_defined_table(table);
625621
let gc_ref = VMGcRef::from_raw_u32(raw.get_externref());
626-
let gc_store = store.gc_store_mut()?;
627622
let items = (0..table.size())
628623
.map(|_| gc_ref.as_ref().map(|r| gc_store.clone_gc_ref(r)));
629624
table.init_gc_refs(0, items)?;
630625
}
631626

632627
WasmHeapTopType::Any => {
628+
let (gc_store, instance) =
629+
store.gc_store_and_instance_mut(context.instance)?;
630+
let table = instance.get_defined_table(table);
633631
let gc_ref = VMGcRef::from_raw_u32(raw.get_anyref());
634-
let gc_store = store.gc_store_mut()?;
635632
let items = (0..table.size())
636633
.map(|_| gc_ref.as_ref().map(|r| gc_store.clone_gc_ref(r)));
637634
table.init_gc_refs(0, items)?;
638635
}
639636

640637
WasmHeapTopType::Func => {
638+
let table = store
639+
.instance_mut(context.instance)
640+
.get_defined_table(table);
641641
let funcref = NonNull::new(raw.get_funcref().cast::<VMFuncRef>());
642642
let items = (0..table.size()).map(|_| funcref);
643643
table.init_func(0, items)?;

0 commit comments

Comments
 (0)