From f58a6f840026e664b2a3b83e59af9809abbd79eb Mon Sep 17 00:00:00 2001 From: Ulrich Weigand Date: Wed, 4 Dec 2024 01:39:17 +0100 Subject: [PATCH] [s390x] Fix SP unwind rule for the tail-call ABI On s390x, the unwound SP is always at current CFA - 160. Therefore, the default rule used on most other platforms (which sets the unwound SP to the current CFA) is incorrect, so we need to provide an explicit DWARF CFI rule to unwind SP. With the platform ABI, the caller's SP is always stored in the register save area like other call-saved GPRs, so we can simply use a normal DW_CFA_offset rule. However, with the new tail-call ABI, the value saved in that slot is incorrect - it is not corrected for the incoming tail-call stack arguments that will have been removed as the tail call returns. To fix this without introducing unnecessary run-time overhead, we can simply use a DW_CFA_val_offset rule that will set the unwound SP to CFA - 160, which is always correct. However, the current UnwindInst abstraction does not allow any way to generate this DWARF CFI instruction. Therefore, we introduce a new UnwindInst::RegStackOffset rule for this purpose. Fixes: https://github.com/bytecodealliance/wasmtime/issues/9719 --- cranelift/codegen/src/isa/s390x/abi.rs | 19 +++++++++++++++++-- cranelift/codegen/src/isa/unwind.rs | 9 +++++++++ cranelift/codegen/src/isa/unwind/systemv.rs | 13 +++++++++++++ cranelift/codegen/src/isa/unwind/winarm64.rs | 3 +++ cranelift/codegen/src/isa/unwind/winx64.rs | 3 +++ 5 files changed, 45 insertions(+), 2 deletions(-) diff --git a/cranelift/codegen/src/isa/s390x/abi.rs b/cranelift/codegen/src/isa/s390x/abi.rs index 986663160300..57b213e4ea81 100644 --- a/cranelift/codegen/src/isa/s390x/abi.rs +++ b/cranelift/codegen/src/isa/s390x/abi.rs @@ -738,14 +738,29 @@ impl ABIMachineSpec for S390xMachineDeps { // Use STMG to save clobbered GPRs into save area. // Note that we always save SP (%r15) here if anything is saved. if let Some((first_clobbered_gpr, _)) = get_clobbered_gprs(frame_layout) { + let mut last_clobbered_gpr = 15; let offset = 8 * first_clobbered_gpr as i64 + incoming_tail_args_size as i64; insts.push(Inst::StoreMultiple64 { rt: gpr(first_clobbered_gpr), - rt2: gpr(15), + rt2: gpr(last_clobbered_gpr), mem: MemArg::reg_plus_off(stack_reg(), offset, MemFlags::trusted()), }); if flags.unwind_info() { - for i in first_clobbered_gpr..16 { + // Normally, we instruct the unwinder to restore the stack pointer + // from its slot in the save area. However, if we have incoming + // tail-call arguments, the value saved in that slot is incorrect. + // In that case, we instead instruct the unwinder to compute the + // unwound SP relative to the current CFA, as CFA == SP + 160. + if incoming_tail_args_size != 0 { + insts.push(Inst::Unwind { + inst: UnwindInst::RegStackOffset { + clobber_offset: frame_layout.clobber_size, + reg: gpr(last_clobbered_gpr).to_real_reg().unwrap(), + }, + }); + last_clobbered_gpr = last_clobbered_gpr - 1; + } + for i in first_clobbered_gpr..(last_clobbered_gpr + 1) { insts.push(Inst::Unwind { inst: UnwindInst::SaveReg { clobber_offset: frame_layout.clobber_size + (i * 8) as u32, diff --git a/cranelift/codegen/src/isa/unwind.rs b/cranelift/codegen/src/isa/unwind.rs index 305e7b4704a2..ada37b03ccbb 100644 --- a/cranelift/codegen/src/isa/unwind.rs +++ b/cranelift/codegen/src/isa/unwind.rs @@ -196,6 +196,15 @@ pub enum UnwindInst { /// The saved register. reg: RealReg, }, + /// Computes the value of the given register in the caller as stack offset. + /// Typically used to unwind the stack pointer if the default rule does not apply. + /// The `clobber_offset` is computed the same way as for the `SaveReg` rule. + RegStackOffset { + /// The offset from the start of the clobber area to this register's value. + clobber_offset: u32, + /// The register whose value is to be set. + reg: RealReg, + }, /// Defines if the aarch64-specific pointer authentication available for ARM v8.3+ devices /// is enabled for certain pointers or not. Aarch64SetPointerAuth { diff --git a/cranelift/codegen/src/isa/unwind/systemv.rs b/cranelift/codegen/src/isa/unwind/systemv.rs index ba77103394c5..5a721fbd8232 100644 --- a/cranelift/codegen/src/isa/unwind/systemv.rs +++ b/cranelift/codegen/src/isa/unwind/systemv.rs @@ -247,6 +247,19 @@ pub(crate) fn create_unwind_info_from_insts>( let off = (clobber_offset as i32) - (clobber_offset_to_cfa as i32); instructions.push((instruction_offset, CallFrameInstruction::Offset(reg, off))); } + &UnwindInst::RegStackOffset { + clobber_offset, + reg, + } => { + let reg = mr + .map(reg.into()) + .map_err(|e| CodegenError::RegisterMappingError(e))?; + let off = (clobber_offset as i32) - (clobber_offset_to_cfa as i32); + instructions.push(( + instruction_offset, + CallFrameInstruction::ValOffset(reg, off), + )); + } &UnwindInst::Aarch64SetPointerAuth { return_addresses } => { instructions.push(( instruction_offset, diff --git a/cranelift/codegen/src/isa/unwind/winarm64.rs b/cranelift/codegen/src/isa/unwind/winarm64.rs index a4ab147fb439..58a1b38b9df4 100644 --- a/cranelift/codegen/src/isa/unwind/winarm64.rs +++ b/cranelift/codegen/src/isa/unwind/winarm64.rs @@ -301,6 +301,9 @@ pub(crate) fn create_unwind_info_from_insts( } } } + &UnwindInst::RegStackOffset { .. } => { + unreachable!("only supported with DWARF"); + } &UnwindInst::Aarch64SetPointerAuth { return_addresses } => { assert!( return_addresses, diff --git a/cranelift/codegen/src/isa/unwind/winx64.rs b/cranelift/codegen/src/isa/unwind/winx64.rs index 8a23494b1f43..55014518b54f 100644 --- a/cranelift/codegen/src/isa/unwind/winx64.rs +++ b/cranelift/codegen/src/isa/unwind/winx64.rs @@ -285,6 +285,9 @@ pub(crate) fn create_unwind_info_from_insts { + unreachable!("only supported with DWARF"); + } &UnwindInst::Aarch64SetPointerAuth { .. } => { unreachable!("no aarch64 on x64"); }