Fixed the clobbered return address in the RISC-V context save - #696
Merged
Conversation
_tx_thread_context_save() returns to its caller with ret, which uses the return address held in ra. When TX_ENABLE_EXECUTION_CHANGE_NOTIFY was defined, the call to _tx_execution_isr_enter overwrote ra with the address of the instruction following the call, so the subsequent ret returned into _tx_thread_context_save itself instead of the interrupt service routine. The return address is now saved on the stack around the call and recovered afterwards, which is the same idiom already used by the Arm ports. The fix covers all three affected paths (nested save, thread save and idle system save) in the risc-v32 GNU, risc-v32 IAR and risc-v64 GNU ports. Fixes eclipse-threadx#348 Assisted-by: Copilot (Opus 5) <noreply@github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_tx_thread_context_save()returns to its caller withret, which uses the return address held inra.When
TX_ENABLE_EXECUTION_CHANGE_NOTIFYis defined, thecall _tx_execution_isr_enteroverwritesrawith the address of the instruction following the call, so the subsequentretreturns into_tx_thread_context_saveitself instead of the interrupt service routine.This PR saves the return address on the stack around the call and recovers it afterwards. That is the same idiom the Arm ports already use, for example
ports_arch/ARMv7-A/threadx/common/src/tx_thread_vectored_context_save.S, which pushes and popslraround the same call.All three affected paths are covered in each port: the nested save, the thread save and the idle system save.
Files changed:
ports/risc-v32/gnu/src/tx_thread_context_save.S,ports/risc-v32/iar/src/tx_thread_context_save.sandports/risc-v64/gnu/src/tx_thread_context_save.S.I checked the remaining RISC-V assembly.
tx_thread_context_restorereloadsrafrom the saved context and returns withmret, whiletx_thread_scheduleandtx_thread_system_returndo not return to their caller, so none of them are affected.The changed files assemble cleanly with and without
TX_ENABLE_EXECUTION_CHANGE_NOTIFY, for rv32imac_zicsr, rv32imafc_zicsr and rv64imac_zicsr.Note that #691 is currently open against
ports/risc-v32/gnu/src/tx_thread_context_save.Sand carries the same defect, so one of the two branches will need a small rebase.Fixes #348