diff --git a/crates/runtime/src/component.rs b/crates/runtime/src/component.rs index fc0d4bf2a983..b6381d20579b 100644 --- a/crates/runtime/src/component.rs +++ b/crates/runtime/src/component.rs @@ -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 { self.resource_tables().resource_new(Some(resource), rep) } @@ -595,7 +595,7 @@ impl ComponentInstance { ) -> Result { 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( @@ -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) { diff --git a/crates/runtime/src/component/libcalls.rs b/crates/runtime/src/component/libcalls.rs index 920a505339a3..cd0323ad3c45 100644 --- a/crates/runtime/src/component/libcalls.rs +++ b/crates/runtime/src/component/libcalls.rs @@ -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 { 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 { diff --git a/crates/runtime/src/component/resources.rs b/crates/runtime/src/component/resources.rs index 4c22db5569b0..310542f76ff7 100644 --- a/crates/runtime/src/component/resources.rs +++ b/crates/runtime/src/component/resources.rs @@ -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, rep: u32) -> u32 { + pub fn resource_new(&mut self, ty: Option, rep: u32) -> Result { self.table(ty).insert(Slot::Own { rep, lend_count: 0 }) } @@ -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, rep: u32) -> u32 { + pub fn resource_lower_own( + &mut self, + ty: Option, + rep: u32, + ) -> Result { self.table(ty).insert(Slot::Own { rep, lend_count: 0 }) } @@ -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, rep: u32) -> u32 { + pub fn resource_lower_borrow( + &mut self, + ty: Option, + rep: u32, + ) -> Result { 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(); @@ -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 { + let next = self.next as usize; if next == self.slots.len() { self.slots.push(Slot::Free { next: self.next.checked_add(1).unwrap(), @@ -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 { - 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 { - 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) } } diff --git a/crates/wasmtime/src/component/func/options.rs b/crates/wasmtime/src/component/func/options.rs index 6cb038f4f692..00e060d174a2 100644 --- a/crates/wasmtime/src/component/func/options.rs +++ b/crates/wasmtime/src/component/func/options.rs @@ -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 { 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 { // 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 @@ -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) } @@ -318,7 +326,7 @@ impl<'a, T> LowerContext<'a, T> { /// /// Note that this is a special case for `Resource`. 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 { self.resource_tables().resource_lower_own(None, rep) } @@ -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 { 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 { self.resource_tables().resource_lower_borrow(None, rep) } diff --git a/crates/wasmtime/src/component/resources.rs b/crates/wasmtime/src/component/resources.rs index 9ed13faf3208..fc8b1806263b 100644 --- a/crates/wasmtime/src/component/resources.rs +++ b/crates/wasmtime/src/component/resources.rs @@ -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) { @@ -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)? @@ -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(), } @@ -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(), } @@ -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, @@ -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, diff --git a/tests/all/component_model/resources.rs b/tests/all/component_model/resources.rs index ba821251f79c..b4576ba48614 100644 --- a/tests/all/component_model/resources.rs +++ b/tests/all/component_model/resources.rs @@ -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(()) @@ -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(()) @@ -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(()) } @@ -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)) diff --git a/tests/misc_testsuite/component-model/resources.wast b/tests/misc_testsuite/component-model/resources.wast index 89b97536aebc..351ed85b3b6f 100644 --- a/tests/misc_testsuite/component-model/resources.wast +++ b/tests/misc_testsuite/component-model/resources.wast @@ -14,7 +14,7 @@ (local $r i32) (local.set $r (call $new (i32.const 100))) - (if (i32.ne (local.get $r) (i32.const 0)) (then (unreachable))) + (if (i32.ne (local.get $r) (i32.const 1)) (then (unreachable))) (if (i32.ne (call $rep (local.get $r)) (i32.const 100)) (then (unreachable))) (call $drop (local.get $r)) @@ -95,13 +95,13 @@ ;; resources assigned sequentially (local.set $r1 (call $new (i32.const 100))) - (if (i32.ne (local.get $r1) (i32.const 0)) (then (unreachable))) + (if (i32.ne (local.get $r1) (i32.const 1)) (then (unreachable))) (local.set $r2 (call $new (i32.const 200))) - (if (i32.ne (local.get $r2) (i32.const 1)) (then (unreachable))) + (if (i32.ne (local.get $r2) (i32.const 2)) (then (unreachable))) (local.set $r3 (call $new (i32.const 300))) - (if (i32.ne (local.get $r3) (i32.const 2)) (then (unreachable))) + (if (i32.ne (local.get $r3) (i32.const 3)) (then (unreachable))) ;; representations all look good (if (i32.ne (call $rep (local.get $r1)) (i32.const 100)) (then (unreachable))) @@ -113,7 +113,7 @@ (local.set $r2 (call $new (i32.const 400))) ;; should have reused index 1 - (if (i32.ne (local.get $r2) (i32.const 1)) (then (unreachable))) + (if (i32.ne (local.get $r2) (i32.const 2)) (then (unreachable))) ;; representations all look good (if (i32.ne (call $rep (local.get $r1)) (i32.const 100)) (then (unreachable))) @@ -135,13 +135,13 @@ (if (i32.ne (call $rep (local.get $r3)) (i32.const 700)) (then (unreachable))) ;; indices should be lifo - (if (i32.ne (local.get $r1) (i32.const 2)) (then (unreachable))) - (if (i32.ne (local.get $r2) (i32.const 1)) (then (unreachable))) - (if (i32.ne (local.get $r3) (i32.const 0)) (then (unreachable))) + (if (i32.ne (local.get $r1) (i32.const 3)) (then (unreachable))) + (if (i32.ne (local.get $r2) (i32.const 2)) (then (unreachable))) + (if (i32.ne (local.get $r3) (i32.const 1)) (then (unreachable))) ;; bump one more time (local.set $r4 (call $new (i32.const 800))) - (if (i32.ne (local.get $r4) (i32.const 3)) (then (unreachable))) + (if (i32.ne (local.get $r4) (i32.const 4)) (then (unreachable))) ;; deallocate everything (call $drop (local.get $r1)) @@ -241,13 +241,13 @@ (local.set $r2 (call $ctor (i32.const 200))) ;; assert r1/r2 are sequential - (if (i32.ne (local.get $r1) (i32.const 0)) (then (unreachable))) - (if (i32.ne (local.get $r2) (i32.const 1)) (then (unreachable))) + (if (i32.ne (local.get $r1) (i32.const 1)) (then (unreachable))) + (if (i32.ne (local.get $r2) (i32.const 2)) (then (unreachable))) ;; reallocate r1 and it should be reassigned the same index (call $drop (local.get $r1)) (local.set $r1 (call $ctor (i32.const 300))) - (if (i32.ne (local.get $r1) (i32.const 0)) (then (unreachable))) + (if (i32.ne (local.get $r1) (i32.const 1)) (then (unreachable))) ;; internal values should match (call $assert (local.get $r1) (i32.const 300)) @@ -443,7 +443,7 @@ (import "" "ctor" (func $ctor (param i32) (result i32))) (func $start - (if (i32.ne (call $ctor (i32.const 100)) (i32.const 0)) (then (unreachable))) + (if (i32.ne (call $ctor (i32.const 100)) (i32.const 1)) (then (unreachable))) ) (start $start) ) @@ -464,7 +464,7 @@ (import "" "drop" (func $drop (param i32))) (func (export "f") - (call $drop (i32.const 0)) + (call $drop (i32.const 1)) ) ) (core instance $i (instantiate $m @@ -492,10 +492,10 @@ (import "" "drop" (func $drop (param i32))) (func (export "alloc") - (if (i32.ne (call $ctor (i32.const 100)) (i32.const 0)) (then (unreachable))) + (if (i32.ne (call $ctor (i32.const 100)) (i32.const 1)) (then (unreachable))) ) (func (export "dealloc") - (call $drop (i32.const 0)) + (call $drop (i32.const 1)) ) ) (core instance $i (instantiate $m @@ -552,10 +552,10 @@ (import "" "drop" (func $drop (param i32))) (func (export "alloc") - (if (i32.ne (call $ctor (i32.const 100)) (i32.const 0)) (then (unreachable))) + (if (i32.ne (call $ctor (i32.const 100)) (i32.const 1)) (then (unreachable))) ) (func (export "dealloc") - (call $drop (i32.const 0)) + (call $drop (i32.const 1)) ) ) (core instance $i (instantiate $m @@ -617,12 +617,12 @@ (call $drop2 (call $new2 (i32.const 104))) ;; should be referencing the same namespace - (if (i32.ne (call $new1 (i32.const 105)) (i32.const 0)) (then (unreachable))) - (if (i32.ne (call $new2 (i32.const 105)) (i32.const 1)) (then (unreachable))) + (if (i32.ne (call $new1 (i32.const 105)) (i32.const 1)) (then (unreachable))) + (if (i32.ne (call $new2 (i32.const 105)) (i32.const 2)) (then (unreachable))) ;; use different drops out of order - (call $drop2 (i32.const 0)) - (call $drop1 (i32.const 1)) + (call $drop2 (i32.const 1)) + (call $drop1 (i32.const 2)) ) (start $start) @@ -701,8 +701,8 @@ (local.set $r2 (call $new2 (i32.const 200))) ;; both should be index 0 - (if (i32.ne (local.get $r1) (i32.const 0)) (then (unreachable))) - (if (i32.ne (local.get $r2) (i32.const 0)) (then (unreachable))) + (if (i32.ne (local.get $r1) (i32.const 1)) (then (unreachable))) + (if (i32.ne (local.get $r2) (i32.const 1)) (then (unreachable))) ;; nothing should be dropped yet (if (i32.ne (call $drops) (i32.const 0)) (then (unreachable))) @@ -866,7 +866,7 @@ ;; table should be empty at this point, so a fresh allocation should get ;; index 0 - (if (i32.ne (call $ctor (i32.const 600)) (i32.const 0)) (then (unreachable))) + (if (i32.ne (call $ctor (i32.const 600)) (i32.const 1)) (then (unreachable))) ) (start $start) @@ -1024,8 +1024,8 @@ (local.set $r1 (call $ctor (i32.const 100))) (local.set $r2 (call $ctor (i32.const 200))) - (if (i32.ne (local.get $r1) (i32.const 0)) (then (unreachable))) - (if (i32.ne (local.get $r2) (i32.const 1)) (then (unreachable))) + (if (i32.ne (local.get $r1) (i32.const 1)) (then (unreachable))) + (if (i32.ne (local.get $r2) (i32.const 2)) (then (unreachable))) (call $assert-borrow (local.get $r2) (i32.const 200)) (call $assert-borrow (local.get $r1) (i32.const 100))