elliottt opened PR #8401 from elliottt:trevor/riscv64-prologue-sp
to bytecodealliance:main
:
Instead of generating negative offsets to SP for clobbers and updating SP after clobbers have been spilled, eagerly bump SP and write the clobbers to positive offsets instead. This change should preserve the locations on the stack where the clobbers are written, so no changes were necessary for the epilogue.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
elliottt updated PR #8401.
elliottt updated PR #8401.
elliottt updated PR #8401.
elliottt has marked PR #8401 as ready for review.
elliottt requested cfallin for a review on PR #8401.
elliottt requested wasmtime-compiler-reviewers for a review on PR #8401.
elliottt requested alexcrichton for a review on PR #8401.
elliottt requested jameysharp for a review on PR #8401.
jameysharp submitted PR review:
I believe this change is correct and also fixes a bug: We're generally not supposed to write important information, like clobber saves, beyond wherever the stack pointer points. A signal handler could run on our stack at any time and is allowed to overwrite anything outside our reserved portion of the stack. Some architectures provide a small "red zone" where we could get away with this, but we shouldn't rely on that.
For the same reason, the epilogue should also be updated to not adjust the stack pointer until it has already restored all the clobbers.
Would you update the commit message and PR description to mention this bug fix?
elliottt updated PR #8401.
Last updated: Nov 22 2024 at 16:03 UTC