diff --git a/crates/cranelift/src/compiler/component.rs b/crates/cranelift/src/compiler/component.rs index 25c815d6ef40..9688cb3b52a7 100644 --- a/crates/cranelift/src/compiler/component.rs +++ b/crates/cranelift/src/compiler/component.rs @@ -1761,82 +1761,25 @@ impl ComponentCompiler for Compiler { &wasm_func_ty, ); - match intrinsic { - UnsafeIntrinsic::U8NativeLoad - | UnsafeIntrinsic::U16NativeLoad - | UnsafeIntrinsic::U32NativeLoad - | UnsafeIntrinsic::U64NativeLoad => c.translate_load_intrinsic(intrinsic)?, - UnsafeIntrinsic::U8NativeStore - | UnsafeIntrinsic::U16NativeStore - | UnsafeIntrinsic::U32NativeStore - | UnsafeIntrinsic::U64NativeStore => c.translate_store_intrinsic(intrinsic)?, - UnsafeIntrinsic::U8CheckedNativeLoad - | UnsafeIntrinsic::U16CheckedNativeLoad - | UnsafeIntrinsic::U32CheckedNativeLoad - | UnsafeIntrinsic::U64CheckedNativeLoad => { - c.translate_checked_load_intrinsic(intrinsic)? - } - UnsafeIntrinsic::U8CheckedNativeStore - | UnsafeIntrinsic::U16CheckedNativeStore - | UnsafeIntrinsic::U32CheckedNativeStore - | UnsafeIntrinsic::U64CheckedNativeStore => { - c.translate_checked_store_intrinsic(intrinsic)? - } - UnsafeIntrinsic::StoreDataAddress => { - let [callee_vmctx, _caller_vmctx] = *c.abi_load_params() else { - unreachable!() - }; - let pointer_type = self.isa.pointer_type(); - - // Load the `*mut VMStoreContext` out of our vmctx. - let vmctx_region = c.builder.func.dfg.alias_regions.insert( - AliasRegionKey::Vm { - ty: VmType::VMContext, - offset: c.offsets.vm_store_context(), - } - .into(), - ); - let store_ctx_region = c.builder.func.dfg.alias_regions.insert( - AliasRegionKey::Vm { - ty: VmType::VMStoreContext, - offset: u32::from(c.offsets.ptr.vmstore_context_store_data()), - } - .into(), - ); - let store_ctx = c.builder.ins().load( - pointer_type, - ir::MemFlagsData::trusted() - .with_readonly() - .with_alias_region(Some(vmctx_region)) - .with_can_move(), - callee_vmctx, - i32::try_from(c.offsets.vm_store_context()).unwrap(), - ); - - // Load the `*mut T` out of the `VMStoreContext`. - let data_address = c.builder.ins().load( - pointer_type, - ir::MemFlagsData::trusted() - .with_readonly() - .with_alias_region(Some(store_ctx_region)) - .with_can_move(), - store_ctx, - i32::from(c.offsets.ptr.vmstore_context_store_data()), - ); - - // Zero-extend the address if we are on a 32-bit architecture. - let data_address = match pointer_type.bits() { - 32 => c.builder.ins().uextend(ir::types::I64, data_address), - 64 => data_address, - p => bail!("unsupported architecture: no support for {p}-bit pointers"), - }; - - c.abi_store_results(&[data_address]); - } - UnsafeIntrinsic::ContextGetI32_0 - | UnsafeIntrinsic::ContextGetI32_1 - | UnsafeIntrinsic::ContextSetI32_0 - | UnsafeIntrinsic::ContextSetI32_1 => c.translate_context_intrinsic(intrinsic)?, + // Translate the intrinsic by reusing the very same + // `UnsafeIntrinsicCompiler` that is used when intrinsics are inlined + // directly into their callers. The only difference is that here we drive + // it over this standalone trampoline's body: we load the trampoline's + // parameters, hand them to the intrinsic compiler, and return whatever + // value (if any) it produces. + let params = c.abi_load_params(); + let isa = c.isa; + let ptr = c.offsets.ptr; + let (mut traps, builder) = c.traps(); + let mut intrinsic_compiler = UnsafeIntrinsicCompiler { + isa, + builder, + ptr, + traps: &mut traps, + }; + match intrinsic_compiler.translate(intrinsic, ¶ms)? { + Some(value) => c.abi_store_results(&[value]), + None => c.abi_store_results(&[]), } c.builder.finalize(c.isa.frontend_config()); @@ -2065,221 +2008,6 @@ impl TrampolineCompiler<'_> { i32::from(self.offsets.ptr.vmmemory_definition_base()), ) } - - fn translate_load_intrinsic(&mut self, intrinsic: UnsafeIntrinsic) -> Result<()> { - debug_assert_eq!(intrinsic.core_params(), &[WasmValType::I64]); - debug_assert_eq!(intrinsic.core_results().len(), 1); - - let wasm_ty = intrinsic.core_results()[0]; - let clif_ty = unsafe_intrinsic_clif_results(intrinsic)[0]; - - let [_callee_vmctx, _caller_vmctx, pointer] = *self.abi_load_params() else { - unreachable!() - }; - - debug_assert_eq!(self.builder.func.dfg.value_type(pointer), ir::types::I64); - let pointer = match self.isa.pointer_bits() { - 32 => self.builder.ins().ireduce(ir::types::I32, pointer), - 64 => pointer, - p => bail!("unsupported architecture: no support for {p}-bit pointers"), - }; - - let mut value = self - .builder - .ins() - .load(clif_ty, ir::MemFlagsData::trusted(), pointer, 0); - - let wasm_clif_ty = crate::value_type(self.isa, wasm_ty); - if clif_ty != wasm_clif_ty { - assert!(clif_ty.bytes() < wasm_clif_ty.bytes()); - value = self.builder.ins().uextend(wasm_clif_ty, value); - } - - self.abi_store_results(&[value]); - Ok(()) - } - - fn translate_store_intrinsic(&mut self, intrinsic: UnsafeIntrinsic) -> Result<()> { - debug_assert!(intrinsic.core_results().is_empty()); - debug_assert!(matches!(intrinsic.core_params(), [WasmValType::I64, _])); - - let wasm_ty = intrinsic.core_params()[1]; - let clif_ty = unsafe_intrinsic_clif_params(intrinsic)[1]; - - let [_callee_vmctx, _caller_vmctx, pointer, mut value] = *self.abi_load_params() else { - unreachable!() - }; - - debug_assert_eq!(self.builder.func.dfg.value_type(pointer), ir::types::I64); - let pointer = match self.isa.pointer_bits() { - 32 => self.builder.ins().ireduce(ir::types::I32, pointer), - 64 => pointer, - p => bail!("unsupported architecture: no support for {p}-bit pointers"), - }; - - let wasm_ty = crate::value_type(self.isa, wasm_ty); - if clif_ty != wasm_ty { - assert!(clif_ty.bytes() < wasm_ty.bytes()); - value = self.builder.ins().ireduce(clif_ty, value); - } - - self.builder - .ins() - .store(ir::MemFlagsData::trusted(), value, pointer, 0); - - self.abi_store_results(&[]); - Ok(()) - } - - fn translate_checked_load_intrinsic(&mut self, intrinsic: UnsafeIntrinsic) -> Result<()> { - debug_assert_eq!( - intrinsic.core_params(), - &[WasmValType::I64, WasmValType::I64, WasmValType::I64] - ); - debug_assert_eq!(intrinsic.core_results().len(), 1); - - let wasm_ty = intrinsic.core_results()[0]; - let clif_ty = unsafe_intrinsic_clif_results(intrinsic)[0]; - - let [_callee_vmctx, _caller_vmctx, base_address, offset, length] = *self.abi_load_params() - else { - unreachable!( - "should've been caught by validation: wrong number of params for checked load \ - intrinsic" - ) - }; - - // Bounds-check the access and compute its native address, trapping if - // it is out of bounds. - let isa = self.isa; - let (mut traps, builder) = self.traps(); - let addr = checked_native_addr( - &mut traps, - builder, - isa, - base_address, - offset, - length, - clif_ty.bytes(), - )?; - - // Do the load! - let mut value = builder - .ins() - .load(clif_ty, ir::MemFlagsData::trusted(), addr, 0); - - // Zero-extend the loaded value to the Wasm result type, if necessary. - let wasm_clif_ty = crate::value_type(isa, wasm_ty); - if clif_ty != wasm_clif_ty { - assert!(clif_ty.bytes() < wasm_clif_ty.bytes()); - value = builder.ins().uextend(wasm_clif_ty, value); - } - - self.abi_store_results(&[value]); - Ok(()) - } - - fn translate_checked_store_intrinsic(&mut self, intrinsic: UnsafeIntrinsic) -> Result<()> { - debug_assert!(intrinsic.core_results().is_empty()); - debug_assert!(matches!( - intrinsic.core_params(), - [ - WasmValType::I64, - WasmValType::I64, - WasmValType::I64, - _value_ty - ] - )); - - let wasm_ty = intrinsic.core_params()[3]; - let clif_ty = unsafe_intrinsic_clif_params(intrinsic)[3]; - - let [ - _callee_vmctx, - _caller_vmctx, - base_address, - offset, - length, - mut value, - ] = *self.abi_load_params() - else { - unreachable!() - }; - - // Bounds-check the access and compute its native address, trapping if - // it is out of bounds. - let isa = self.isa; - let (mut traps, builder) = self.traps(); - let addr = checked_native_addr( - &mut traps, - builder, - isa, - base_address, - offset, - length, - clif_ty.bytes(), - )?; - - // Truncate the value to the access type, if necessary. - let wasm_ty = crate::value_type(isa, wasm_ty); - if clif_ty != wasm_ty { - assert!(clif_ty.bytes() < wasm_ty.bytes()); - value = builder.ins().ireduce(clif_ty, value); - } - - // Do the store! - builder - .ins() - .store(ir::MemFlagsData::trusted(), value, addr, 0); - - self.abi_store_results(&[]); - Ok(()) - } - - fn translate_context_intrinsic(&mut self, intrinsic: UnsafeIntrinsic) -> Result<()> { - let ty = match intrinsic { - UnsafeIntrinsic::ContextGetI32_0 - | UnsafeIntrinsic::ContextSetI32_0 - | UnsafeIntrinsic::ContextGetI32_1 - | UnsafeIntrinsic::ContextSetI32_1 => ir::types::I32, - _ => unreachable!(), - }; - let slot = match intrinsic { - UnsafeIntrinsic::ContextGetI32_0 | UnsafeIntrinsic::ContextSetI32_0 => 0, - UnsafeIntrinsic::ContextGetI32_1 | UnsafeIntrinsic::ContextSetI32_1 => 1, - _ => unreachable!(), - }; - let offset = self - .offsets - .ptr - .vmstore_context_component_context_slot(slot); - let vmstore_context = self.load_vm_store_context(); - match intrinsic { - UnsafeIntrinsic::ContextGetI32_0 | UnsafeIntrinsic::ContextGetI32_1 => { - let context = self.builder.ins().load( - ty, - ir::MemFlagsData::trusted(), - vmstore_context, - i32::from(offset), - ); - self.abi_store_results(&[context]); - } - UnsafeIntrinsic::ContextSetI32_0 | UnsafeIntrinsic::ContextSetI32_1 => { - let [_callee_vmctx, _caller_vmctx, new_context] = *self.abi_load_params() else { - unreachable!() - }; - self.builder.ins().store( - ir::MemFlagsData::trusted(), - new_context, - vmstore_context, - i32::from(offset), - ); - self.abi_store_results(&[]); - } - _ => unreachable!(), - } - Ok(()) - } } /// A helper structure to translate an `UnsafeIntrinsic`. diff --git a/tests/all/intrinsics.rs b/tests/all/intrinsics.rs index c5a21002af4b..e39ae38c991d 100644 --- a/tests/all/intrinsics.rs +++ b/tests/all/intrinsics.rs @@ -627,6 +627,97 @@ fn store_data_address() -> Result<()> { Ok(()) } +/// Exercise intrinsics that are: +/// +/// - lowered to core functions, +/// - re-exported directly from a core instance *without* being wrapped in any +/// core function, +/// - lifted back into component functions, +/// - and then called directly by the host. +/// +/// This is a tricky case for intrinsics like `store-data-address` that read out +/// of a vmctx: in this path the intrinsic trampoline is reached without any +/// intervening core-Wasm caller, so we must not assume that the trampoline's +/// caller vmctx is a core-Wasm `VMContext`. +#[test] +#[cfg_attr(miri, ignore)] +fn directly_reexported_and_lifted_intrinsics() -> Result<()> { + let engine = Engine::default(); + + let wat = r#" + (component + (import "unsafe-intrinsics" + (instance $intrinsics + (export "store-data-address" (func (result u64))) + (export "u64-native-load" (func (param "pointer" u64) (result u64))) + ) + ) + + ;; Lower the intrinsics to core functions. + (core func $store-data-address' (canon lower (func $intrinsics "store-data-address"))) + (core func $u64-native-load' (canon lower (func $intrinsics "u64-native-load"))) + + ;; A core module that imports the intrinsics and re-exports them + ;; directly, without wrapping them in any core function of its own. + (core module $m + (import "" "store-data-address" (func $store-data-address (result i64))) + (import "" "u64-native-load" (func $load (param i64) (result i64))) + (export "store-data-address" (func $store-data-address)) + (export "u64-native-load" (func $load)) + ) + + (core instance $i + (instantiate $m + (with "" (instance (export "store-data-address" (func $store-data-address')) + (export "u64-native-load" (func $u64-native-load')))) + ) + ) + + ;; Lift the re-exported core functions directly back into component + ;; functions and export them. + (func (export "store-data-address") (result u64) + (canon lift (core func $i "store-data-address"))) + (func (export "u64-native-load") (param "pointer" u64) (result u64) + (canon lift (core func $i "u64-native-load"))) + ) + "#; + + let mut code_builder = CodeBuilder::new(&engine); + code_builder.wasm_binary_or_text(wat.as_bytes(), None)?; + unsafe { + code_builder.expose_unsafe_intrinsics("unsafe-intrinsics"); + } + let component = code_builder.compile_component()?; + + let known = 0x1122_3344_5566_7788_u64; + let linker = component::Linker::new(&engine); + let mut store = Store::new(&engine, known); + let instance = linker.instantiate(&mut store, &component)?; + + let store_data_address = + instance.get_typed_func::<(), (u64,)>(&mut store, "store-data-address")?; + let load = instance.get_typed_func::<(u64,), (u64,)>(&mut store, "u64-native-load")?; + + // `store-data-address` must return the address of the store's `T` data, + // which is the same address that `Store::data` exposes. + let (address,) = store_data_address.call(&mut store, ())?; + let expected = core::ptr::from_ref(store.data()) as u64; + assert_eq!( + address, expected, + "store-data-address returned the wrong pointer" + ); + + // And loading through that address (also via a directly-lifted intrinsic) + // must observe the known store data. + let (value,) = load.call(&mut store, (address,))?; + assert_eq!( + value, known, + "u64-native-load through store-data-address read the wrong data" + ); + + Ok(()) +} + /// A 16-byte buffer aligned to 8 bytes so that aligned native accesses of any /// of our intrinsic widths (`u8`/`u16`/`u32`/`u64`) are well-defined. #[repr(align(8))] diff --git a/tests/disas/component-model/unsafe-intrinsics-used.wat b/tests/disas/component-model/unsafe-intrinsics-used.wat index 441c176ce06e..fdc5a2808a5c 100644 --- a/tests/disas/component-model/unsafe-intrinsics-used.wat +++ b/tests/disas/component-model/unsafe-intrinsics-used.wat @@ -35,11 +35,11 @@ ) ;; function u0:0(i64 vmctx, i64) -> i64 tail { -;; region0 = 16 "VMContext+0x10" +;; region0 = 8 "VMContext+0x8" ;; region1 = 268435560 "VMStoreContext+0x68" ;; ;; block0(v0: i64, v1: i64): -;; v2 = load.i64 notrap aligned readonly can_move region0 v0+16 +;; v2 = load.i64 notrap aligned readonly can_move region0 v1+8 ;; v3 = load.i64 notrap aligned readonly can_move region1 v2+104 ;; return v3 ;; }