From 62f4d99d2e2bb1d058003b76362ae670b84fcb72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Cabrera?= Date: Wed, 17 May 2023 10:55:26 -0400 Subject: [PATCH 1/2] winch(trampolines): Save SP, FP and return address This change is a follow-up to https://github.com/bytecodealliance/wasmtime/pull/6358 This change implements the necessary stores of SP, FP and return address for fast stack walking. --- tests/all/winch.rs | 116 +++++++++++++++++------ winch/codegen/src/abi/mod.rs | 6 ++ winch/codegen/src/isa/aarch64/abi.rs | 8 ++ winch/codegen/src/isa/x64/abi.rs | 16 ++++ winch/codegen/src/trampoline.rs | 135 +++++++++++++++++++++++---- 5 files changed, 236 insertions(+), 45 deletions(-) diff --git a/tests/all/winch.rs b/tests/all/winch.rs index ac1541aa6d3c..beddceb1ac8a 100644 --- a/tests/all/winch.rs +++ b/tests/all/winch.rs @@ -1,3 +1,4 @@ +use anyhow::{bail, Result}; use wasmtime::*; const MODULE: &'static str = r#" @@ -45,80 +46,139 @@ fn add_fn(store: impl AsContextMut) -> Func { #[test] #[cfg_attr(miri, ignore)] -fn array_to_wasm() { +fn array_to_wasm() -> Result<()> { let mut c = Config::new(); c.strategy(Strategy::Winch); - let engine = Engine::new(&c).unwrap(); + let engine = Engine::new(&c)?; let mut store = Store::new(&engine, ()); - let module = Module::new(&engine, MODULE).unwrap(); + let module = Module::new(&engine, MODULE)?; let add_fn = add_fn(store.as_context_mut()); - let instance = Instance::new(&mut store, &module, &[add_fn.into()]).unwrap(); + let instance = Instance::new(&mut store, &module, &[add_fn.into()])?; let constant = instance .get_func(&mut store, "42") - .ok_or(anyhow::anyhow!("test function not found")) - .unwrap(); + .ok_or(anyhow::anyhow!("test function not found"))?; let mut returns = vec![Val::null(); 1]; - constant.call(&mut store, &[], &mut returns).unwrap(); + constant.call(&mut store, &[], &mut returns)?; assert_eq!(returns.len(), 1); assert_eq!(returns[0].unwrap_i32(), 42); let sum = instance .get_func(&mut store, "sum10") - .ok_or(anyhow::anyhow!("sum10 function not found")) - .unwrap(); + .ok_or(anyhow::anyhow!("sum10 function not found"))?; let mut returns = vec![Val::null(); 1]; let args = vec![Val::I32(1); 10]; - sum.call(&mut store, &args, &mut returns).unwrap(); + sum.call(&mut store, &args, &mut returns)?; assert_eq!(returns.len(), 1); assert_eq!(returns[0].unwrap_i32(), 10); + Ok(()) } #[test] #[cfg_attr(miri, ignore)] -fn native_to_wasm() { +fn native_to_wasm() -> Result<()> { let mut c = Config::new(); c.strategy(Strategy::Winch); - let engine = Engine::new(&c).unwrap(); + let engine = Engine::new(&c)?; let mut store = Store::new(&engine, ()); - let module = Module::new(&engine, MODULE).unwrap(); + let module = Module::new(&engine, MODULE)?; let add_fn = add_fn(store.as_context_mut()); - let instance = Instance::new(&mut store, &module, &[add_fn.into()]).unwrap(); + let instance = Instance::new(&mut store, &module, &[add_fn.into()])?; - let f = instance - .get_typed_func::<(i32, i32, i32, i32, i32, i32, i32, i32, i32, i32), i32>( - &mut store, "sum10", - ) - .unwrap(); + let f = instance.get_typed_func::<(i32, i32, i32, i32, i32, i32, i32, i32, i32, i32), i32>( + &mut store, "sum10", + )?; let args = (1, 1, 1, 1, 1, 1, 1, 1, 1, 1); - let result = f.call(&mut store, args).unwrap(); + let result = f.call(&mut store, args)?; assert_eq!(result, 10); + Ok(()) } #[test] #[cfg_attr(miri, ignore)] -fn wasm_to_native() { +fn wasm_to_native() -> Result<()> { let mut c = Config::new(); c.strategy(Strategy::Winch); - let engine = Engine::new(&c).unwrap(); + let engine = Engine::new(&c)?; let mut store = Store::new(&engine, ()); - let module = Module::new(&engine, MODULE).unwrap(); + let module = Module::new(&engine, MODULE)?; let add_fn = add_fn(store.as_context_mut()); - let instance = Instance::new(&mut store, &module, &[add_fn.into()]).unwrap(); - let f = instance - .get_typed_func::<(i32, i32), i32>(&mut store, "call_add") - .unwrap(); + let instance = Instance::new(&mut store, &module, &[add_fn.into()])?; + + let f = instance.get_typed_func::<(i32, i32), i32>(&mut store, "call_add")?; let args = (41, 1); - let result = f.call(&mut store, args).unwrap(); + let result = f.call(&mut store, args)?; assert_eq!(result, 42); + Ok(()) +} + +#[test] +#[cfg_attr(miri, ignore)] +// NB +// This and the following test(`native_to_wasm_trap` and `wasm_to_native_trap`), +// are mostly smoke tests to ensure Winch's trampolines are compliant with fast +// stack walking. The ideal state is one in which we should not have to worry +// about testing the backtrace implementation per compiler, but instead be +// certain that a single set of test cases is enough to ensure that the machine +// code generated by Winch and Cranelift is compliant. One way to achieve this +// could be to share the implementation of trampolines between Cranelift and +// Winch. +fn native_to_wasm_trap() -> Result<()> { + let mut c = Config::new(); + c.strategy(Strategy::Winch); + let engine = Engine::new(&c)?; + let wat = r#" + (module + (func $div_by_zero (result i32) + (i32.const 1) + (i32.const 0) + i32.div_u) + + (export "div_by_zero" (func $div_by_zero))) + "#; + let mut store = Store::new(&engine, ()); + let module = Module::new(&engine, wat)?; + let instance = Instance::new(&mut store, &module, &[])?; + let f = instance.get_typed_func::<(), i32>(&mut store, "div_by_zero")?; + let result = f.call(&mut store, ()).unwrap_err(); + + assert!(result.downcast_ref::().is_some()); + + Ok(()) +} + +#[test] +#[cfg_attr(miri, ignore)] +fn wasm_to_native_trap() -> Result<()> { + let mut c = Config::new(); + c.strategy(Strategy::Winch); + let engine = Engine::new(&c)?; + let wat = r#" + (module + (import "" "" (func $fail)) + (func $call_fail + call $fail) + + (export "call_fail" (func $call_fail))) + "#; + let mut store = Store::new(&engine, ()); + let module = Module::new(&engine, wat)?; + let func = Func::wrap::<(), (), Result<()>>(&mut store, || bail!("error")); + let instance = Instance::new(&mut store, &module, &[func.into()])?; + let f = instance.get_typed_func::<(), ()>(&mut store, "call_fail")?; + let result = f.call(&mut store, ()).unwrap_err(); + + assert!(result.downcast_ref::().is_some()); + + Ok(()) } diff --git a/winch/codegen/src/abi/mod.rs b/winch/codegen/src/abi/mod.rs index 87e95b9101c2..79e5229198f4 100644 --- a/winch/codegen/src/abi/mod.rs +++ b/winch/codegen/src/abi/mod.rs @@ -64,6 +64,9 @@ pub(crate) trait ABI { /// The offset to the argument base, relative to the frame pointer. fn arg_base_offset(&self) -> u8; + /// The offset to the return address, relative to the frame pointer. + fn ret_addr_offset() -> u8; + /// Construct the ABI-specific signature from a WebAssembly /// function type. fn sig(&self, wasm_sig: &FuncType, call_conv: &CallingConvention) -> ABISig; @@ -82,6 +85,9 @@ pub(crate) trait ABI { /// Returns the frame pointer register. fn fp_reg() -> Reg; + /// Returns the stack pointer register. + fn sp_reg() -> Reg; + /// Returns the pinned register used to hold /// the `VMContext`. fn vmctx_reg() -> Reg; diff --git a/winch/codegen/src/isa/aarch64/abi.rs b/winch/codegen/src/isa/aarch64/abi.rs index 96026bd3e832..2b3a335b7482 100644 --- a/winch/codegen/src/isa/aarch64/abi.rs +++ b/winch/codegen/src/isa/aarch64/abi.rs @@ -55,6 +55,10 @@ impl ABI for Aarch64ABI { 16 } + fn ret_addr_offset() -> u8 { + 8 + } + fn word_bits() -> u32 { 64 } @@ -87,6 +91,10 @@ impl ABI for Aarch64ABI { todo!() } + fn sp_reg() -> Reg { + todo!() + } + fn fp_reg() -> Reg { regs::fp() } diff --git a/winch/codegen/src/isa/x64/abi.rs b/winch/codegen/src/isa/x64/abi.rs index 7f6553ca4a33..e02c264c241d 100644 --- a/winch/codegen/src/isa/x64/abi.rs +++ b/winch/codegen/src/isa/x64/abi.rs @@ -80,6 +80,18 @@ impl ABI for X64ABI { 16 } + fn ret_addr_offset() -> u8 { + // 1 8-byte slot. + // ┌──────────┬ + // │ Ret │ + // │ Addr │ + // ├──────────┼ * offset + // │ │ + // │ FP │ + // └──────────┴ + 8 + } + fn word_bits() -> u32 { 64 } @@ -125,6 +137,10 @@ impl ABI for X64ABI { regs::rbp() } + fn sp_reg() -> Reg { + regs::rsp() + } + fn vmctx_reg() -> Reg { regs::vmctx() } diff --git a/winch/codegen/src/trampoline.rs b/winch/codegen/src/trampoline.rs index b35d4ca84551..b310735b9c98 100644 --- a/winch/codegen/src/trampoline.rs +++ b/winch/codegen/src/trampoline.rs @@ -9,8 +9,6 @@ // loading/storing the VM context pointer. The real value of the operand size // and VM context type should be derived from the ABI's pointer size. This is // going to be relevant once 32-bit architectures are supported. -// -// * Save the fp and pc for fast stack walking. use crate::{ abi::{ABIArg, ABIParams, ABIResult, ABISig, ABI}, isa::CallingConvention, @@ -107,10 +105,15 @@ where // Get the VM context pointer and move it to the designated pinned // register. - let vmctx_ptr = Self::vmctx(&native_sig.params)?; - self.masm - .mov(vmctx_ptr, ::vmctx_reg().into(), OperandSize::S64); + let (vmctx, caller_vmctx) = Self::callee_and_caller_vmctx(&native_sig.params)?; + self.masm.mov( + vmctx.into(), + ::vmctx_reg().into(), + OperandSize::S64, + ); + + let vmctx_runtime_limits_addr = self.vmctx_runtime_limits_addr(caller_vmctx); let (offsets, spill_size) = self.spill(&native_sig.params); let val_ptr_offset = offsets[2]; @@ -121,6 +124,19 @@ where self.abi.arg_base_offset().into(), wasm_sig.stack_bytes, |masm| { + // Save the SP when entering Wasm. + // TODO: Once Winch supports comparison operators, + // check that the caller VM context is what we expect. + // See [`wasmtime_environ::MAGIC`]. + Self::save_last_wasm_entry_sp( + masm, + vmctx_runtime_limits_addr, + self.scratch_reg, + &self.pointer_size, + ); + + // Move the values register to the scratch + // register for argument assignment. masm.mov(*val_ptr, self.scratch_reg.into(), OperandSize::S64); Self::assign_args_from_array( masm, @@ -161,13 +177,17 @@ where pub fn emit_native_to_wasm(&mut self, ty: &FuncType, callee_index: FuncIndex) -> Result<()> { let native_sig = self.native_sig(&ty); let wasm_sig = self.wasm_sig(&ty); - let vmctx_ptr = Self::vmctx(&native_sig.params)?; + let (vmctx, caller_vmctx) = Self::callee_and_caller_vmctx(&native_sig.params)?; self.prologue_with_callee_saved(); // Move the VM context pointer to the designated pinned register. - self.masm - .mov(vmctx_ptr, ::vmctx_reg().into(), OperandSize::S64); + self.masm.mov( + vmctx.into(), + ::vmctx_reg().into(), + OperandSize::S64, + ); + let vmctx_runtime_limits_addr = self.vmctx_runtime_limits_addr(caller_vmctx); let (offsets, spill_size) = self.spill(&native_sig.params); let reserved_stack = self.masm.call( @@ -175,6 +195,16 @@ where self.abi.arg_base_offset().into(), wasm_sig.stack_bytes, |masm| { + // Save the SP when entering Wasm. + // TODO: Once Winch supports comparison operators, + // check that the caller VM context is what we expect. + // See [`wasmtime_environ::MAGIC`]. + Self::save_last_wasm_entry_sp( + masm, + vmctx_runtime_limits_addr, + self.scratch_reg, + &self.pointer_size, + ); Self::assign_args( masm, &wasm_sig.params, @@ -195,14 +225,30 @@ where /// Emit a wasm-to-native trampoline. pub fn emit_wasm_to_native(&mut self, ty: &FuncType) -> Result<()> { - let mut params = Self::callee_and_caller_vmctx(); + let mut params = Self::callee_and_caller_vmctx_types(); params.extend_from_slice(ty.params()); let func_ty = FuncType::new(params, ty.results().to_owned()); let wasm_sig = self.wasm_sig(&func_ty); let native_sig = self.native_sig(ty); + let (vmctx, caller_vmctx) = Self::callee_and_caller_vmctx(&wasm_sig.params).unwrap(); + let vmctx_runtime_limits_addr = self.vmctx_runtime_limits_addr(caller_vmctx); + self.prologue(); + + // Save the FP and return address when exiting Wasm. + // TODO: Once Winch supports comparison operators, + // check that the caller VM context is what we expect. + // See [`wasmtime_environ::MAGIC`]. + Self::save_last_wasm_exit_fp_and_pc( + self.masm, + vmctx_runtime_limits_addr, + self.scratch_reg, + self.alloc_scratch_reg, + &self.pointer_size, + ); + let (offsets, spill_size) = self.spill(&wasm_sig.params); let reserved_stack = self.masm.call( @@ -211,7 +257,6 @@ where native_sig.stack_bytes, |masm| { // Move the VM context into one of the scratch registers. - let vmctx = Self::vmctx(&wasm_sig.params).unwrap(); masm.mov( vmctx.into(), self.alloc_scratch_reg.into(), @@ -304,13 +349,13 @@ where } /// Get the type of the caller and callee VM contexts. - fn callee_and_caller_vmctx() -> Vec { + fn callee_and_caller_vmctx_types() -> Vec { vec![ValType::I64, ValType::I64] } /// Returns a signature using the system's calling convention. fn native_sig(&self, ty: &FuncType) -> ABISig { - let mut params = Self::callee_and_caller_vmctx(); + let mut params = Self::callee_and_caller_vmctx_types(); params.extend_from_slice(ty.params()); let native_type = FuncType::new(params, ty.results().to_owned()); @@ -322,12 +367,26 @@ where self.abi.sig(ty, &CallingConvention::Default) } - /// Returns the register containing the VM context pointer. - fn vmctx(params: &ABIParams) -> Result { - params[0] + /// Returns the register pair containing the callee and caller VM context pointers. + fn callee_and_caller_vmctx(params: &ABIParams) -> Result<(Reg, Reg)> { + let vmctx = params[0] .get_reg() - .map(RegImm::reg) - .ok_or_else(|| anyhow!("Expected vm context pointer to be in a register")) + .ok_or_else(|| anyhow!("Expected vm context pointer to be in a register"))?; + + let caller_vmctx = params[1] + .get_reg() + .ok_or_else(|| anyhow!("Expected caller vm context pointer to be in a register"))?; + + Ok((vmctx, caller_vmctx)) + } + + /// Returns the address of the VM context runtime limits + /// field. + fn vmctx_runtime_limits_addr(&mut self, caller_vmctx: Reg) -> M::Address { + self.masm.address_at_reg( + caller_vmctx, + self.pointer_size.vmcontext_runtime_limits().into(), + ) } /// Performs a spill of the register params. @@ -380,6 +439,48 @@ where }); } + fn save_last_wasm_entry_sp( + masm: &mut M, + vm_runtime_limits_addr: M::Address, + scratch: Reg, + ptr: &impl PtrSize, + ) { + let sp = ::sp_reg(); + masm.load(vm_runtime_limits_addr, scratch, OperandSize::S64); + let addr = masm.address_at_reg(scratch, ptr.vmruntime_limits_last_wasm_entry_sp().into()); + masm.store(sp.into(), addr, OperandSize::S64); + } + + fn save_last_wasm_exit_fp_and_pc( + masm: &mut M, + vm_runtime_limits_addr: M::Address, + scratch: Reg, + alloc_scratch: Reg, + ptr: &impl PtrSize, + ) { + masm.load(vm_runtime_limits_addr, alloc_scratch, OperandSize::S64); + let last_wasm_exit_fp_addr = masm.address_at_reg( + alloc_scratch, + ptr.vmruntime_limits_last_wasm_exit_fp().into(), + ); + let last_wasm_exit_pc_addr = masm.address_at_reg( + alloc_scratch, + ptr.vmruntime_limits_last_wasm_exit_pc().into(), + ); + + // Handle the frame pointer. + let fp = ::fp_reg(); + let fp_addr = masm.address_at_reg(fp, 0); + masm.load(fp_addr, scratch, OperandSize::S64); + masm.store(scratch.into(), last_wasm_exit_fp_addr, OperandSize::S64); + + // Handle the return address. + let ret_addr_offset = ::ret_addr_offset(); + let ret_addr = masm.address_at_reg(fp, ret_addr_offset.into()); + masm.load(ret_addr, scratch, OperandSize::S64); + masm.store(scratch.into(), last_wasm_exit_pc_addr, OperandSize::S64); + } + /// The trampoline's prologue. fn prologue(&mut self) { self.masm.prologue(); From 8d75bdc996ab33998cf5376e3820ec7f7897c90c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Cabrera?= Date: Mon, 22 May 2023 14:43:28 -0400 Subject: [PATCH 2/2] Ignore backtrace test on Windows Temporarily ignoring Winch's trap test on Windows while support for unwind information is added. --- tests/all/winch.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/all/winch.rs b/tests/all/winch.rs index beddceb1ac8a..503d393dc1c8 100644 --- a/tests/all/winch.rs +++ b/tests/all/winch.rs @@ -124,7 +124,9 @@ fn wasm_to_native() -> Result<()> { #[test] #[cfg_attr(miri, ignore)] +#[cfg_attr(windows, ignore)] // NB +// // This and the following test(`native_to_wasm_trap` and `wasm_to_native_trap`), // are mostly smoke tests to ensure Winch's trampolines are compliant with fast // stack walking. The ideal state is one in which we should not have to worry @@ -133,6 +135,9 @@ fn wasm_to_native() -> Result<()> { // code generated by Winch and Cranelift is compliant. One way to achieve this // could be to share the implementation of trampolines between Cranelift and // Winch. +// +// FIXME The following two tests are also temporarily ignored on Windows, since +// we are not emitting the require unwind information yet. fn native_to_wasm_trap() -> Result<()> { let mut c = Config::new(); c.strategy(Strategy::Winch); @@ -159,6 +164,7 @@ fn native_to_wasm_trap() -> Result<()> { #[test] #[cfg_attr(miri, ignore)] +#[cfg_attr(windows, ignore)] fn wasm_to_native_trap() -> Result<()> { let mut c = Config::new(); c.strategy(Strategy::Winch);