Skip to content

Always use fallible accesses of the GC heap #13320

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.

Sign up for GitHub

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

Open
alexcrichton wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Always use fallible accesses of the GC heap #13320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion crates/environ/src/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,10 +467,12 @@ impl BuiltinFunctionIndex {
// The final epoch represents a trap
(@get new_epoch u64) => (TrapSentinel::NegativeOne);

// Failure here indicates GC heap corruption.
(@get get_interned_func_ref pointer) => (TrapSentinel::NegativeOne);

// These libcalls can't trap
(@get ref_func pointer) => (return None);
(@get table_get_lazy_init_func_ref pointer) => (return None);
(@get get_interned_func_ref pointer) => (return None);
(@get intern_func_ref_for_gc_heap u64) => (return None);
(@get is_subtype u32) => (return None);
(@get ceil_f32 f32) => (return None);
Expand Down
3 changes: 0 additions & 3 deletions crates/wasmtime/src/runtime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ use core::marker;
use core::pin::Pin;
use core::task::{Context, Poll};

#[cfg(feature = "component-model-async")]
mod bug;

#[macro_use]
Expand Down Expand Up @@ -81,9 +80,7 @@ cfg_if::cfg_if! {
}
}

#[cfg(feature = "component-model-async")]
pub use bug::WasmtimeBug;
#[cfg(feature = "component-model-async")]
pub(crate) use bug::bail_bug;
pub use code_memory::CodeMemory;
#[cfg(feature = "debug")]
Expand Down
10 changes: 5 additions & 5 deletions crates/wasmtime/src/runtime/externals/global.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::prelude::*;
use crate::runtime::vm::{self, VMGlobalDefinition, VMGlobalKind, VMOpaqueContext};
use crate::{
AnyRef, AsContext, AsContextMut, ExnRef, ExternRef, Func, GlobalType, HeapType, Mutability,
Ref, RootedGcRefImpl, Val, ValType,
Ref, Val, ValType,
store::{AutoAssertNoGc, InstanceId, StoreId, StoreInstanceId, StoreOpaque},
trampoline::generate_global_export,
};
Expand Down Expand Up @@ -270,27 +270,27 @@ impl Global {
Some(e) => Some(e.try_gc_ref(&store)?.unchecked_copy()),
};
let new = new.as_ref();
definition.write_gc_ref(&mut store, new);
definition.write_gc_ref(&mut store, new)?;
}
Val::AnyRef(a) => {
let new = match a {
None => None,
Some(a) => Some(a.try_gc_ref(&store)?.unchecked_copy()),
};
let new = new.as_ref();
definition.write_gc_ref(&mut store, new);
definition.write_gc_ref(&mut store, new)?;
}
Val::ExnRef(e) => {
let new = match e {
None => None,
Some(e) => Some(e.try_gc_ref(&store)?.unchecked_copy()),
};
let new = new.as_ref();
definition.write_gc_ref(&mut store, new);
definition.write_gc_ref(&mut store, new)?;
}
Val::ContRef(None) => {
// Allow null continuation references for globals - these are just placeholders
definition.write_gc_ref(&mut store, None);
definition.write_gc_ref(&mut store, None)?;
}
Val::ContRef(Some(_)) => {
// TODO(#10248): Implement non-null global continuation reference handling
Expand Down
12 changes: 6 additions & 6 deletions crates/wasmtime/src/runtime/externals/table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use crate::prelude::*;
use crate::runtime::RootedGcRefImpl;
use crate::runtime::vm::{
self, GcStore, SendSyncPtr, TableElementType, VMFuncRef, VMGcRef, VMStore,
};
use crate::store::{AutoAssertNoGc, StoreInstanceId, StoreOpaque, StoreResourceLimiter};
use crate::trampoline::generate_table_export;
use crate::{
AnyRef, AsContext, AsContextMut, ExnRef, ExternRef, Func, HeapType, Ref, RefType,
StoreContextMut, TableType, Trap,
StoreContextMut, TableType,
};
use core::iter;
use core::ptr::NonNull;
Expand Down Expand Up @@ -441,7 +440,7 @@ impl Table {
src_table: &Table,
src_index: u64,
len: u64,
) -> Result<(), Trap> {
) -> Result<()> {
// Handle lazy initialization of the source table first before doing
// anything else.
let src_range = src_index..(src_index.checked_add(len).unwrap_or(u64::MAX));
Expand Down Expand Up @@ -479,7 +478,7 @@ impl Table {
dst_index,
src_index,
len,
)
)?;
}

// 2. Intra-instance, distinct-tables copy: split the mutable
Expand All @@ -491,17 +490,18 @@ impl Table {
.tables_mut()
.get_disjoint_mut([src_table.index, dst_table.index])
.unwrap();
src_table.copy_to(dst_table, gc_store, dst_index, src_index, len)
src_table.copy_to(dst_table, gc_store, dst_index, src_index, len)?;
}

// 3. Intra-table copy: get the table and copy within it!
(true, true) => {
let (gc_store, instance) = store.optional_gc_store_and_instance_mut(src_instance);
instance
.get_defined_table(src_table.index)
.copy_within(gc_store, dst_index, src_index, len)
.copy_within(gc_store, dst_index, src_index, len)?;
}
}
Ok(())
}

/// Fill `table[dst..(dst + len)]` with the given value.
Expand Down
4 changes: 2 additions & 2 deletions crates/wasmtime/src/runtime/func.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2157,11 +2157,11 @@ impl<T> Caller<'_, T> {
///
/// Same as [`Store::gc_async`](crate::Store::gc_async).
#[cfg(all(feature = "async", feature = "gc"))]
pub async fn gc_async(&mut self, why: Option<&crate::GcHeapOutOfMemory<()>>)
pub async fn gc_async(&mut self, why: Option<&crate::GcHeapOutOfMemory<()>>) -> Result<()>
where
T: Send + 'static,
{
self.store.gc_async(why).await;
self.store.gc_async(why).await
}

/// Returns the remaining fuel in the store.
Expand Down
8 changes: 8 additions & 0 deletions crates/wasmtime/src/runtime/gc/disabled/rooting.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,14 @@ impl<T: GcRef> Rooted<T> {
) -> Result<bool> {
a.assert_unreachable()
}

pub(crate) fn try_gc_ref<'a>(&self, _: &'a StoreOpaque) -> Result<&'a VMGcRef> {
match self.inner {}
}

pub(crate) fn try_clone_gc_ref(&self, _: &mut AutoAssertNoGc<'_>) -> Result<VMGcRef> {
match self.inner {}
}
}

/// This type has been disabled because the `gc` cargo feature was not enabled
Expand Down
28 changes: 18 additions & 10 deletions crates/wasmtime/src/runtime/gc/enabled/anyref.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
//! Implementation of `anyref` in Wasmtime.

use super::{ExternRef, RootedGcRefImpl};
use super::ExternRef;
use crate::prelude::*;
use crate::runtime::vm::VMGcRef;
use crate::{
ArrayRef, ArrayType, AsContext, AsContextMut, EqRef, GcRefImpl, GcRootIndex, HeapType, I31,
OwnedRooted, RefType, Result, Rooted, StructRef, StructType, ValRaw, ValType, WasmTy,
OwnedRooted, RefType, Result, Rooted, StructRef, StructType, ValRaw, ValType, WasmTy, bail_bug,
store::{AutoAssertNoGc, StoreOpaque},
};
use core::mem;
Expand Down Expand Up @@ -298,11 +298,13 @@ impl AnyRef {
|| store
.unwrap_gc_store()
.header(&gc_ref)
.unwrap()
.kind()
.matches(VMGcKind::AnyRef)
|| store
.unwrap_gc_store()
.header(&gc_ref)
.unwrap()
.kind()
.matches(VMGcKind::ExternRef)
);
Expand Down Expand Up @@ -336,7 +338,9 @@ impl AnyRef {
let raw = if gc_ref.is_i31() {
gc_ref.as_raw_non_zero_u32()
} else {
store.require_gc_store_mut()?.expose_gc_ref_to_wasm(gc_ref)
store
.require_gc_store_mut()?
.expose_gc_ref_to_wasm(gc_ref)?
};
Ok(raw.get())
}
Expand All @@ -360,29 +364,33 @@ impl AnyRef {
return Ok(HeapType::I31);
}

let header = store.require_gc_store()?.header(gc_ref);
let header = store.require_gc_store()?.header(gc_ref)?;

if header.kind().matches(VMGcKind::ExternRef) {
return Ok(HeapType::Any);
}

debug_assert!(header.kind().matches(VMGcKind::AnyRef));
debug_assert!(header.kind().matches(VMGcKind::EqRef));
let ty = match header.ty() {
Some(ty) => ty,
None => bail_bug!("ty should be present"),
};

if header.kind().matches(VMGcKind::StructRef) {
return Ok(HeapType::ConcreteStruct(
StructType::from_shared_type_index(store.engine(), header.ty().unwrap()),
StructType::from_shared_type_index(store.engine(), ty),
));
}

if header.kind().matches(VMGcKind::ArrayRef) {
return Ok(HeapType::ConcreteArray(ArrayType::from_shared_type_index(
store.engine(),
header.ty().unwrap(),
ty,
)));
}

unreachable!("no other kinds of `anyref`s")
bail_bug!("no other kinds of `anyref`s")
}

/// Does this `anyref` match the given type?
Expand Down Expand Up @@ -436,7 +444,7 @@ impl AnyRef {
Ok(gc_ref.is_i31()
|| store
.require_gc_store()?
.kind(gc_ref)
.kind(gc_ref)?
.matches(VMGcKind::EqRef))
}

Expand Down Expand Up @@ -564,7 +572,7 @@ impl AnyRef {
Ok(!gc_ref.is_i31()
&& store
.require_gc_store()?
.kind(gc_ref)
.kind(gc_ref)?
.matches(VMGcKind::StructRef))
}

Expand Down Expand Up @@ -632,7 +640,7 @@ impl AnyRef {
Ok(!gc_ref.is_i31()
&& store
.require_gc_store()?
.kind(gc_ref)
.kind(gc_ref)?
.matches(VMGcKind::ArrayRef))
}

Expand Down
26 changes: 14 additions & 12 deletions crates/wasmtime/src/runtime/gc/enabled/arrayref.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ impl ArrayRef {
"attempted to use a `ArrayRefPre` with the wrong store"
);

let len = u32::try_from(elems.len()).unwrap();
let len = u32::try_from(elems.len())?;

// Allocate the array.
let arrayref = store
Expand All @@ -424,15 +424,17 @@ impl ArrayRef {
match (|| {
let elem_ty = allocator.ty.element_type();
for (i, elem) in elems.enumerate() {
let i = u32::try_from(i).unwrap();
let i = u32::try_from(i)?;
debug_assert!(i < len);
arrayref.initialize_elem(&mut store, allocator.layout(), &elem_ty, i, *elem)?;
}
Ok(())
})() {
Ok(()) => Ok(Rooted::new(&mut store, arrayref.into())),
Err(e) => {
store.require_gc_store_mut()?.dealloc_uninit_array(arrayref);
store
.require_gc_store_mut()?
.dealloc_uninit_array(arrayref)?;
Err(e)
}
}
Expand Down Expand Up @@ -621,11 +623,11 @@ impl ArrayRef {
assert!(self.comes_from_same_store(store));
let gc_ref = self.inner.try_gc_ref(store)?;
debug_assert!({
let header = store.require_gc_store()?.header(gc_ref);
let header = store.require_gc_store()?.header(gc_ref)?;
header.kind().matches(VMGcKind::ArrayRef)
});
let arrayref = gc_ref.as_arrayref_unchecked();
Ok(arrayref.len(store))
arrayref.len(store)
}

/// Get the values of this array's elements.
Expand Down Expand Up @@ -655,7 +657,7 @@ impl ArrayRef {
let store = AutoAssertNoGc::new(store);

let gc_ref = self.inner.try_gc_ref(&store)?;
let header = store.require_gc_store()?.header(gc_ref);
let header = store.require_gc_store()?.header(gc_ref)?;
debug_assert!(header.kind().matches(VMGcKind::ArrayRef));

let len = self._len(&store)?;
Expand Down Expand Up @@ -685,7 +687,7 @@ impl ArrayRef {
return None;
}
self.index += 1;
Some(self.arrayref._get(&mut self.store, i).unwrap())
self.arrayref._get(&mut self.store, i).ok()
}

#[inline]
Expand All @@ -708,7 +710,7 @@ impl ArrayRef {
fn header<'a>(&self, store: &'a AutoAssertNoGc<'_>) -> Result<&'a VMGcHeader> {
assert!(self.comes_from_same_store(&store));
let gc_ref = self.inner.try_gc_ref(store)?;
Ok(store.require_gc_store()?.header(gc_ref))
Ok(store.require_gc_store()?.header(gc_ref)?)
}

fn arrayref<'a>(&self, store: &'a AutoAssertNoGc<'_>) -> Result<&'a VMArrayRef> {
Expand Down Expand Up @@ -763,12 +765,12 @@ impl ArrayRef {
let arrayref = self.arrayref(store)?.unchecked_copy();
let field_ty = self.field_ty(store)?;
let layout = self.layout(store)?;
let len = arrayref.len(store);
let len = arrayref.len(store)?;
ensure!(
index < len,
"index out of bounds: the length is {len} but the index is {index}"
);
Ok(arrayref.read_elem(store, &layout, field_ty.element_type(), index))
arrayref.read_elem(store, &layout, field_ty.element_type(), index)
}

/// Set this array's `index`th element.
Expand Down Expand Up @@ -819,7 +821,7 @@ impl ArrayRef {
let layout = self.layout(&store)?;
let arrayref = self.arrayref(&store)?.unchecked_copy();

let len = arrayref.len(&store);
let len = arrayref.len(&store)?;
ensure!(
index < len,
"index out of bounds: the length is {len} but the index is {index}"
Expand All @@ -830,7 +832,7 @@ impl ArrayRef {

pub(crate) fn type_index(&self, store: &StoreOpaque) -> Result<VMSharedTypeIndex> {
let gc_ref = self.inner.try_gc_ref(store)?;
let header = store.require_gc_store()?.header(gc_ref);
let header = store.require_gc_store()?.header(gc_ref)?;
debug_assert!(header.kind().matches(VMGcKind::ArrayRef));
Ok(header.ty().expect("arrayrefs should have concrete types"))
}
Expand Down
Loading
Loading