Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

winch(aarch64): Sync SP with SSP when claiming stack #10263

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

saulecabrera
Copy link
Member

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.

This commit is a follow-up to
bytecodealliance#10146  and represents
another step toward fixing the remaining issues discovered through spec
tests in the same vein as bytecodealliance#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.
@saulecabrera saulecabrera requested a review from a team as a code owner February 20, 2025 20:44
@saulecabrera saulecabrera requested review from abrown and removed request for a team February 20, 2025 20:44
@saulecabrera saulecabrera requested a review from a team as a code owner February 20, 2025 21:01
@saulecabrera saulecabrera requested review from dicej and removed request for a team February 20, 2025 21:01
@saulecabrera
Copy link
Member Author

The diff is considerably large, since this change touches all of the disassembly tests for aarch64. The relevant changes are in the first commit 97d7f74

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant