Register RA will be modified when TX_ENABLE_EXECUTION_CHANGE_NOTIFY is enabled

[xuzihan351]

Let us discuss the code below(ports/risc-v32/iar/src/tx_thread_context_save.s and others asm code)

#ifdef TX_ENABLE_EXECUTION_CHANGE_NOTIFY
    /* _tx_execution_isr_enter is called with thread stack pointer */
    call    _tx_execution_isr_enter                     ; Call the ISR execution enter function
#endif
...
...
   ret
   END

The function tx_thread_context_save is called from ISR or another function, and tx_thread_context_save will return to it by ret.
Register RA stored the return address of the caller function.
When TX_ENABLE_EXECUTION_CHANGE_NOTIFY is enabled, function _tx_execution_isr_enter will be called, and register RA will store the return address of the next line of call _tx_execution_isr_enter . Without saving register RA before call _tx_execution_isr_enter, the return address of tx_thread_context_save's caller function will be replaced. Resulting in function tx_thread_context_save will never return to the right address.

[williamelamie]

This is definitely a problem. A solution is to move RA into a compiler-preserved register (like S0) before the call. After the call returns, RA can be restored from the same register. Another option is to move the call to _tx_execution_isr_enter to AFTER _tx_thread_context_save returns. The downside of this is that it would need to be done in all ISRs that use _tx_thread_context_save. So that's more of a quick patch rather than a solution.

As for the other assembly files that have a _tx_execution* call, these look okay since RA is either saved prior to the call or restored subsequent to the call.

[fdesbiens]

Thank you for the report, and my apologies for the very long delay in answering.
Your analysis is correct, and @williamelamie's assessment is too. call _tx_execution_isr_enter overwrites ra, so the following ret does not return to the interrupt service routine.
The defect is present in three places in each of ports/risc-v32/gnu/src/tx_thread_context_save.S, ports/risc-v32/iar/src/tx_thread_context_save.s and ports/risc-v64/gnu/src/tx_thread_context_save.S, covering the nested save, the thread save and the idle system save paths.
I opened PR #696, which saves the return address on the stack around the call and recovers it afterwards. This is the idiom the Arm ports already use for the same call, so the RISC-V ports now behave consistently with the rest of the tree.
I also reviewed the other RISC-V assembly files. tx_thread_context_restore reloads ra from the saved context and returns with mret, and neither tx_thread_schedule nor tx_thread_system_return returns to its caller, so they are fine, as @williamelamie noted.
I will close this issue once the pull request is merged.

[fdesbiens]

PR #696 has been merged into dev. The return address is now saved and restored around every _tx_execution_isr_enter call in the RISC-V 32-bit (GNU and IAR) and RISC-V 64-bit context save routines.
Thank you for the report, and apologies for the long delay! Closing this one.