freertos: don't clobber a4 while spilling register windows

Commit 891eb3b0 was fixing an issue with PS and EPC1 not being
preserved after the window spill procedure. It did so by saving PS in
a2 and EPC1 in a4. However the a4 register may be a live register of
another window in the call stack, and if it is overwritten and then
spilled to the stack, then the corresponding register value will end
up being corrupted. In practice the problem would show up as an
IllegalInstruction exception, when trying to return from a function
when a0 value was 0x40020.
Fix by using a0 register instead of a4 as scratch. Also fix a comment
about xthal_save_extra_nw, as this function in fact doesn't clobber
a4 or a5 because XCHAL_NCP_NUM_ATMPS is defined as 1.

Closes https://github.com/espressif/esp-idf/issues/5758

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__

components/freertos/test/test_context_save_clobber_asm.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__

components/freertos/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__ */