winch(aarch64): Sync SP with SSP when dropping stack

saulecabrera · saulecabrera · commit 97d7f740b818 · 2025-02-20T15:35:11.000-05:00
This commit is a follow-up to #10146 and represents another step toward fixing the remaining issues discovered through spec tests in the same vein as #10201 Specifically, this commit ensures that the stack pointer is always in sync with the shadow stack pointer. The previous approach was lossy because it only performed the sync when reserving stack space. While this approach worked in some cases, it failed to account for situations where the shadow stack pointer might be adjusted and aligned for calls. As a result, the stack pointer could become unaligned when claiming stack space, leading to issues at call sites. It is possible to avoid the unconditional move and perform it only when alignment is needed, i.e., at call sites and when the real stack pointer is unaligned. However, as of now, the simplest solution is to always perform the sync, which integrates best with the current infrastructure.
diff --git a/winch/codegen/src/isa/aarch64/masm.rs b/winch/codegen/src/isa/aarch64/masm.rs
@@ -163,6 +163,17 @@ impl Masm for MacroAssembler {
         self.asm
             .add_ir(bytes as u64, ssp, writable!(ssp), OperandSize::S64);
 
+        // We must ensure that the real stack pointer reflects the the offset
+        // tracked by `self.sp_offset`, we use such value to calculate
+        // alignment, which is crucial for calls.
+        //
+        // As an optimization: this synchronization doesn't need to happen all
+        // the time, in theory we could ensure to sync the shadow stack pointer
+        // with the stack pointer when alignment is required, like at callsites.
+        // This is the simplest approach at the time of writing, which
+        // integrates well with the rest of the aarch64 infrastructure.
+        self.move_shadow_sp_to_sp();
+
         self.decrement_sp(bytes);
         Ok(())
     }
