diff --git a/crates/cranelift/src/compiler/component.rs b/crates/cranelift/src/compiler/component.rs index 265388be819b..34a7def1a459 100644 --- a/crates/cranelift/src/compiler/component.rs +++ b/crates/cranelift/src/compiler/component.rs @@ -713,7 +713,7 @@ impl<'a> TrampolineCompiler<'a> { |_, _| {}, ); } - Trampoline::ThreadIndex => { + Trampoline::ThreadIndex { .. } => { self.translate_libcall( host::thread_index, TrapSentinel::NegativeOne, @@ -1478,7 +1478,6 @@ impl<'a> TrampolineCompiler<'a> { let instance = match trampoline { // These intrinsics explicitly do not check the may-leave flag. Trampoline::ResourceRep { .. } - | Trampoline::ThreadIndex | Trampoline::BackpressureInc { .. } | Trampoline::BackpressureDec { .. } => return, @@ -1508,6 +1507,7 @@ impl<'a> TrampolineCompiler<'a> { | Trampoline::WaitableSetPoll { instance, .. } | Trampoline::WaitableSetDrop { instance } | Trampoline::WaitableJoin { instance } + | Trampoline::ThreadIndex { instance } | Trampoline::ThreadNewIndirect { instance, .. } | Trampoline::ThreadResumeLater { instance, .. } | Trampoline::ThreadSuspend { instance, .. } diff --git a/crates/environ/src/component/dfg.rs b/crates/environ/src/component/dfg.rs index 23bf095dffa7..e89fccf18e55 100644 --- a/crates/environ/src/component/dfg.rs +++ b/crates/environ/src/component/dfg.rs @@ -472,7 +472,9 @@ pub enum Trampoline { Trap, EnterSyncCall, ExitSyncCall, - ThreadIndex, + ThreadIndex { + instance: RuntimeComponentInstanceIndex, + }, ThreadNewIndirect { instance: RuntimeComponentInstanceIndex, start_func_ty_idx: ComponentTypeIndex, @@ -1156,7 +1158,9 @@ impl LinearizeDfg<'_> { Trampoline::Trap => info::Trampoline::Trap, Trampoline::EnterSyncCall => info::Trampoline::EnterSyncCall, Trampoline::ExitSyncCall => info::Trampoline::ExitSyncCall, - Trampoline::ThreadIndex => info::Trampoline::ThreadIndex, + Trampoline::ThreadIndex { instance } => info::Trampoline::ThreadIndex { + instance: *instance, + }, Trampoline::ThreadNewIndirect { instance, start_func_ty_idx, diff --git a/crates/environ/src/component/info.rs b/crates/environ/src/component/info.rs index e839f1d6e7a2..6f9386cc0a13 100644 --- a/crates/environ/src/component/info.rs +++ b/crates/environ/src/component/info.rs @@ -1109,7 +1109,10 @@ pub enum Trampoline { ExitSyncCall, /// Intrinsic used to implement the `thread.index` component model builtin. - ThreadIndex, + ThreadIndex { + /// The specific component instance which is calling the intrinsic. + instance: RuntimeComponentInstanceIndex, + }, /// Intrinsic used to implement the `thread.new-indirect` component model builtin. ThreadNewIndirect { @@ -1247,7 +1250,7 @@ impl Trampoline { Trap => format!("trap"), EnterSyncCall => format!("enter-sync-call"), ExitSyncCall => format!("exit-sync-call"), - ThreadIndex => format!("thread-index"), + ThreadIndex { .. } => format!("thread-index"), ThreadNewIndirect { .. } => format!("thread-new-indirect"), ThreadResumeLater { .. } => format!("thread-resume-later"), ThreadSuspend { .. } => format!("thread-suspend"), diff --git a/crates/environ/src/component/translate/inline.rs b/crates/environ/src/component/translate/inline.rs index cd4d4e13d1b1..d5e8c41379e1 100644 --- a/crates/environ/src/component/translate/inline.rs +++ b/crates/environ/src/component/translate/inline.rs @@ -1092,10 +1092,12 @@ impl<'a> Inliner<'a> { .push((*func, dfg::CoreDef::UnsafeIntrinsic(*func, intrinsic))); } ThreadIndex { func } => { - let index = self - .result - .trampolines - .push((*func, dfg::Trampoline::ThreadIndex)); + let index = self.result.trampolines.push(( + *func, + dfg::Trampoline::ThreadIndex { + instance: frame.instance, + }, + )); frame.funcs.push((*func, dfg::CoreDef::Trampoline(index))); } ThreadNewIndirect { diff --git a/crates/wasmtime/src/runtime/component/func/options.rs b/crates/wasmtime/src/runtime/component/func/options.rs index 6538ab0837cf..ce4e96566ba3 100644 --- a/crates/wasmtime/src/runtime/component/func/options.rs +++ b/crates/wasmtime/src/runtime/component/func/options.rs @@ -146,6 +146,17 @@ impl<'a, T: 'static> LowerContext<'a, T> { ) -> Result { assert!(self.allow_realloc); + // All calls to `realloc` options in the canonical ABI zero out the + // `context.{get,set}` slots for the duration of the call. This sort of + // fakes a "fresh thread" for each call, but this is the only observable + // state so nothing else needs adjusting. Note though that the original + // values are preserved still to get restored after this call. + #[cfg(feature = "component-model-async")] + let orig_context = core::mem::replace( + self.store.0.vm_store_context_mut().component_context_mut(), + Default::default(), + ); + let (component, store) = self.instance.component_and_store_mut(self.store.0); let instance = self.instance.id().get(store); let options = &component.env_component().options[self.options]; @@ -185,6 +196,14 @@ impl<'a, T: 'static> LowerContext<'a, T> { bail!("realloc return: beyond end of memory") } + // Note that this restoration isn't part of a `Drop` guard which works + // because once a component traps it's locked-down and inaccessible, so + // it's ok if this isn't restored. + #[cfg(feature = "component-model-async")] + { + *self.store.0.vm_store_context_mut().component_context_mut() = orig_context; + } + Ok(result) } diff --git a/tests/all/component_model/func.rs b/tests/all/component_model/func.rs index c6ba6d041f61..57281f1ecc5a 100644 --- a/tests/all/component_model/func.rs +++ b/tests/all/component_model/func.rs @@ -3150,110 +3150,6 @@ async fn thread_index_via_call(style: ApiStyle) -> Result<()> { Ok(()) } -#[tokio::test] -async fn thread_index_via_post_return_sync() -> Result<()> { - thread_index_via_post_return(ApiStyle::Sync).await -} - -#[tokio::test] -async fn thread_index_via_post_return_async() -> Result<()> { - thread_index_via_post_return(ApiStyle::Async).await -} - -#[tokio::test] -async fn thread_index_via_post_return_concurrent() -> Result<()> { - thread_index_via_post_return(ApiStyle::Concurrent).await -} - -async fn thread_index_via_post_return(style: ApiStyle) -> Result<()> { - let component = r#" -(component - (core module $m - (import "" "thread.index" (func $thread-index (result i32))) - (global $index (mut i32) (i32.const 0)) - (func (export "run") - (global.set $index (call $thread-index)) - (if (i32.eqz (global.get $index)) (then unreachable)) - ) - (func (export "run-post-return") - (local $index i32) - (local.set $index (call $thread-index)) - (if (i32.eqz (local.get $index)) (then unreachable)) - (if (i32.ne (local.get $index) (global.get $index)) (then unreachable)) - ) - ) - (core func $thread-index (canon thread.index)) - (core instance $m (instantiate $m (with "" (instance - (export "thread.index" (func $thread-index)) - )))) - (func (export "run") (canon lift (core func $m "run") (post-return (func $m "run-post-return")))) -) -"#; - let engine = Engine::new(&style.config())?; - let component = Component::new(&engine, component)?; - let mut store = Store::new(&engine, ()); - let linker = Linker::new(&engine); - let instance = style.instantiate(&mut store, &linker, &component).await?; - let run = instance.get_typed_func::<(), ()>(&mut store, "run")?; - style.call(&mut store, run, ()).await?; - Ok(()) -} - -#[tokio::test] -async fn thread_index_via_cabi_realloc_sync() -> Result<()> { - thread_index_via_cabi_realloc(ApiStyle::Sync).await -} - -#[tokio::test] -async fn thread_index_via_cabi_realloc_async() -> Result<()> { - thread_index_via_cabi_realloc(ApiStyle::Async).await -} - -#[tokio::test] -async fn thread_index_via_cabi_realloc_concurrent() -> Result<()> { - thread_index_via_cabi_realloc(ApiStyle::Concurrent).await -} - -async fn thread_index_via_cabi_realloc(style: ApiStyle) -> Result<()> { - let component = r#" -(component - (core module $m - (import "" "thread.index" (func $thread-index (result i32))) - (global $index (mut i32) (i32.const 0)) - (memory (export "memory") 1) - (func (export "realloc") (param i32 i32 i32 i32) (result i32) - (global.set $index (call $thread-index)) - (if (i32.eqz (global.get $index)) (then unreachable)) - (i32.const 100) - ) - (func (export "run") (param i32 i32) - (local $index i32) - (local.set $index (call $thread-index)) - (if (i32.eqz (local.get $index)) (then unreachable)) - (if (i32.ne (local.get $index) (global.get $index)) (then unreachable)) - ) - ) - (core func $thread-index (canon thread.index)) - (core instance $m (instantiate $m (with "" (instance - (export "thread.index" (func $thread-index)) - )))) - (func (export "run") (param "s" string) (canon lift - (core func $m "run") - (memory $m "memory") - (realloc (func $m "realloc")) - )) -) -"#; - let engine = Engine::new(&style.config())?; - let component = Component::new(&engine, component)?; - let mut store = Store::new(&engine, ()); - let linker = Linker::new(&engine); - let instance = style.instantiate(&mut store, &linker, &component).await?; - let run = instance.get_typed_func::<(String,), ()>(&mut store, "run")?; - style.call(&mut store, run, ("hola".to_string(),)).await?; - Ok(()) -} - #[tokio::test] async fn thread_index_via_resource_drop_sync() -> Result<()> { thread_index_via_resource_drop(ApiStyle::Sync).await diff --git a/tests/misc_testsuite/component-model-threading/threading-builtins.wast b/tests/misc_testsuite/component-model-threading/threading-builtins.wast index 9889c2b9ce18..c10fa3c5f9a8 100644 --- a/tests/misc_testsuite/component-model-threading/threading-builtins.wast +++ b/tests/misc_testsuite/component-model-threading/threading-builtins.wast @@ -101,7 +101,7 @@ (assert_return (invoke "run") (u32.const 42)) -;; Test that `thread.index` is exempt from may-leave checks +;; Test that `thread.index` is not exempt from may-leave checks (component (core func $thread.index (canon thread.index)) @@ -118,4 +118,33 @@ (canon lift (core func $dm "run") (post-return (func $dm "post-return")))) ) -(assert_return (invoke "run")) +(assert_trap (invoke "run") "cannot leave component instance") + +(component + (core module $m + (import "" "thread.index" (func $thread-index (result i32))) + (global $index (mut i32) (i32.const 0)) + (memory (export "memory") 1) + (func (export "realloc") (param i32 i32 i32 i32) (result i32) + (global.set $index (call $thread-index)) + (if (i32.eqz (global.get $index)) (then unreachable)) + (i32.const 100) + ) + (func (export "run") (param i32 i32) + (local $index i32) + (local.set $index (call $thread-index)) + (if (i32.eqz (local.get $index)) (then unreachable)) + (if (i32.ne (local.get $index) (global.get $index)) (then unreachable)) + ) + ) + (core func $thread-index (canon thread.index)) + (core instance $m (instantiate $m (with "" (instance + (export "thread.index" (func $thread-index)) + )))) + (func (export "run") (param "s" string) (canon lift + (core func $m "run") + (memory $m "memory") + (realloc (func $m "realloc")) + )) +) +(assert_trap (invoke "run" (str.const "x")) "cannot leave component instance") diff --git a/tests/misc_testsuite/component-model/async/task-builtins.wast b/tests/misc_testsuite/component-model/async/task-builtins.wast index 966ece64ba40..e266ef6e2185 100644 --- a/tests/misc_testsuite/component-model/async/task-builtins.wast +++ b/tests/misc_testsuite/component-model/async/task-builtins.wast @@ -180,8 +180,8 @@ call $backpressure.inc call $backpressure.dec - ;; context.get should be what was set in `realloc` - (if (i32.ne (call $context.get) (i32.const 100)) (then (unreachable))) + ;; context set in realloc should be separate from this thread's context + (if (i32.ne (call $context.get) (i32.const 0)) (then (unreachable))) ) ) (core instance $m (instantiate $M (with "" (instance @@ -416,7 +416,7 @@ (if (i32.ne (local.get 2) (i32.const 1)) (then (unreachable))) (if (i32.ne (local.get 3) (i32.const 2)) (then (unreachable))) - (if (i32.ne (call $context.get) (i32.const 400)) (then (unreachable))) + (if (i32.ne (call $context.get) (i32.const 0)) (then (unreachable))) (call $context.set (i32.const 500)) call $backpressure.inc @@ -448,11 +448,11 @@ (func (export "run") ;; set this tasks's context before calling $run, in calling $run the ;; runtime will then call `realloc` above for the string return value - ;; which should see our 400 value. That will then set 500 which we - ;; should then see after the return. + ;; which should NOT see our 400 value. That will then set 500 which we + ;; should NOT then see after the return. (call $context.set (i32.const 400)) (call $run (i32.const 20)) - (if (i32.ne (call $context.get) (i32.const 500)) (then (unreachable))) + (if (i32.ne (call $context.get) (i32.const 400)) (then (unreachable))) ) ) (core instance $m (instantiate $M (with "" (instance @@ -616,7 +616,7 @@ (if (i32.ne (local.get 3) (i32.const 2)) (then (unreachable))) call $context.get - i32.const 400 + i32.const 0 i32.ne if unreachable end @@ -663,15 +663,15 @@ (import "" "task.return" (func $task.return (param i32))) (import "" "memory" (memory 1)) - ;; Set context[0] to 400, then read the future, which should call realloc and set - ;; context[0] to 500, then check that we see that value. + ;; Set context[0] to 400, then read the future, which should call realloc + ;; and set context[0] to 500, then check that we DON'T see that value. (func (export "run-future") (param $fr i32) (result i32) (local $ret i32) (call $context.set (i32.const 400)) (local.set $ret (call $future.read (local.get $fr) (i32.const 40))) (if (i32.ne (i32.const 0 (; COMPLETED ;)) (local.get $ret)) (then (unreachable))) - (if (i32.ne (call $context.get) (i32.const 500)) (then (unreachable))) + (if (i32.ne (call $context.get) (i32.const 400)) (then (unreachable))) (call $task.return (i32.const 42)) (i32.const 0 (; EXIT ;)) @@ -684,7 +684,7 @@ (call $context.set (i32.const 400)) (local.set $ret (call $stream.read (local.get $sr) (i32.const 40) (i32.const 1))) (if (i32.ne (i32.const 0x10 (; COMPLETED | 1<<4 ;)) (local.get $ret)) (then (unreachable))) - (if (i32.ne (call $context.get) (i32.const 500)) (then (unreachable))) + (if (i32.ne (call $context.get) (i32.const 400)) (then (unreachable))) (call $task.return (i32.const 42)) (i32.const 0 (; EXIT ;))