diff --git a/components/freertos/test/test_context_save_clobber.S b/components/freertos/test/test_context_save_clobber.S new file mode 100644 index 0000000000..a9af1baa0a --- /dev/null +++ b/components/freertos/test/test_context_save_clobber.S @@ -0,0 +1,55 @@ +/* Helper function for the test case in test_context_save_clobber.c */ + +#if defined(__XTENSA__) +#include "xtensa/config/core-isa.h" +#if defined(XCHAL_HAVE_WINDOWED) + + .data +recursion_count: + .word 0 + + .text + .global test_context_save_clober_func + .type test_context_save_clober_func,@function + .align 4 + +/* This function recursively calls itself via call4, making sure each frame + * uses only 4 registers. For this reason, recursion count can not be + * a function argument (it would have to be in a6) and is placed into .data + * section above. + */ +test_context_save_clober_func: + entry a1, 16 + + /* load recursion count from memory */ + movi a3, recursion_count + l32i a2, a3, 0 + + /* if it is zero, initialize it to 16 (=64 physical registers / 4 registers per call) */ + bnez a2, 1f + movi a2, 16 +1: + + /* decrement the counter and write it back */ + addi a2, a2, -1 + s32i a2, a3, 0 + + /* counter not zero? do a recursive call */ + beqz a2, wait + call4 test_context_save_clober_func + j end + +wait: + /* Counter has reached zero, and we have 16 frames on the stack. + * Delay for a few seconds, expecting in interrupt to happen. + */ + movi a3, 100000000 +1: + addi a3, a3, -1 + bnez a3, 1b + +end: + retw + +#endif // XCHAL_HAVE_WINDOWED +#endif // __XTENSA__ diff --git a/components/freertos/test/test_context_save_clobber.c b/components/freertos/test/test_context_save_clobber.c new file mode 100644 index 0000000000..7fd18cb57a --- /dev/null +++ b/components/freertos/test/test_context_save_clobber.c @@ -0,0 +1,43 @@ +#include "unity.h" +#include "esp_intr_alloc.h" + +#if defined(__XTENSA__) +#include "xtensa/config/core-isa.h" +#include "xtensa/hal.h" +#if defined(XCHAL_HAVE_WINDOWED) +/* Regression test for a0 register being corrupted in _xt_context_save. + * + * The idea in this test is to have a function which recursively calls itself + * with call4, eventually filling up all the register windows. At that point, + * it does some lengthy operation. If an interrupt occurs at that point, and + * corrupts a0 register of one of the windows, this will cause an exception + * when the recursive function returns. + */ + + +/* See test_context_save_clober_func.S */ +extern void test_context_save_clober_func(void); + +static void int_timer_handler(void *arg) +{ + xthal_set_ccompare(1, xthal_get_ccount() + 10000); + (*(int*) arg)++; +} + +TEST_CASE("context save doesn't corrupt return address register", "[freertos]") +{ + /* set up an interrupt */ + intr_handle_t ih; + int int_triggered = 0; + TEST_ESP_OK(esp_intr_alloc(ETS_INTERNAL_TIMER1_INTR_SOURCE, 0, int_timer_handler, &int_triggered, &ih)); + xthal_set_ccompare(1, xthal_get_ccount() + 10000); + + /* fill all the windows and delay a bit, waiting for an interrupt to happen */ + test_context_save_clober_func(); + + esp_intr_free(ih); + TEST_ASSERT_GREATER_THAN(0, int_triggered); +} + +#endif // XCHAL_HAVE_WINDOWED +#endif // __XTENSA__ diff --git a/components/freertos/xtensa/xtensa_context.S b/components/freertos/xtensa/xtensa_context.S index c46c8e4f2a..cb8c8ccbe3 100644 --- a/components/freertos/xtensa/xtensa_context.S +++ b/components/freertos/xtensa/xtensa_context.S @@ -156,7 +156,7 @@ _xt_context_save: movi a3, -XCHAL_EXTRA_SA_ALIGN and a2, a2, a3 /* align dynamically >16 bytes */ # endif - call0 xthal_save_extra_nw /* destroys a0,2,3,4,5 */ + call0 xthal_save_extra_nw /* destroys a0,2,3 */ #endif #ifndef __XTENSA_CALL0_ABI__ @@ -173,17 +173,23 @@ _xt_context_save: * at entry (CINTLEVEL=max(PS.INTLEVEL, XCHAL_EXCM_LEVEL) when PS.EXCM is set. * Since WindowOverflow exceptions will trigger inside SPILL_ALL_WINDOWS, * need to save/restore EPC1 as well. + * Note: even though a4-a15 are saved into the exception frame, we should not + * clobber them until after SPILL_ALL_WINDOWS. This is because these registers + * may contain live windows belonging to previous frames in the call stack. + * These frames will be spilled by SPILL_ALL_WINDOWS, and if the register was + * used as a temporary by this code, the temporary value would get stored + * onto the stack, instead of the real value. */ rsr a2, PS /* to be restored after SPILL_ALL_WINDOWS */ - movi a4, PS_INTLEVEL_MASK - and a3, a2, a4 /* get the current INTLEVEL */ + movi a0, PS_INTLEVEL_MASK + and a3, a2, a0 /* get the current INTLEVEL */ bgeui a3, XCHAL_EXCM_LEVEL, 1f /* calculate max(INTLEVEL, XCHAL_EXCM_LEVEL) */ movi a3, XCHAL_EXCM_LEVEL 1: - movi a4, PS_UM | PS_WOE /* clear EXCM, enable window overflow, set new INTLEVEL */ - or a3, a3, a4 + movi a0, PS_UM | PS_WOE /* clear EXCM, enable window overflow, set new INTLEVEL */ + or a3, a3, a0 wsr a3, ps - rsr a4, EPC1 /* to be restored after SPILL_ALL_WINDOWS */ + rsr a0, EPC1 /* to be restored after SPILL_ALL_WINDOWS */ addi sp, sp, XT_STK_FRMSZ /* go back to spill register region */ SPILL_ALL_WINDOWS /* place the live register windows there */ @@ -191,7 +197,7 @@ _xt_context_save: wsr a2, PS /* restore to the value at entry */ rsync - wsr a4, EPC1 /* likewise */ + wsr a0, EPC1 /* likewise */ #endif /* __XTENSA_CALL0_ABI__ */