Skip to content
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.

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

Reserve handle index 0 in the component model #7661

Merged
merged 1 commit into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions crates/runtime/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ impl ComponentInstance {

/// Implementation of the `resource.new` intrinsic for `i32`
/// representations.
pub fn resource_new32(&mut self, resource: TypeResourceTableIndex, rep: u32) -> u32 {
pub fn resource_new32(&mut self, resource: TypeResourceTableIndex, rep: u32) -> Result<u32> {
self.resource_tables().resource_new(Some(resource), rep)
}

Expand Down Expand Up @@ -600,7 +600,7 @@ impl ComponentInstance {
) -> Result<u32> {
let mut tables = self.resource_tables();
let rep = tables.resource_lift_own(Some(src), idx)?;
Ok(tables.resource_lower_own(Some(dst), rep))
tables.resource_lower_own(Some(dst), rep)
}

pub(crate) fn resource_transfer_borrow(
Expand All @@ -623,7 +623,7 @@ impl ComponentInstance {
if dst_owns_resource {
return Ok(rep);
}
Ok(tables.resource_lower_borrow(Some(dst), rep))
tables.resource_lower_borrow(Some(dst), rep)
}

pub(crate) fn resource_enter_call(&mut self) {
Expand Down
4 changes: 1 addition & 3 deletions crates/runtime/src/component/libcalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,9 +519,7 @@ fn inflate_latin1_bytes(dst: &mut [u16], latin1_bytes_so_far: usize) -> &mut [u1

unsafe fn resource_new32(vmctx: *mut VMComponentContext, resource: u32, rep: u32) -> Result<u32> {
let resource = TypeResourceTableIndex::from_u32(resource);
Ok(ComponentInstance::from_vmctx(vmctx, |instance| {
instance.resource_new32(resource, rep)
}))
ComponentInstance::from_vmctx(vmctx, |instance| instance.resource_new32(resource, rep))
}

unsafe fn resource_rep32(vmctx: *mut VMComponentContext, resource: u32, idx: u32) -> Result<u32> {
Expand Down
70 changes: 46 additions & 24 deletions crates/runtime/src/component/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ use std::mem;
use wasmtime_environ::component::TypeResourceTableIndex;
use wasmtime_environ::PrimaryMap;

/// The maximum handle value is specified in
/// <https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md>
/// currently and keeps the upper bit free for use in the component.
const MAX_RESOURCE_HANDLE: u32 = 1 << 30;

/// Contextual state necessary to perform resource-related operations.
///
/// This state a bit odd since it has a few optional bits, but the idea is that
Expand Down Expand Up @@ -125,7 +130,7 @@ impl ResourceTables<'_> {
/// Implementation of the `resource.new` canonical intrinsic.
///
/// Note that this is the same as `resource_lower_own`.
pub fn resource_new(&mut self, ty: Option<TypeResourceTableIndex>, rep: u32) -> u32 {
pub fn resource_new(&mut self, ty: Option<TypeResourceTableIndex>, rep: u32) -> Result<u32> {
self.table(ty).insert(Slot::Own { rep, lend_count: 0 })
}

Expand Down Expand Up @@ -173,7 +178,11 @@ impl ResourceTables<'_> {
/// the same as `resource_new` implementation-wise.
///
/// This is an implementation of the canonical ABI `lower_own` function.
pub fn resource_lower_own(&mut self, ty: Option<TypeResourceTableIndex>, rep: u32) -> u32 {
pub fn resource_lower_own(
&mut self,
ty: Option<TypeResourceTableIndex>,
rep: u32,
) -> Result<u32> {
self.table(ty).insert(Slot::Own { rep, lend_count: 0 })
}

Expand Down Expand Up @@ -237,7 +246,11 @@ impl ResourceTables<'_> {
/// function. The other half of this implementation is located on
/// `VMComponentContext` which handles the special case of avoiding borrow
/// tracking entirely.
pub fn resource_lower_borrow(&mut self, ty: Option<TypeResourceTableIndex>, rep: u32) -> u32 {
pub fn resource_lower_borrow(
&mut self,
ty: Option<TypeResourceTableIndex>,
rep: u32,
) -> Result<u32> {
let scope = self.calls.scopes.len() - 1;
let borrow_count = &mut self.calls.scopes.last_mut().unwrap().borrow_count;
*borrow_count = borrow_count.checked_add(1).unwrap();
Expand Down Expand Up @@ -278,12 +291,8 @@ impl ResourceTables<'_> {
}

impl ResourceTable {
fn next(&self) -> usize {
self.next as usize
}

fn insert(&mut self, new: Slot) -> u32 {
let next = self.next();
fn insert(&mut self, new: Slot) -> Result<u32> {
let next = self.next as usize;
if next == self.slots.len() {
self.slots.push(Slot::Free {
next: self.next.checked_add(1).unwrap(),
Expand All @@ -294,36 +303,49 @@ impl ResourceTable {
Slot::Free { next } => next,
_ => unreachable!(),
};
u32::try_from(ret).unwrap()

// The component model reserves index 0 as never allocatable so add one
// to the table index to start the numbering at 1 instead. Also note
// that the component model places an upper-limit per-table on the
// maximum allowed index.
let ret = ret + 1;
if ret >= MAX_RESOURCE_HANDLE {
bail!("cannot allocate another handle: index overflow");
}
Ok(ret)
}

fn handle_index_to_table_index(&self, idx: u32) -> Option<usize> {
// NB: `idx` is decremented by one to account for the `+1` above during
// allocation.
let idx = idx.checked_sub(1)?;
usize::try_from(idx).ok()
}

fn rep(&self, idx: u32) -> Result<u32> {
match usize::try_from(idx).ok().and_then(|i| self.slots.get(i)) {
let slot = self
.handle_index_to_table_index(idx)
.and_then(|i| self.slots.get(i));
match slot {
None | Some(Slot::Free { .. }) => bail!("unknown handle index {idx}"),
Some(Slot::Own { rep, .. } | Slot::Borrow { rep, .. }) => Ok(*rep),
}
}

fn get_mut(&mut self, idx: u32) -> Result<&mut Slot> {
match usize::try_from(idx)
.ok()
.and_then(|i| self.slots.get_mut(i))
{
let slot = self
.handle_index_to_table_index(idx)
.and_then(|i| self.slots.get_mut(i));
match slot {
None | Some(Slot::Free { .. }) => bail!("unknown handle index {idx}"),
Some(other) => Ok(other),
}
}

fn remove(&mut self, idx: u32) -> Result<Slot> {
match usize::try_from(idx).ok().and_then(|i| self.slots.get(i)) {
Some(Slot::Own { .. }) | Some(Slot::Borrow { .. }) => {}
_ => bail!("unknown handle index {idx}"),
};
let ret = mem::replace(
&mut self.slots[idx as usize],
Slot::Free { next: self.next },
);
self.next = idx;
let to_fill = Slot::Free { next: self.next };
let ret = mem::replace(self.get_mut(idx)?, to_fill);
self.next = idx - 1;
Ok(ret)
}
}
20 changes: 14 additions & 6 deletions crates/wasmtime/src/runtime/component/func/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,13 +289,21 @@ impl<'a, T> LowerContext<'a, T> {
/// into a guest-local index.
///
/// The `ty` provided is which table to put this into.
pub fn guest_resource_lower_own(&mut self, ty: TypeResourceTableIndex, rep: u32) -> u32 {
pub fn guest_resource_lower_own(
&mut self,
ty: TypeResourceTableIndex,
rep: u32,
) -> Result<u32> {
self.resource_tables().guest_resource_lower_own(rep, ty)
}

/// Lowers a `borrow` resource into the guest, converting the `rep` to a
/// guest-local index in the `ty` table specified.
pub fn guest_resource_lower_borrow(&mut self, ty: TypeResourceTableIndex, rep: u32) -> u32 {
pub fn guest_resource_lower_borrow(
&mut self,
ty: TypeResourceTableIndex,
rep: u32,
) -> Result<u32> {
// Implement `lower_borrow`'s special case here where if a borrow is
// inserted into a table owned by the instance which implemented the
// original resource then no borrow tracking is employed and instead the
Expand All @@ -308,7 +316,7 @@ impl<'a, T> LowerContext<'a, T> {
// Note that the unsafety here should be valid given the contract of
// `LowerContext::new`.
if unsafe { (*self.instance).resource_owned_by_own_instance(ty) } {
return rep;
return Ok(rep);
}
self.resource_tables().guest_resource_lower_borrow(rep, ty)
}
Expand All @@ -330,7 +338,7 @@ impl<'a, T> LowerContext<'a, T> {
///
/// Note that this is a special case for `Resource<T>`. Most of the time a
/// host value shouldn't be lowered with a lowering context.
pub fn host_resource_lower_own(&mut self, rep: u32) -> HostResourceIndex {
pub fn host_resource_lower_own(&mut self, rep: u32) -> Result<HostResourceIndex> {
self.resource_tables().host_resource_lower_own(rep)
}

Expand Down Expand Up @@ -483,13 +491,13 @@ impl<'a> LiftContext<'a> {

/// Lowers a resource into the host-owned table, returning the index it was
/// inserted at.
pub fn host_resource_lower_own(&mut self, rep: u32) -> HostResourceIndex {
pub fn host_resource_lower_own(&mut self, rep: u32) -> Result<HostResourceIndex> {
self.resource_tables().host_resource_lower_own(rep)
}

/// Lowers a resource into the host-owned table, returning the index it was
/// inserted at.
pub fn host_resource_lower_borrow(&mut self, rep: u32) -> HostResourceIndex {
pub fn host_resource_lower_borrow(&mut self, rep: u32) -> Result<HostResourceIndex> {
self.resource_tables().host_resource_lower_borrow(rep)
}

Expand Down
48 changes: 31 additions & 17 deletions crates/wasmtime/src/runtime/component/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,15 +397,15 @@ impl<'a> HostResourceTables<'a> {
/// will point to the `rep` specified as well as recording that it has the
/// `ty` specified. The returned index is suitable for conversion into
/// either [`Resource`] or [`ResourceAny`].
pub fn host_resource_lower_own(&mut self, rep: u32) -> HostResourceIndex {
let idx = self.tables.resource_lower_own(None, rep);
self.new_host_index(idx)
pub fn host_resource_lower_own(&mut self, rep: u32) -> Result<HostResourceIndex> {
let idx = self.tables.resource_lower_own(None, rep)?;
Ok(self.new_host_index(idx))
}

/// See [`HostResourceTables::host_resource_lower_own`].
pub fn host_resource_lower_borrow(&mut self, rep: u32) -> HostResourceIndex {
let idx = self.tables.resource_lower_borrow(None, rep);
self.new_host_index(idx)
pub fn host_resource_lower_borrow(&mut self, rep: u32) -> Result<HostResourceIndex> {
let idx = self.tables.resource_lower_borrow(None, rep)?;
Ok(self.new_host_index(idx))
}

/// Validates that `idx` is still valid for the host tables, notably
Expand Down Expand Up @@ -453,6 +453,12 @@ impl<'a> HostResourceTables<'a> {
match list.get_mut(idx as usize) {
Some(slot) => *slot = gen,
None => {
// Resource handles start at 1, not zero, so push two elements
// for the first resource handle.
if list.is_empty() {
assert_eq!(idx, 1);
list.push(0);
}
assert_eq!(idx as usize, list.len());
list.push(gen);
}
Expand Down Expand Up @@ -482,7 +488,11 @@ impl<'a> HostResourceTables<'a> {
/// into a guest-local index.
///
/// The `ty` provided is which table to put this into.
pub fn guest_resource_lower_own(&mut self, rep: u32, ty: TypeResourceTableIndex) -> u32 {
pub fn guest_resource_lower_own(
&mut self,
rep: u32,
ty: TypeResourceTableIndex,
) -> Result<u32> {
self.tables.resource_lower_own(Some(ty), rep)
}

Expand All @@ -495,7 +505,11 @@ impl<'a> HostResourceTables<'a> {
/// into a guest has a special case where `rep` is returned directly if `ty`
/// belongs to the component being lowered into. That property must be
/// handled by the caller of this function.
pub fn guest_resource_lower_borrow(&mut self, rep: u32, ty: TypeResourceTableIndex) -> u32 {
pub fn guest_resource_lower_borrow(
&mut self,
rep: u32,
ty: TypeResourceTableIndex,
) -> Result<u32> {
self.tables.resource_lower_borrow(Some(ty), rep)
}

Expand Down Expand Up @@ -612,7 +626,7 @@ where
// can move the rep into the guest table.
ResourceState::Index(idx) => cx.host_resource_lift_own(idx)?,
};
Ok(cx.guest_resource_lower_own(t, rep))
cx.guest_resource_lower_own(t, rep)
}
InterfaceType::Borrow(t) => {
let rep = match self.state.get() {
Expand All @@ -632,7 +646,7 @@ where
//
// Afterwards this is the same as the `idx` case below.
ResourceState::NotInTable => {
let idx = cx.host_resource_lower_own(self.rep);
let idx = cx.host_resource_lower_own(self.rep)?;
let prev = self.state.swap(ResourceState::Index(idx));
assert_eq!(prev, ResourceState::NotInTable);
cx.host_resource_lift_borrow(idx)?
Expand All @@ -642,7 +656,7 @@ where
// out of the table with borrow-tracking employed.
ResourceState::Index(idx) => cx.host_resource_lift_borrow(idx)?,
};
Ok(cx.guest_resource_lower_borrow(t, rep))
cx.guest_resource_lower_borrow(t, rep)
}
_ => bad_type_info(),
}
Expand Down Expand Up @@ -879,9 +893,9 @@ impl ResourceAny {

let mut tables = HostResourceTables::new_host(store.0);
let (idx, own_state) = match state.get() {
ResourceState::Borrow => (tables.host_resource_lower_borrow(rep), None),
ResourceState::Borrow => (tables.host_resource_lower_borrow(rep)?, None),
ResourceState::NotInTable => {
let idx = tables.host_resource_lower_own(rep);
let idx = tables.host_resource_lower_own(rep)?;
(
idx,
Some(OwnState {
Expand Down Expand Up @@ -1025,14 +1039,14 @@ impl ResourceAny {
bail!("mismatched resource types");
}
let rep = cx.host_resource_lift_own(self.idx)?;
Ok(cx.guest_resource_lower_own(t, rep))
cx.guest_resource_lower_own(t, rep)
}
InterfaceType::Borrow(t) => {
if cx.resource_type(t) != self.ty {
bail!("mismatched resource types");
}
let rep = cx.host_resource_lift_borrow(self.idx)?;
Ok(cx.guest_resource_lower_borrow(t, rep))
cx.guest_resource_lower_borrow(t, rep)
}
_ => bad_type_info(),
}
Expand All @@ -1043,7 +1057,7 @@ impl ResourceAny {
InterfaceType::Own(t) => {
let ty = cx.resource_type(t);
let (rep, dtor, flags) = cx.guest_resource_lift_own(t, index)?;
let idx = cx.host_resource_lower_own(rep);
let idx = cx.host_resource_lower_own(rep)?;
Ok(ResourceAny {
idx,
ty,
Expand All @@ -1057,7 +1071,7 @@ impl ResourceAny {
InterfaceType::Borrow(t) => {
let ty = cx.resource_type(t);
let rep = cx.guest_resource_lift_borrow(t, index)?;
let idx = cx.host_resource_lower_borrow(rep);
let idx = cx.host_resource_lower_borrow(rep)?;
Ok(ResourceAny {
idx,
ty,
Expand Down
12 changes: 6 additions & 6 deletions tests/all/component_model/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ fn mismatch_intrinsics() -> Result<()> {
let ctor = i.get_typed_func::<(u32,), (ResourceAny,)>(&mut store, "ctor")?;
assert_eq!(
ctor.call(&mut store, (100,)).unwrap_err().to_string(),
"unknown handle index 0"
"unknown handle index 1"
);

Ok(())
Expand Down Expand Up @@ -371,7 +371,7 @@ fn drop_guest_twice() -> Result<()> {

assert_eq!(
dtor.call(&mut store, (&t,)).unwrap_err().to_string(),
"unknown handle index 0"
"unknown handle index 1"
);

Ok(())
Expand Down Expand Up @@ -1250,7 +1250,7 @@ fn pass_guest_back_as_borrow() -> Result<()> {

// Should not be valid to use `resource` again
let err = take.call(&mut store, (&resource,)).unwrap_err();
assert_eq!(err.to_string(), "unknown handle index 0");
assert_eq!(err.to_string(), "unknown handle index 1");

Ok(())
}
Expand Down Expand Up @@ -1412,9 +1412,9 @@ fn guest_different_host_same() -> Result<()> {
(import "" "drop2" (func $drop2 (param i32)))

(func (export "f") (param i32 i32)
;; separate tables both have initial index of 0
(if (i32.ne (local.get 0) (i32.const 0)) (then (unreachable)))
(if (i32.ne (local.get 1) (i32.const 0)) (then (unreachable)))
;; separate tables both have initial index of 1
(if (i32.ne (local.get 0) (i32.const 1)) (then (unreachable)))
(if (i32.ne (local.get 1) (i32.const 1)) (then (unreachable)))

;; host should end up getting the same resource
(call $f (local.get 0) (local.get 1))
Expand Down
Loading