Merge branch 'bugfix/window_spill_a0_corruption' into 'master'

freertos: don't clobber a4 while spilling register windows

Closes IDFGH-3852 and IDF-1935

See merge request espressif/esp-idf!10252

components/cxx/test/test_cxx.cpp

@@ -251,6 +251,60 @@ TEST_CASE("c++ exceptions emergency pool", "[cxx] [exceptions] [ignore] [leaks="
 #endif
 }
 
+
+#define TIMEOUT 19
+
+#define RECURSION 19
+
+static esp_timer_handle_t crash_timer;
+
+static uint32_t result = 0;
+
+uint32_t calc_fac(uint32_t n) {
+    if (n == 1 || n == 0) {
+        return 1;
+    } else {
+        return n * calc_fac(n - 1);
+    }
+}
+
+static void timer_cb(void *arg) {
+    result = calc_fac(RECURSION);
+}
+
+// TODO: Not a unit test, refactor to integration test/system test, etc.
+TEST_CASE("frequent interrupts don't interfere with c++ exceptions", "[cxx] [exceptions] [leaks=" LEAKS "]")
+{// if exception workaround is disabled, this is almost guaranteed to fail
+    const esp_timer_create_args_t timer_args {
+        timer_cb,
+        NULL,
+        ESP_TIMER_TASK,
+        "crash_timer"
+    };
+
+    TEST_ESP_OK(esp_timer_create(&timer_args, &crash_timer));
+    TEST_ESP_OK(esp_timer_start_periodic(crash_timer, TIMEOUT));
+
+    for (int i = 0; i < 500; i++) {
+        bool thrown_value = false;
+        try {
+            throw true;
+        } catch (bool e) {
+            thrown_value = e;
+        }
+
+        if (thrown_value) {
+            printf("ex thrown %d\n", i);
+        } else {
+            printf("ex not thrown\n");
+            TEST_ASSERT(false);
+        }
+    }
+
+    TEST_ESP_OK(esp_timer_stop(crash_timer));
+    TEST_ESP_OK(esp_timer_delete(crash_timer));
+}
+
 #else // !CONFIG_COMPILER_CXX_EXCEPTIONS
 
 TEST_CASE("std::out_of_range exception when -fno-exceptions", "[cxx][reset=abort,SW_CPU_RESET]")

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__

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