diff --git a/crates/wasmtime/src/runtime/component/concurrent.rs b/crates/wasmtime/src/runtime/component/concurrent.rs index 9dce2129d0..7fb5411452 100644 --- a/crates/wasmtime/src/runtime/component/concurrent.rs +++ b/crates/wasmtime/src/runtime/component/concurrent.rs @@ -946,14 +946,23 @@ impl ComponentInstance { Ok(()) } - fn queue_call( + /// Add the specified guest call to the `Self::high_priority` queue, to be + /// started as soon as backpressure and/or reentrance rules allow. + /// + /// SAFETY: `self` must belong to a store whose data type parameter is `T`, + /// and the caller must confer exclusive access to that store. Likewise, + /// when the closures queued by this function are run, the same rules apply + /// to their `&mut ComponentInstance` parameters. + // TODO: This should queue `impl UnsafeFnOnce` "closures" (where + // `UnsafeFnOnce` would need to be a trait we define ourselves, since + // there's no standard equivalent) rather than `impl FnOnce` closures. That + // would force the caller to use an unsafe block and (hopefully) uphold the + // contract we've described above. + unsafe fn queue_call( &mut self, guest_task: TableId, async_: bool, - call: impl FnOnce( - StoreContextMut, - &mut ComponentInstance, - ) -> Result<[MaybeUninit; MAX_FLAT_PARAMS]> + call: impl FnOnce(&mut ComponentInstance) -> Result<[MaybeUninit; MAX_FLAT_PARAMS]> + Send + Sync + 'static, @@ -966,20 +975,6 @@ impl ComponentInstance { assert!(async_); Box::new(move |instance: &mut ComponentInstance| { - // SAFETY: This `ComponentInstance` belongs to the store in - // which it resides, so if it is valid then so is its store. - // Furthermore, this closure is only called (transitively) from - // `ComponentInstance::poll_until`, which has exclusive access - // to both the `ComponentInstance` and the store. - // - // In addition, the store's data type is known to be `T` because - // this closure will have been called with the same - // `ComponentInstance` that was passed to the outer `queue_call` - // scope. See - // `ComponentInstance::{poll_until,handle_work_item,handle_guest_call}`, - // where we pop the work item containing this closure and pass - // it the same `ComponentInstance`. - let mut store = unsafe { StoreContextMut(&mut *instance.store().cast()) }; let old_task = instance.guest_task().replace(guest_task); log::trace!( "stackless call: replaced {old_task:?} with {guest_task:?} as current task" @@ -989,7 +984,13 @@ impl ComponentInstance { instance.enter_instance(callee_instance); - let storage = call(store.as_context_mut(), instance)?; + // SAFETY: See the documentation for `make_call` to review the + // contract we must uphold for `call` here. + // + // Per the contract described in the `queue_call` documentation, + // we can rely on exclusive access to the store, whose data type + // parameter is `T`. + let storage = call(instance)?; instance.exit_instance(callee_instance)?; @@ -1015,9 +1016,6 @@ impl ComponentInstance { }) as Box Result<()> + Send + Sync> } else { Box::new(move |instance: &mut ComponentInstance| { - // SAFETY: See comment in the closure created in the - // `callback.is_some()` case above. - let mut store = unsafe { StoreContextMut::(&mut *instance.store().cast()) }; let old_task = instance.guest_task().replace(guest_task); log::trace!( "stackful call: replaced {old_task:?} with {guest_task:?} as current task", @@ -1031,7 +1029,10 @@ impl ComponentInstance { instance.enter_instance(callee_instance); } - let storage = call(store.as_context_mut(), instance)?; + // SAFETY: Per the contract described in the `queue_call` + // documentation, we can rely on exclusive access to the store, + // whose data type parameter is `T`. + let storage = call(instance)?; if async_ { if instance.get(guest_task)?.lift_result.is_some() { @@ -1062,6 +1063,14 @@ impl ComponentInstance { 1 => unsafe { storage[0].assume_init() }, _ => unreachable!(), }; + + // SAFETY: Per the contract described in the + // `queue_call` documentation, we can rely on exclusive + // access to the store, whose data type parameter is + // `T`. + let mut store = + unsafe { StoreContextMut::(&mut *instance.store().cast()) }; + // SAFETY: `func` is a valid `*mut VMFuncRef` from // either `wasmtime-cranelift`-generated fused adapter // code or `component::Options`. Per `wasmparser` @@ -1096,7 +1105,13 @@ impl ComponentInstance { Ok(()) } - fn enter_call( + /// Prepare (but do not start) a guest->guest call. + /// + /// SAFETY: All the pointer arguments must be valid pointers to guest + /// entities. In addition the caller must confer exclusive access to the + /// store to which the passed `&mut ComponentInstance` belongs, which must + /// have a data type parameter of `T`. + unsafe fn prepare_call( &mut self, start: *mut VMFuncRef, return_: *mut VMFuncRef, @@ -1141,8 +1156,8 @@ impl ComponentInstance { // // In addition, the store's data type is known to be `T` because // this closure will have been called with the same - // `ComponentInstance` that was passed to the outer `enter_call` - // scope. See + // `ComponentInstance` that was passed to the outer + // `prepare_call` scope. See // `ComponentInstance::{poll_until,handle_work_item,handle_guest_call}`, // where we pop the work item containing this closure and pass // it the same `ComponentInstance`. @@ -1269,7 +1284,7 @@ impl ComponentInstance { // // In addition, the store's data type is known to be `T` because this // function will have been called with the same `ComponentInstance` that - // was in scope in `ComponentInstance::exit_call` or `prepare_call` -- + // was in scope in `ComponentInstance::start_call` or `prepare_call` -- // the two functions where this function is monomophized and stored as // an `fn(..)` in `GuestTask::callback`. See // `ComponentInstance::{poll_until,handle_work_item,handle_guest_call}`, @@ -1300,7 +1315,14 @@ impl ComponentInstance { Ok(params[0].get_u32()) } - fn exit_call( + /// Start a guest->guest call previously prepared using + /// `Self::prepare_call`. + /// + /// SAFETY: All the pointer arguments must be valid pointers to guest + /// entities. In addition the caller must confer exclusive access to the + /// store to which the passed `&mut ComponentInstance` belongs, which must + /// have a data type parameter of `T`. + unsafe fn start_call( &mut self, callback: *mut VMFuncRef, post_return: *mut VMFuncRef, @@ -1331,7 +1353,7 @@ impl ComponentInstance { instance, } = &task.caller else { - // As of this writing, `exit_call` is only used for + // As of this writing, `start_call` is only used for // guest->guest calls. unreachable!() }; @@ -1340,26 +1362,31 @@ impl ComponentInstance { let callee_instance = task.instance; - let call = make_call::( - guest_task, - callee, - param_count, - result_count, - if callback.is_null() { - None - } else { - Some(self.instance_flags(callee_instance)) - }, - ); + // SAFETY: Per the contract described in this function's documentation, + // we can rely on exclusive access to the store, whose data type + // parameter is `T`. + unsafe { + let call = make_call::( + guest_task, + callee, + param_count, + result_count, + if callback.is_null() { + None + } else { + Some(self.instance_flags(callee_instance)) + }, + ); - self.queue_call( - guest_task, - (flags & EXIT_FLAG_ASYNC_CALLEE) != 0, - call, - NonNull::new(callback).map(SendSyncPtr::new), - NonNull::new(post_return).map(SendSyncPtr::new), - result_count, - )?; + self.queue_call::( + guest_task, + (flags & EXIT_FLAG_ASYNC_CALLEE) != 0, + call, + NonNull::new(callback).map(SendSyncPtr::new), + NonNull::new(post_return).map(SendSyncPtr::new), + result_count, + )?; + } let set = self.get_mut(caller)?.sync_call_set; Waitable::Guest(guest_task).join(self, Some(set))?; @@ -1409,7 +1436,6 @@ impl ComponentInstance { /// transitively) using `ComponentInstance::poll_until`. pub(crate) unsafe fn wrap_call( &mut self, - store: StoreContextMut, closure: Arc, params: P, ) -> Pin> + Send + 'static>> @@ -1431,7 +1457,7 @@ impl ComponentInstance { let mut accessor = unsafe { Accessor::new(get_host_and_store, spawn_task, self.instance()) }; let mut future = Box::pin(async move { closure(&mut accessor, params).await }); - let store = VMStoreRawPtr(store.traitobj()); + let store = VMStoreRawPtr(NonNull::new(self.store()).unwrap()); let instance = SendSyncPtr::new(NonNull::new(self).unwrap()); // SAFETY: `poll_with_state` will populate and reset the thread-local // state as described above. @@ -1458,9 +1484,14 @@ impl ComponentInstance { } } - pub(crate) fn first_poll( + /// Poll the specified future once, and if it returns `Pending`, add it to + /// the set of futures to be polled as part of this instance's event loop + /// until it completes. + /// + /// SAFETY: `self` must belong to a store whose data type parameter is `T`, + /// and the caller must confer exclusive access to that store. + pub(crate) unsafe fn first_poll( &mut self, - store: StoreContextMut, future: impl Future> + Send + 'static, caller_instance: RuntimeComponentInstanceIndex, lower: impl FnOnce(StoreContextMut, R) -> Result<()> + Send + 'static, @@ -1490,7 +1521,8 @@ impl ComponentInstance { call_context: &mut Option, task: TableId, ) { - // SAFETY: See SAFETY comment in above in wrapping function. + // SAFETY: See SAFETY comment in above in `first_poll`, as well as + // the precondition documented for that function. let store = unsafe { StoreContextMut::(&mut *store.0.as_ptr().cast()) }; if let Some(call_context) = call_context.take() { log::trace!("push call context for {task:?}"); @@ -1504,13 +1536,14 @@ impl ComponentInstance { task: TableId, ) { log::trace!("pop call context for {task:?}"); - // SAFETY: See SAFETY comment in above in wrapping function. + // SAFETY: See SAFETY comment in above in `first_poll`, as well as + // the precondition documented for that function. let store = unsafe { StoreContextMut::(&mut *store.0.as_ptr().cast()) }; *call_context = Some(store.0.component_resource_state().0.pop().unwrap()); } let future = future::poll_fn({ - let store = VMStoreRawPtr(store.0.traitobj()); + let store = VMStoreRawPtr(NonNull::new(self.store()).unwrap()); let mut call_context = None; move |cx| { let mut wrapped = wrapped.try_lock().unwrap(); @@ -1545,7 +1578,8 @@ impl ComponentInstance { let mut future = Box::pin(future.map(move |result| { if let Some(result) = result { HostTaskOutput::Function(Box::new(move |instance| { - // SAFETY: See SAFETY comment in above in wrapping function. + // SAFETY: See SAFETY comment in above in `first_poll`, as + // well as the precondition documented for that function. let store = unsafe { StoreContextMut(&mut *instance.store().cast()) }; lower(store, result?)?; instance.get_mut(task)?.abort_handle.take(); @@ -2766,10 +2800,21 @@ impl Instance { /// Trait representing component model ABI async intrinsics and fused adapter /// helper functions. +/// +/// SAFETY (callers): Most of the methods in this trait accept raw pointers, +/// which must be valid for at least the duration of the call. In addition, the +/// `&mut ComponentInstance` parameter must point to a `ComponentInstance` that +/// belongs to `self` (i.e. both should have been derived from the same +/// `VMComponentContext`). +/// +/// SAFETY (implementors): Care must be taken to treat the `&mut Self` and `&mut +/// ComponentInstance` parameters as if they were disjoint borrows and not +/// create aliases by retrieving a `*mut ComponentInstance` from `self` and +/// converting it to a `&mut ComponentInstance`. pub unsafe trait VMComponentAsyncStore { /// A helper function for fused adapter modules involving calls where the /// caller is sync-lowered but the callee is async-lifted. - fn sync_enter( + unsafe fn sync_enter( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -2786,7 +2831,7 @@ pub unsafe trait VMComponentAsyncStore { /// A helper function for fused adapter modules involving calls where the /// caller is sync-lowered but the callee is async-lifted. - fn sync_exit( + unsafe fn sync_exit( &mut self, instance: &mut ComponentInstance, callback: *mut VMFuncRef, @@ -2798,7 +2843,7 @@ pub unsafe trait VMComponentAsyncStore { /// A helper function for fused adapter modules involving calls where the /// caller is async-lowered. - fn async_enter( + unsafe fn async_enter( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -2814,7 +2859,7 @@ pub unsafe trait VMComponentAsyncStore { /// A helper function for fused adapter modules involving calls where the /// caller is async-lowered. - fn async_exit( + unsafe fn async_exit( &mut self, instance: &mut ComponentInstance, callback: *mut VMFuncRef, @@ -2826,7 +2871,7 @@ pub unsafe trait VMComponentAsyncStore { ) -> Result; /// The `future.write` intrinsic. - fn future_write( + unsafe fn future_write( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -2839,7 +2884,7 @@ pub unsafe trait VMComponentAsyncStore { ) -> Result; /// The `future.read` intrinsic. - fn future_read( + unsafe fn future_read( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -2852,7 +2897,7 @@ pub unsafe trait VMComponentAsyncStore { ) -> Result; /// The `stream.write` intrinsic. - fn stream_write( + unsafe fn stream_write( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -2866,7 +2911,7 @@ pub unsafe trait VMComponentAsyncStore { ) -> Result; /// The `stream.read` intrinsic. - fn stream_read( + unsafe fn stream_read( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -2881,7 +2926,7 @@ pub unsafe trait VMComponentAsyncStore { /// The "fast-path" implementation of the `stream.write` intrinsic for /// "flat" (i.e. memcpy-able) payloads. - fn flat_stream_write( + unsafe fn flat_stream_write( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -2897,7 +2942,7 @@ pub unsafe trait VMComponentAsyncStore { /// The "fast-path" implementation of the `stream.read` intrinsic for "flat" /// (i.e. memcpy-able) payloads. - fn flat_stream_read( + unsafe fn flat_stream_read( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -2912,7 +2957,7 @@ pub unsafe trait VMComponentAsyncStore { ) -> Result; /// The `error-context.debug-message` intrinsic. - fn error_context_debug_message( + unsafe fn error_context_debug_message( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -2924,8 +2969,9 @@ pub unsafe trait VMComponentAsyncStore { ) -> Result<()>; } +/// SAFETY: See trait docs. unsafe impl VMComponentAsyncStore for StoreInner { - fn sync_enter( + unsafe fn sync_enter( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -2939,7 +2985,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { storage: *mut ValRaw, storage_len: usize, ) -> Result<()> { - instance.enter_call::( + instance.prepare_call::( start, return_, caller_instance, @@ -2957,7 +3003,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { ) } - fn sync_exit( + unsafe fn sync_exit( &mut self, instance: &mut ComponentInstance, callback: *mut VMFuncRef, @@ -2967,7 +3013,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { storage_len: usize, ) -> Result<()> { instance - .exit_call::( + .start_call::( callback, ptr::null_mut(), callee, @@ -2982,7 +3028,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { .map(drop) } - fn async_enter( + unsafe fn async_enter( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -2995,7 +3041,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { params: u32, results: u32, ) -> Result<()> { - instance.enter_call::( + instance.prepare_call::( start, return_, caller_instance, @@ -3007,7 +3053,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { ) } - fn async_exit( + unsafe fn async_exit( &mut self, instance: &mut ComponentInstance, callback: *mut VMFuncRef, @@ -3017,7 +3063,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { result_count: u32, flags: u32, ) -> Result { - instance.exit_call::( + instance.start_call::( callback, post_return, callee, @@ -3028,7 +3074,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { ) } - fn future_write( + unsafe fn future_write( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -3060,7 +3106,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { } } - fn future_read( + unsafe fn future_read( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -3089,7 +3135,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { } } - fn stream_write( + unsafe fn stream_write( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -3119,7 +3165,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { } } - fn stream_read( + unsafe fn stream_read( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -3149,7 +3195,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { } } - fn flat_stream_write( + unsafe fn flat_stream_write( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -3183,7 +3229,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { } } - fn flat_stream_read( + unsafe fn flat_stream_read( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -3217,7 +3263,7 @@ unsafe impl VMComponentAsyncStore for StoreInner { } } - fn error_context_debug_message( + unsafe fn error_context_debug_message( &mut self, instance: &mut ComponentInstance, memory: *mut VMMemoryDefinition, @@ -3905,10 +3951,14 @@ fn check_recursive_run() { }); } +/// Run the specified function on a newly-created fiber and `.await` its +/// completion. pub(crate) async fn on_fiber( store: StoreContextMut<'_, T>, func: impl FnOnce(&mut StoreContextMut) -> R + Send, ) -> Result { + // SAFETY: The returned future closes over `store`, so the borrow checker + // will ensure the requirements of `on_fiber_raw` are met. unsafe { on_fiber_raw(VMStoreRawPtr(store.traitobj()), move |store| { func(&mut StoreContextMut(&mut *store.cast())) @@ -3917,11 +3967,15 @@ pub(crate) async fn on_fiber( } } +/// Same as `on_fiber`, but accepts a `&mut StoreOpaque` instead of a +/// `StoreContextMut`. #[cfg(feature = "gc")] pub(crate) async fn on_fiber_opaque( store: &mut StoreOpaque, func: impl FnOnce(&mut StoreOpaque) -> R + Send, ) -> Result { + // SAFETY: The returned future closes over `store`, so the borrow checker + // will ensure the requirements of `on_fiber_raw` are met. unsafe { on_fiber_raw(VMStoreRawPtr(store.traitobj()), move |store| { func((*store).store_opaque_mut()) @@ -3930,6 +3984,12 @@ pub(crate) async fn on_fiber_opaque( } } +/// Wrap the specified function in a fiber and return it. +/// +/// SAFETY: `store` must be a valid pointer to a store and remain so for as long +/// as the returned fiber exists. Furthermore, the resuming thread(s) must +/// confer exclusive access to that fiber until it is either dropped, resolved, +/// or forgotten. unsafe fn prepare_fiber<'a, R: Send + 'static>( store: VMStoreRawPtr, func: impl FnOnce(*mut dyn VMStore) -> R + Send + 'a, @@ -3944,6 +4004,13 @@ unsafe fn prepare_fiber<'a, R: Send + 'static>( Ok((fiber, rx)) } +/// Run the specified function on a newly-created fiber and `.await` its +/// completion. +/// +/// SAFETY: `store` must be a valid pointer to a store and remain so for as long +/// as the returned future exists. Furthermore, the polling thread(s) must +/// confer exclusive access to that future until it is either dropped, resolved, +/// or forgotten. async unsafe fn on_fiber_raw( store: VMStoreRawPtr, func: impl FnOnce(*mut dyn VMStore) -> R + Send, @@ -4006,6 +4073,9 @@ impl StoreFiber<'_> { impl Drop for StoreFiber<'_> { fn drop(&mut self) { if !self.fiber.as_ref().unwrap().done() { + // Safety: `self` is a valid pointer and we pass a `store` + // parameter of `None` to inform the resumed fiber that it does + // _not_ have access to the store. let result = unsafe { resume_fiber_raw(self, None, Err(anyhow!("future dropped"))) }; debug_assert!(result.is_ok()); } @@ -4135,26 +4205,46 @@ pub(crate) enum WaitableCheck { Yield, } -fn make_call( +/// Return a closure which will call the specified function in the scope of the +/// specified task. +/// +/// This will use `GuestTask::lower_params` to lower the parameters, but will +/// not lift the result; instead, it returns a `[MaybeUninit; +/// MAX_FLAT_PARAMS]` from which the result, if any, may be lifted. Note that +/// an async-lifted export will have returned its result using the `task.return` +/// intrinsic (or not returned a result at all, in the case of `task.cancel`), +/// in which case the "result" of this call will either be a callback code or +/// nothing. +/// +/// SAFETY: `callee` must be a valid `*mut VMFuncRef` at the time when the +/// returned closure is called. In addition the caller must confer exclusive +/// access to the store to which the passed `&mut ComponentInstance` belongs, +/// which must have a data type parameter of `T`. +// TODO: This should return a `impl UnsafeFnOnce` (where `UnsafeFnOnce` would +// need to be a trait we define ourselves, since there's no standard equivalent) +// rather than a `impl FnOnce`. That would force the caller to use an unsafe +// block and (hopefully) uphold the contract we've described above. +unsafe fn make_call( guest_task: TableId, callee: SendSyncPtr, param_count: usize, result_count: usize, flags: Option, -) -> impl FnOnce( - StoreContextMut, - &mut ComponentInstance, -) -> Result<[MaybeUninit; MAX_FLAT_PARAMS]> +) -> impl FnOnce(&mut ComponentInstance) -> Result<[MaybeUninit; MAX_FLAT_PARAMS]> + Send + Sync + 'static { - move |mut store: StoreContextMut, instance: &mut ComponentInstance| { + move |instance: &mut ComponentInstance| { let mut storage = [MaybeUninit::uninit(); MAX_FLAT_PARAMS]; let lower = instance.get_mut(guest_task)?.lower_params.take().unwrap(); lower(instance, &mut storage[..param_count])?; + // SAFETY: Per the contract documented above, `callee` is a valid + // pointer and we have exclusive access to the store this instance + // belongs to, which has a data type parameter of `T`. unsafe { + let mut store = StoreContextMut::(&mut *instance.store().cast()); if let Some(mut flags) = flags { flags.set_may_enter(false); } @@ -4184,8 +4274,10 @@ impl<'a> Drop for ResetPtr<'a> { } } -// SAFETY: The specified pointer must be a valid `*mut T` which was originally -// allocated as a `Box`. +/// Drop the specified pointer as a `Box`. +/// +/// SAFETY: The specified pointer must be a valid `*mut T` which was originally +/// allocated as a `Box`. pub(crate) unsafe fn drop_params(pointer: *mut u8) { drop(unsafe { Box::from_raw(pointer as *mut T) }) } @@ -4392,29 +4484,33 @@ fn start_call( log::trace!("starting call {guest_task:?}"); - let call = make_call::( - guest_task, - SendSyncPtr::new(callee), - param_count, - 1, - if callback.is_none() { - None - } else { - Some(instance.instance_flags(component_instance)) - }, - ); - *instance.guest_task() = Some(guest_task); log::trace!("pushed {guest_task:?} as current task; old task was None"); - instance.queue_call( - guest_task, - is_concurrent, - call, - callback.map(SendSyncPtr::new), - post_return.map(|f| SendSyncPtr::new(f.func_ref)), - 1, - )?; + // SAFETY: We have exclusive access to the store, whose data type parameter + // is `T`, and `instance` was extracted from it above. + unsafe { + let call = make_call::( + guest_task, + SendSyncPtr::new(callee), + param_count, + 1, + if callback.is_none() { + None + } else { + Some(instance.instance_flags(component_instance)) + }, + ); + + instance.queue_call::( + guest_task, + is_concurrent, + call, + callback.map(SendSyncPtr::new), + post_return.map(|f| SendSyncPtr::new(f.func_ref)), + 1, + )?; + } *instance.guest_task() = None; log::trace!("popped current task {guest_task:?}; new task is None"); diff --git a/crates/wasmtime/src/runtime/component/concurrent/futures_and_streams/buffers.rs b/crates/wasmtime/src/runtime/component/concurrent/futures_and_streams/buffers.rs index 4472a16ab0..f3cda4dac8 100644 --- a/crates/wasmtime/src/runtime/component/concurrent/futures_and_streams/buffers.rs +++ b/crates/wasmtime/src/runtime/component/concurrent/futures_and_streams/buffers.rs @@ -27,6 +27,10 @@ pub trait ReadBuffer: Send + Sync + 'static { /// Number of items which may be read before this buffer is full. fn remaining_capacity(&self) -> usize; /// Move (i.e. take ownership of) the specified items into this buffer. + /// + /// SAFETY: `input` must be a valid `*const T` array of `count` items on + /// entry. Those items will be invalidated on exit and must be forgotten + /// using e.g. `mem::forget` to avoid unsoundness. unsafe fn copy_from(&mut self, input: *const T, count: usize); } @@ -41,6 +45,8 @@ impl> Extend for Extender<'_, B> { impl WriteBuffer for Option { fn remaining(&self) -> &[T] { if let Some(me) = self { + // SAFETY: This effectively transmutes a `&T` to a `&[T; 1]`, which + // should be sound. unsafe { slice::from_raw_parts(me, 1) } } else { &[] @@ -87,6 +93,7 @@ impl ReadBuffer for Option { } } + /// SAFETY: See trait docs. unsafe fn copy_from(&mut self, input: *const T, count: usize) { match count { 0 => {} @@ -126,11 +133,15 @@ impl VecBuffer { } fn remaining_(&self) -> &[T] { + // SAFETY: This relies on the invariant (upheld in the other methods of + // this type) that all the elements from `self.offset` onward are + // initialized and valid for `self.buffer`. unsafe { mem::transmute::<&[MaybeUninit], &[T]>(&self.buffer[self.offset..]) } } fn skip_(&mut self, count: usize) { assert!(count <= self.remaining_().len()); + // SAFETY: See comment in `Self::remaining_` for item in &mut self.buffer[self.offset..][..count] { drop(unsafe { item.as_mut_ptr().read() }) } @@ -156,6 +167,8 @@ impl WriteBuffer for VecBuffer { impl From> for VecBuffer { fn from(buffer: Vec) -> Self { Self { + // SAFETY: Transmuting from `Vec` to `Vec>` should + // be sound for any `T`. buffer: unsafe { mem::transmute::, Vec>>(buffer) }, offset: 0, } @@ -177,6 +190,7 @@ impl ReadBuffer for Vec { self.capacity().checked_sub(self.len()).unwrap() } + /// SAFETY: See trait docs. unsafe fn copy_from(&mut self, input: *const T, count: usize) { assert!(count <= self.remaining_capacity()); ptr::copy(input, self.as_mut_ptr().add(self.len()), count); @@ -231,6 +245,7 @@ impl ReadBuffer for BytesMut { self.capacity().checked_sub(self.len()).unwrap() } + /// SAFETY: See trait docs. unsafe fn copy_from(&mut self, input: *const u8, count: usize) { assert!(count <= self.remaining_capacity()); ptr::copy(input, self.as_mut_ptr().add(self.len()), count); diff --git a/crates/wasmtime/src/runtime/component/func.rs b/crates/wasmtime/src/runtime/component/func.rs index edae9fe31b..e230b0ec4e 100644 --- a/crates/wasmtime/src/runtime/component/func.rs +++ b/crates/wasmtime/src/runtime/component/func.rs @@ -779,8 +779,15 @@ impl Func { } } + /// Equivalent to `lower_args`, but with a monomorphic signature + /// suitable for use with `concurrent::prepare_call`. + /// + /// SAFETY: `store` must be a valid pointer to a store with data type + /// parameter `T`, and the caller must confer exclusive access to that + /// store. Also, `params_in` must be a valid pointer to a + /// `MaybeUninit<[ValRaw; MAX_FLAT_PARAMS]>`. #[cfg(feature = "component-model-async")] - fn lower_args_fn( + unsafe fn lower_args_fn( func: Func, store: *mut dyn VMStore, params_in: *mut u8, @@ -790,6 +797,8 @@ impl Func { store, params_out, func, + // SAFETY: Per this function's precondition, `params_in` is a valid + // pointer to a `Self::Params`. unsafe { &*params_in.cast() }, Self::lower_args::, ) @@ -822,8 +831,14 @@ impl Func { Self::lift_results(cx, results_ty, src, false) } + /// Equivalent to `lift_results_sync`, but with a monomorphic signature + /// suitable for use with `concurrent::prepare_call`. + /// + /// SAFETY: `store` must be a valid pointer to a store with data type + /// parameter `T`, and the caller must confer exclusive access to that + /// store. #[cfg(feature = "component-model-async")] - fn lift_results_sync_fn( + unsafe fn lift_results_sync_fn( func: Func, store: *mut dyn VMStore, results: &[ValRaw], @@ -840,8 +855,14 @@ impl Func { Self::lift_results(cx, results_ty, src, true) } + /// Equivalent to `lift_results_async`, but with a monomorphic signature + /// suitable for use with `concurrent::prepare_call`. + /// + /// SAFETY: `store` must be a valid pointer to a store with data type + /// parameter `T`, and the caller must confer exclusive access to that + /// store. #[cfg(feature = "component-model-async")] - fn lift_results_async_fn( + unsafe fn lift_results_async_fn( func: Func, store: *mut dyn VMStore, results: &[ValRaw], @@ -906,8 +927,12 @@ impl Func { } } +/// Lower parameters of the specified type using the specified function. +/// +/// SAFETY: `store` must be a valid pointer to a store with data type parameter +/// `T`, and the caller must confer exclusive access to that store. #[cfg(feature = "component-model-async")] -fn lower_params< +unsafe fn lower_params< Params, LowerParams, T, @@ -928,7 +953,9 @@ fn lower_params< ) -> Result<()> { use crate::component::storage::slice_to_storage_mut; - let mut store = unsafe { StoreContextMut(&mut *store.cast()) }; + // SAFETY: Per the function contract documented above, we have exclusive + // access to the store and the data type parameters is `T`. + let mut store = unsafe { StoreContextMut::(&mut *store.cast()) }; let FuncData { options, instance, @@ -942,6 +969,9 @@ fn lower_params< let instance_ptr = instance.instance_ptr(); let mut flags = instance.instance().instance_flags(component_instance); + // SAFETY: We have exclusive access to the store, which we means we have + // exclusive access to any `ComponentInstance` which resides in the + // store, including the one we pass to `LowerContext::new` below. unsafe { if !flags.may_enter() { bail!(crate::Trap::CannotEnterComponent); @@ -967,8 +997,12 @@ fn lower_params< } } +/// Lift results of the specified type using the specified function. +/// +/// SAFETY: `store` must be a valid pointer to a store with data type parameter +/// `T`, and the caller must confer exclusive access to that store. #[cfg(feature = "component-model-async")] -fn lift_results< +unsafe fn lift_results< Return: Send + Sync + 'static, T, F: FnOnce(&mut LiftContext, InterfaceType, &[ValRaw]) -> Result + Send + Sync, @@ -978,6 +1012,8 @@ fn lift_results< me: Func, lift: F, ) -> Result> { + // SAFETY: Per the function contract documented above, we have exclusive + // access to the store and the data type parameters is `T`. let store = unsafe { StoreContextMut::(&mut *store.cast()) }; let FuncData { options, @@ -990,6 +1026,9 @@ fn lift_results< let types = instance.component_types().clone(); let instance_ptr = instance.instance_ptr(); + // SAFETY: We have exclusive access to the store, which we means we have + // exclusive access to any `ComponentInstance` which resides in the store, + // including the one we pass to `LiftContext::new` below. unsafe { Ok(Box::new(lift( &mut LiftContext::new(store.0, &options, &types, instance_ptr), diff --git a/crates/wasmtime/src/runtime/component/func/host.rs b/crates/wasmtime/src/runtime/component/func/host.rs index bee5fd9e4f..8ed7502175 100644 --- a/crates/wasmtime/src/runtime/component/func/host.rs +++ b/crates/wasmtime/src/runtime/component/func/host.rs @@ -10,7 +10,7 @@ use crate::runtime::vm::component::{ }; use crate::runtime::vm::SendSyncPtr; use crate::runtime::vm::{VMFuncRef, VMGlobalDefinition, VMMemoryDefinition, VMOpaqueContext}; -use crate::{AsContextMut, CallHook, StoreContextMut, ValRaw}; +use crate::{CallHook, StoreContextMut, ValRaw}; use alloc::sync::Arc; use core::any::Any; use core::future::Future; @@ -39,8 +39,7 @@ impl HostFunc { fn from_canonical(func: F) -> Arc where F: for<'a> Fn( - StoreContextMut<'a, T>, - *mut ComponentInstance, + &mut ComponentInstance, P, ) -> Pin> + Send + 'static>> + Send @@ -63,7 +62,14 @@ impl HostFunc { P: ComponentNamedList + Lift + Send + Sync + 'static, R: ComponentNamedList + Lower + Send + Sync + 'static, { - Self::from_canonical(move |store, _, params| { + Self::from_canonical::(move |instance, params| { + // SAFETY: This relies on `from_closure` being called from + // `LinkerInstance::<'_, T>::func_wrap`, ensuring it is only used + // with a store with a data type parameter of `T`. In addition, + // this closure will only be called via `Self::entrypoint`, which + // confers exclusive access to the store via its `VMOpaqueContext` + // parameter. + let store = unsafe { StoreContextMut(&mut *instance.store().cast()) }; let result = func(store, params); Box::pin(async move { result }) }) @@ -83,8 +89,14 @@ impl HostFunc { R: ComponentNamedList + Lower + Send + Sync + 'static, { let func = Arc::new(func); - Self::from_canonical(move |store, instance, params| unsafe { - (*instance).wrap_call(store, func.clone(), params) + Self::from_canonical::(move |instance, params| { + // SAFETY: This relies on `from_closure` being called from + // `LinkerInstance::<'_, T>::func_wrap_concurrent`, ensuring it is + // only used with a store with a data type parameter of `T`. In + // addition, this closure will only be called via + // `Self::entrypoint`, which confers exclusive access to the store + // via its `VMOpaqueContext` parameter. + unsafe { instance.wrap_call(func.clone(), params) } }) } @@ -103,8 +115,7 @@ impl HostFunc { ) -> bool where F: for<'a> Fn( - StoreContextMut<'a, T>, - *mut ComponentInstance, + &mut ComponentInstance, P, ) -> Pin> + Send + 'static>> + Send @@ -115,11 +126,10 @@ impl HostFunc { { let data = SendSyncPtr::new(NonNull::new(data.as_ptr() as *mut F).unwrap()); unsafe { - call_host_and_handle_result::(cx, |instance, types, store| { - call_host( + call_host_and_handle_result::(cx, |instance, types| { + call_host::( instance, types, - store, TypeFuncIndex::from_u32(ty), RuntimeComponentInstanceIndex::from_u32(caller_instance), InstanceFlags::from_raw(flags), @@ -128,7 +138,7 @@ impl HostFunc { StringEncoding::from_u8(string_encoding).unwrap(), async_ != 0, NonNull::slice_from_raw_parts(storage, storage_len).as_mut(), - move |store, instance, args| (*data.as_ptr())(store, instance, args), + move |instance, args| (*data.as_ptr())(instance, args), ) }) } @@ -137,8 +147,7 @@ impl HostFunc { fn new_dynamic_canonical(func: F) -> Arc where F: for<'a> Fn( - StoreContextMut<'a, T>, - *mut ComponentInstance, + &mut ComponentInstance, Vec, usize, ) @@ -161,10 +170,17 @@ impl HostFunc { where F: Fn(StoreContextMut<'_, T>, &[Val], &mut [Val]) -> Result<()> + Send + Sync + 'static, { - Self::new_dynamic_canonical(move |store, _, params: Vec, result_count| { + Self::new_dynamic_canonical::(move |instance, params: Vec, result_count| { let mut results = iter::repeat(Val::Bool(false)) .take(result_count) .collect::>(); + // SAFETY: This relies on `new_dynamic` being called from + // `LinkerInstance::<'_, T>::func_new`, ensuring it is only used + // with a store with a data type parameter of `T`. In addition, + // this closure will only be called via `dynamic_entrypoint`, which + // confers exclusive access to the store via its `VMOpaqueContext` + // parameter. + let store = unsafe { StoreContextMut(&mut *instance.store().cast()) }; let result = func(store, ¶ms, &mut results); let result = result.map(move |()| results); Box::pin(async move { result }) @@ -183,8 +199,8 @@ impl HostFunc { + 'static, { let func = Arc::new(func); - Self::new_dynamic_canonical(move |store, instance, params, _| unsafe { - (*instance).wrap_call(store, func.clone(), params) + Self::new_dynamic_canonical::(move |instance, params, _| unsafe { + (*instance).wrap_call(func.clone(), params) }) } @@ -235,9 +251,8 @@ where /// must be upheld. Generally that's done by ensuring this is only called from /// the select few places it's intended to be called from. unsafe fn call_host( - instance: *mut ComponentInstance, + instance: &mut ComponentInstance, types: &Arc, - mut cx: StoreContextMut<'_, T>, ty: TypeFuncIndex, caller_instance: RuntimeComponentInstanceIndex, mut flags: InstanceFlags, @@ -250,8 +265,7 @@ unsafe fn call_host( ) -> Result<()> where F: for<'a> Fn( - StoreContextMut<'a, T>, - *mut ComponentInstance, + &mut ComponentInstance, Params, ) -> Pin> + Send + 'static>> + Send @@ -279,7 +293,7 @@ where } let options = Options::new( - cx.0.id(), + (*instance.store()).store_opaque().id(), NonNull::new(memory), NonNull::new(realloc), string_encoding, @@ -305,20 +319,25 @@ where let retptr = storage[1].assume_init(); let params = { - let lift = &mut LiftContext::new(cx.0, &options, types, instance); + let lift = &mut LiftContext::new( + (*instance.store()).store_opaque_mut(), + &options, + types, + instance, + ); lift.enter_call(); let ptr = validate_inbounds::(lift.memory(), ¶mptr)?; Params::load(lift, param_tys, &lift.memory()[ptr..][..Params::SIZE32])? }; - let future = closure(cx.as_context_mut(), instance, params); + let future = closure(instance, params); - let task = (*instance).first_poll(cx.as_context_mut(), future, caller_instance, { + let instance_ptr = SendSyncPtr::new(NonNull::new(instance).unwrap()); + let task = instance.first_poll(future, caller_instance, { let types = types.clone(); - let instance = SendSyncPtr::new(NonNull::new(instance).unwrap()); - move |cx, ret: Return| { + move |cx: StoreContextMut, ret: Return| { flags.set_may_leave(false); - let mut lower = LowerContext::new(cx, &options, &types, instance.as_ptr()); + let mut lower = LowerContext::new(cx, &options, &types, instance_ptr.as_ptr()); let ptr = validate_inbounds::(lower.as_slice_mut(), &retptr)?; ret.store(&mut lower, result_tys, ptr)?; flags.set_may_leave(true); @@ -366,16 +385,26 @@ where Storage::Indirect(slice_to_storage_mut(storage).assume_init_ref()) } }; - let mut lift = LiftContext::new(cx.0, &options, types, instance); + let mut lift = LiftContext::new( + (*instance.store()).store_opaque_mut(), + &options, + types, + instance, + ); lift.enter_call(); let params = storage.lift_params(&mut lift, param_tys)?; - let future = closure(cx.as_context_mut(), instance, params); + let future = closure(instance, params); let ret = (*instance).poll_and_block(future, caller_instance)?; flags.set_may_leave(false); - let mut lower = LowerContext::new(cx, &options, types, instance); + let mut lower = LowerContext::new( + StoreContextMut::(&mut *instance.store().cast()), + &options, + types, + instance, + ); storage.lower_results(&mut lower, result_tys, ret)?; flags.set_may_leave(true); lower.exit_call()?; @@ -452,30 +481,25 @@ pub(crate) fn validate_inbounds(memory: &[u8], ptr: &ValRaw) - unsafe fn call_host_and_handle_result( cx: NonNull, - func: impl FnOnce( - *mut ComponentInstance, - &Arc, - StoreContextMut<'_, T>, - ) -> Result<()>, + func: impl FnOnce(&mut ComponentInstance, &Arc) -> Result<()>, ) -> bool { let cx = VMComponentContext::from_opaque(cx); - let instance = cx.as_ref().instance(); - let types = (*instance).component_types(); - let raw_store = (*instance).store(); - let mut store = StoreContextMut(&mut *raw_store.cast()); + let instance_ptr = cx.as_ref().instance(); + let types = (*instance_ptr).component_types(); + let instance = &mut *instance_ptr; + let store = unsafe { StoreContextMut::(&mut *instance.store().cast()) }; crate::runtime::vm::catch_unwind_and_record_trap(|| { store.0.call_hook(CallHook::CallingHost)?; - let res = func(instance, types, store.as_context_mut()); + let res = func(instance, types); store.0.call_hook(CallHook::ReturningFromHost)?; res }) } unsafe fn call_host_dynamic( - instance: *mut ComponentInstance, + instance: &mut ComponentInstance, types: &Arc, - mut store: StoreContextMut<'_, T>, ty: TypeFuncIndex, caller_instance: RuntimeComponentInstanceIndex, mut flags: InstanceFlags, @@ -488,8 +512,7 @@ unsafe fn call_host_dynamic( ) -> Result<()> where F: for<'a> Fn( - StoreContextMut<'a, T>, - *mut ComponentInstance, + &mut ComponentInstance, Vec, usize, ) -> Pin>> + Send + 'static>> @@ -498,7 +521,7 @@ where + 'static, { let options = Options::new( - store.0.id(), + (*instance.store()).store_opaque().id(), NonNull::new(memory), NonNull::new(realloc), string_encoding, @@ -527,7 +550,12 @@ where let retptr = storage[1].assume_init(); let params = { - let mut lift = &mut LiftContext::new(store.0, &options, types, instance); + let mut lift = &mut LiftContext::new( + (*instance.store()).store_opaque_mut(), + &options, + types, + instance, + ); lift.enter_call(); let mut offset = validate_inbounds_dynamic(¶m_tys.abi, lift.memory(), ¶mptr)?; @@ -543,45 +571,36 @@ where .collect::>>()? }; - let future = closure( - store.as_context_mut(), - instance, - params, - result_tys.types.len(), - ); + let future = closure(instance, params, result_tys.types.len()); + + let instance_ptr = SendSyncPtr::new(NonNull::new(instance).unwrap()); + let task = instance.first_poll(future, caller_instance, { + let types = types.clone(); + let result_tys = func_ty.results; + move |store: StoreContextMut, result_vals: Vec| { + let result_tys = &types[result_tys]; + if result_vals.len() != result_tys.types.len() { + bail!("result length mismatch"); + } + + flags.set_may_leave(false); - let task = - (*instance).first_poll(store.as_context_mut(), future, caller_instance, { - let types = types.clone(); - let instance = SendSyncPtr::new(NonNull::new(instance).unwrap()); - let result_tys = func_ty.results; - move |store, result_vals: Vec| { - let result_tys = &types[result_tys]; - if result_vals.len() != result_tys.types.len() { - bail!("result length mismatch"); - } - - flags.set_may_leave(false); - - let mut lower = - LowerContext::new(store, &options, &types, instance.as_ptr()); - let mut ptr = validate_inbounds_dynamic( - &result_tys.abi, - lower.as_slice_mut(), - &retptr, - )?; - for (val, ty) in result_vals.iter().zip(result_tys.types.iter()) { - let offset = types.canonical_abi(ty).next_field32_size(&mut ptr); - val.store(&mut lower, *ty, offset)?; - } - - flags.set_may_leave(true); - - lower.exit_call()?; - - Ok(()) + let mut lower = + LowerContext::new(store, &options, &types, instance_ptr.as_ptr()); + let mut ptr = + validate_inbounds_dynamic(&result_tys.abi, lower.as_slice_mut(), &retptr)?; + for (val, ty) in result_vals.iter().zip(result_tys.types.iter()) { + let offset = types.canonical_abi(ty).next_field32_size(&mut ptr); + val.store(&mut lower, *ty, offset)?; } - })?; + + flags.set_may_leave(true); + + lower.exit_call()?; + + Ok(()) + } + })?; let status = if let Some(task) = task { Status::Started.pack(Some(task)) @@ -599,7 +618,12 @@ where ); } } else { - let mut cx = LiftContext::new(store.0, &options, types, instance); + let mut cx = LiftContext::new( + (*instance.store()).store_opaque_mut(), + &options, + types, + instance, + ); cx.enter_call(); if let Some(param_count) = param_tys.abi.flat_count(MAX_FLAT_PARAMS) { // NB: can use `MaybeUninit::slice_assume_init_ref` when that's stable @@ -631,17 +655,17 @@ where ret_index = 1; }; - let future = closure( - store.as_context_mut(), - instance, - args, - result_tys.types.len(), - ); + let future = closure(instance, args, result_tys.types.len()); let result_vals = (*instance).poll_and_block(future, caller_instance)?; flags.set_may_leave(false); - let mut cx = LowerContext::new(store, &options, types, instance); + let mut cx = LowerContext::new( + StoreContextMut::(&mut *instance.store().cast()), + &options, + types, + instance, + ); if let Some(cnt) = result_tys.abi.flat_count(MAX_FLAT_RESULTS) { let mut dst = storage[..cnt].iter_mut(); for (val, ty) in result_vals.iter().zip(result_tys.types.iter()) { @@ -700,8 +724,7 @@ extern "C" fn dynamic_entrypoint( ) -> bool where F: for<'a> Fn( - StoreContextMut<'a, T>, - *mut ComponentInstance, + &mut ComponentInstance, Vec, usize, ) -> Pin>> + Send + 'static>> @@ -711,11 +734,10 @@ where { let data = SendSyncPtr::new(NonNull::new(data.as_ptr() as *mut F).unwrap()); unsafe { - call_host_and_handle_result(cx, |instance, types, store| { - call_host_dynamic( + call_host_and_handle_result::(cx, |instance, types| { + call_host_dynamic::( instance, types, - store, TypeFuncIndex::from_u32(ty), RuntimeComponentInstanceIndex::from_u32(caller_instance), InstanceFlags::from_raw(flags), @@ -724,9 +746,7 @@ where StringEncoding::from_u8(string_encoding).unwrap(), async_ != 0, NonNull::slice_from_raw_parts(storage, storage_len).as_mut(), - move |store, instance, params, results| { - (*data.as_ptr())(store, instance, params, results) - }, + move |instance, params, results| (*data.as_ptr())(instance, params, results), ) }) } diff --git a/crates/wasmtime/src/runtime/component/func/typed.rs b/crates/wasmtime/src/runtime/component/func/typed.rs index d5146aaf38..8af79c9109 100644 --- a/crates/wasmtime/src/runtime/component/func/typed.rs +++ b/crates/wasmtime/src/runtime/component/func/typed.rs @@ -463,8 +463,14 @@ where Ok(()) } + /// Equivalent to `lower_stack_args`, but with a monomorphic signature + /// suitable for use with `concurrent::prepare_call`. + /// + /// SAFETY: `store` must be a valid pointer to a store with data type + /// parameter `T`, and the caller must confer exclusive access to that + /// store. Also, `params_in` must be a valid pointer to a `Self::Params`. #[cfg(feature = "component-model-async")] - fn lower_stack_args_fn( + unsafe fn lower_stack_args_fn( func: Func, store: *mut dyn VMStore, params_in: *mut u8, @@ -474,6 +480,8 @@ where store, params_out, func, + // SAFETY: Per this function's precondition, `params_in` is a valid + // pointer to a `Self::Params`. unsafe { &*params_in.cast() }, Self::lower_stack_args::, ) @@ -516,8 +524,14 @@ where Ok(()) } + /// Equivalent to `lower_heap_args`, but with a monomorphic signature + /// suitable for use with `concurrent::prepare_call`. + /// + /// SAFETY: `store` must be a valid pointer to a store with data type + /// parameter `T`, and the caller must confer exclusive access to that + /// store. Also, `params_in` must be a valid pointer to a `Self::Params`. #[cfg(feature = "component-model-async")] - fn lower_heap_args_fn( + unsafe fn lower_heap_args_fn( func: Func, store: *mut dyn VMStore, params_in: *mut u8, @@ -527,6 +541,8 @@ where store, params_out, func, + // SAFETY: Per this function's precondition, `params_in` is a valid + // pointer to a `Self::Params`. unsafe { &*params_in.cast() }, Self::lower_heap_args::, ) @@ -544,19 +560,40 @@ where Return::lift(cx, ty, dst) } + /// Equivalent to `lift_stack_result`, but with a partially-monomorphic + /// signature suitable for use with `super::lift_results`. #[cfg(feature = "component-model-async")] fn lift_stack_result_raw( cx: &mut LiftContext<'_>, ty: InterfaceType, dst: &[ValRaw], ) -> Result { + // SAFETY: Per the safety requiments documented for the `ComponentType` + // trait, `Return::Lower` must be compatible at the binary level with a + // `[ValRaw; N]`, where `N` is `mem::size_of::() / + // mem::size_of::()`. And since this function is only used when + // `Return::flatten_count() <= MAX_FLAT_RESULTS` and `MAX_FLAT_RESULTS + // == 1`, `N` can only either be 0 or 1. + // + // See `ComponentInstance::exit_call` for where we use the result count + // passed from `wasmtime_environ::fact::trampoline`-generated code to + // ensure the slice has the correct length, and also + // `concurrent::start_call` for where we conservatively use a slice + // length of 1 unconditionally. Also note that, as of this writing + // `slice_to_storage` double-checks the slice length is sufficient. Self::lift_stack_result(cx, ty, unsafe { crate::component::storage::slice_to_storage(dst) }) } + /// Equivalent to `lift_stack_result`, but with a monomorphic signature + /// suitable for use with `concurrent::prepare_call`. + /// + /// SAFETY: `store` must be a valid pointer to a store with data type + /// parameter `T`, and the caller must confer exclusive access to that + /// store. #[cfg(feature = "component-model-async")] - fn lift_stack_result_fn( + unsafe fn lift_stack_result_fn( func: Func, store: *mut dyn VMStore, results: &[ValRaw], @@ -589,6 +626,8 @@ where Return::load(cx, ty, bytes) } + /// Equivalent to `lift_heap_result`, but with a partially-monomorphic + /// signature suitable for use with `super::lift_results`. #[cfg(feature = "component-model-async")] fn lift_heap_result_raw( cx: &mut LiftContext<'_>, @@ -598,8 +637,14 @@ where Self::lift_heap_result(cx, ty, &dst[0]) } + /// Equivalent to `lift_heap_result`, but with a monomorphic signature + /// suitable for use with `concurrent::prepare_call`. + /// + /// SAFETY: `store` must be a valid pointer to a store with data type + /// parameter `T`, and the caller must confer exclusive access to that + /// store. #[cfg(feature = "component-model-async")] - fn lift_heap_result_fn( + unsafe fn lift_heap_result_fn( func: Func, store: *mut dyn VMStore, results: &[ValRaw], diff --git a/crates/wasmtime/src/runtime/component/instance.rs b/crates/wasmtime/src/runtime/component/instance.rs index 3047d0ff97..c98be9d5a4 100644 --- a/crates/wasmtime/src/runtime/component/instance.rs +++ b/crates/wasmtime/src/runtime/component/instance.rs @@ -916,8 +916,6 @@ impl InstancePre { ); #[cfg(feature = "component-model-async")] { - // TODO: do we need to return the store here due to the possible - // invalidation of the reference we were passed? crate::component::concurrent::on_fiber(store, move |store| self.instantiate_impl(store)) .await? } @@ -947,6 +945,9 @@ impl InstancePre { store.0.push_component_instance(instance); #[cfg(feature = "component-model-async")] { + // SAFETY: We have exclusive access to the store, which we means we + // have exclusive access to any `ComponentInstance` which resides in + // the store. let reference = unsafe { &mut *store.0[instance.0].as_ref().unwrap().instance_ptr() }; reference.instance = Some(instance); }