Skip to content

Commit

Permalink
Reserve handle index 0 in the component model
Browse files Browse the repository at this point in the history
This commit updates the allocation scheme for resources in the component
model to start at 1 instead of 0 when communicating with components.
This is an implementation of WebAssembly/component-model#284.

While this broke a number of tests we have this shouldn't actually break
any components in practice. The broken tests were all overly-precise in
their assertions and error messages and this shouldn't idiomatically
come up in any guest language, so this should not be a practically
breaking change.

This change additionally places an upper limit on the maximum
allocatable index at `1 << 30` which is also specified in the above PR.
  • Loading branch information
alexcrichton committed Dec 8, 2023
1 parent 63e2606 commit a78bfbf
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 76 deletions.
6 changes: 3 additions & 3 deletions crates/runtime/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,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 @@ -595,7 +595,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 @@ -618,7 +618,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
58 changes: 34 additions & 24 deletions crates/runtime/src/component/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,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 +173,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 +241,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 +286,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 +298,42 @@ 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 >= 1 << 30 {
bail!("cannot allocate another handle: index overflow");
}
Ok(ret)
}

fn rep(&self, idx: u32) -> Result<u32> {
match usize::try_from(idx).ok().and_then(|i| self.slots.get(i)) {
// NB: `idx` is decremented by one to account for the `+1` above during
// allocation.
let table_index = idx.checked_sub(1).and_then(|i| usize::try_from(i).ok());
match table_index.and_then(|i| self.slots.get(i)) {
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))
{
// NB: `idx` is decremented by one to account for the `+1` above during
// allocation.
let table_index = idx.checked_sub(1).and_then(|i| usize::try_from(i).ok());
match table_index.and_then(|i| self.slots.get_mut(i)) {
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/component/func/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,13 +277,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().resource_lower_own(Some(ty), rep)
}

/// 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 @@ -296,7 +304,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().resource_lower_borrow(Some(ty), rep)
}
Expand All @@ -318,7 +326,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) -> u32 {
pub fn host_resource_lower_own(&mut self, rep: u32) -> Result<u32> {
self.resource_tables().resource_lower_own(None, rep)
}

Expand Down Expand Up @@ -468,13 +476,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) -> u32 {
pub fn host_resource_lower_own(&mut self, rep: u32) -> Result<u32> {
self.resource_tables().resource_lower_own(None, 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) -> u32 {
pub fn host_resource_lower_borrow(&mut self, rep: u32) -> Result<u32> {
self.resource_tables().resource_lower_borrow(None, rep)
}

Expand Down
14 changes: 7 additions & 7 deletions crates/wasmtime/src/component/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ where
// can move the rep into the guest table.
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.load(Relaxed) {
Expand All @@ -343,7 +343,7 @@ where
//
// Afterwards this is the same as the `idx` case below.
NOT_IN_TABLE => {
let idx = cx.host_resource_lower_own(self.rep);
let idx = cx.host_resource_lower_own(self.rep)?;
let prev = self.state.swap(idx, Relaxed);
assert_eq!(prev, NOT_IN_TABLE);
cx.host_resource_lift_borrow(idx)?
Expand All @@ -353,7 +353,7 @@ where
// out of the table with borrow-tracking employed.
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 @@ -613,14 +613,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 @@ -631,7 +631,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 @@ -645,7 +645,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 @@ -203,7 +203,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 @@ -341,7 +341,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 @@ -1069,7 +1069,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 @@ -1227,9 +1227,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

0 comments on commit a78bfbf

Please sign in to comment.