Merge branch 'fix/freertos_port_assert_in_isr_bug_v5.1' into 'release/v5.1'

fix(freertos): Incorrect assert in FreeRTOS port layer when not in ISR context (v5.1)

See merge request espressif/esp-idf!32374

components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/include/freertos/portmacro.h

@@ -85,6 +85,13 @@ typedef spinlock_t                          portMUX_TYPE;               /**< Spi
 
 BaseType_t xPortCheckIfInISR(void);
 
+/**
+ * @brief Assert if in ISR context
+ *
+ * - Asserts on xPortCheckIfInISR() internally
+ */
+void vPortAssertIfInISR(void);
+
 // ------------------ Critical Sections --------------------
 
 /*
@@ -148,6 +155,15 @@ void vPortCleanUpTCB ( void *pxTCB );
 #define portENABLE_INTERRUPTS()                     vPortClearInterruptMask(1)
 #define portRESTORE_INTERRUPTS(x)                   vPortClearInterruptMask(x)
 
+/**
+ * @brief Assert if in ISR context
+ *
+ * TODO: Enable once ISR safe version of vTaskEnter/ExitCritical() is implemented
+ * for single-core SMP FreeRTOS Kernel. (IDF-10540)
+ */
+// #define portASSERT_IF_IN_ISR() vPortAssertIfInISR()
+
+
 // ------------------ Critical Sections --------------------
 
 #define portGET_TASK_LOCK()                         vPortTakeLock(&port_xTaskLock)

components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c

@@ -279,6 +279,12 @@ BaseType_t xPortCheckIfInISR(void)
     return uxInterruptNesting;
 }
 
+void vPortAssertIfInISR(void)
+{
+    /* Assert if the interrupt nesting count is > 0 */
+    configASSERT(xPortCheckIfInISR() == 0);
+}
+
 // ------------------ Critical Sections --------------------
 
 void IRAM_ATTR vPortTakeLock( portMUX_TYPE *lock )
@@ -438,7 +444,7 @@ FORCE_INLINE_ATTR UBaseType_t uxInitialiseStackTLS(UBaseType_t uxStackPointer, u
 #if CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER
 static void vPortTaskWrapper(TaskFunction_t pxCode, void *pvParameters)
 {
-    __asm__ volatile(".cfi_undefined ra");  // tell to debugger that it's outermost (inital) frame
+    __asm__ volatile(".cfi_undefined ra");  // tell to debugger that it's outermost (initial) frame
     extern void __attribute__((noreturn)) panic_abort(const char *details);
     static char DRAM_ATTR msg[80] = "FreeRTOS: FreeRTOS Task \"\0";
     pxCode(pvParameters);
@@ -505,7 +511,7 @@ StackType_t *pxPortInitialiseStack(StackType_t *pxTopOfStack, TaskFunction_t pxC
     HIGH ADDRESS
     |---------------------------| <- pxTopOfStack on entry
     | TLS Variables             |
-    | ------------------------- | <- Start of useable stack
+    | ------------------------- | <- Start of usable stack
     | Starting stack frame      |
     | ------------------------- | <- pxTopOfStack on return (which is the tasks current SP)
     |             |             |

components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/include/freertos/portmacro.h

@@ -95,6 +95,13 @@ typedef spinlock_t                          portMUX_TYPE;               /**< Spi
 
 BaseType_t xPortCheckIfInISR(void);
 
+/**
+ * @brief Assert if in ISR context
+ *
+ * - Asserts on xPortCheckIfInISR() internally
+ */
+void vPortAssertIfInISR(void);
+
 // ------------------ Critical Sections --------------------
 
 UBaseType_t uxPortEnterCriticalFromISR( void );
@@ -152,6 +159,14 @@ void vPortCleanUpTCB ( void *pxTCB );
     XTOS_SET_INTLEVEL(0); \
 })
 
+/**
+ * @brief Assert if in ISR context
+ *
+ * TODO: Enable once ISR safe version of vTaskEnter/ExitCritical() is implemented
+ * for single-core SMP FreeRTOS Kernel. (IDF-10540)
+ */
+// #define portASSERT_IF_IN_ISR() vPortAssertIfInISR()
+
 /*
 Note: XTOS_RESTORE_INTLEVEL() will overwrite entire PS register on XEA2. So we need ot make the value INTLEVEL field ourselves
 */

components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/port.c

@@ -127,7 +127,7 @@ BaseType_t xPortEnterCriticalTimeout(portMUX_TYPE *lock, BaseType_t timeout)
 void vPortExitCriticalIDF(portMUX_TYPE *lock)
 {
     /* This function may be called in a nested manner. Therefore, we only need
-     * to reenable interrupts if this is the last call to exit the critical. We
+     * to re-enable interrupts if this is the last call to exit the critical. We
      * can use the nesting count to determine whether this is the last exit call.
      */
     spinlock_release(lock);
@@ -330,6 +330,12 @@ BaseType_t xPortCheckIfInISR(void)
     return ret;
 }
 
+void vPortAssertIfInISR(void)
+{
+    /* Assert if the interrupt nesting count is > 0 */
+    configASSERT(xPortCheckIfInISR() == 0);
+}
+
 // ------------------ Critical Sections --------------------
 
 void vPortTakeLock( portMUX_TYPE *lock )
@@ -662,7 +668,7 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack,
     | Coproc Save Area          | (CPSA MUST BE FIRST)
     | ------------------------- |
     | TLS Variables             |
-    | ------------------------- | <- Start of useable stack
+    | ------------------------- | <- Start of usable stack
     | Starting stack frame      |
     | ------------------------- | <- pxTopOfStack on return (which is the tasks current SP)
     |             |             |

components/freertos/FreeRTOS-Kernel/portable/riscv/include/freertos/portmacro.h

@@ -133,6 +133,13 @@ typedef uint32_t TickType_t;
  */
 BaseType_t xPortInIsrContext(void);
 
+/**
+ * @brief Assert if in ISR context
+ *
+ * - Asserts on xPortInIsrContext() internally
+ */
+void vPortAssertIfInISR(void);
+
 /**
  * @brief Check if in ISR context from High priority ISRs
  *
@@ -310,6 +317,11 @@ FORCE_INLINE_ATTR BaseType_t xPortGetCoreID(void)
 #define portSET_INTERRUPT_MASK_FROM_ISR()                       vPortSetInterruptMask()
 #define portCLEAR_INTERRUPT_MASK_FROM_ISR(uxSavedStatusValue)   vPortClearInterruptMask(uxSavedStatusValue)
 
+/**
+ * @brief Assert if in ISR context
+ */
+#define portASSERT_IF_IN_ISR() vPortAssertIfInISR()
+
 // ------------------ Critical Sections --------------------
 
 #define portENTER_CRITICAL(mux)                 {(void)mux;  vPortEnterCritical();}

components/freertos/FreeRTOS-Kernel/portable/riscv/port.c

@@ -306,6 +306,12 @@ BaseType_t xPortInIsrContext(void)
     return uxInterruptNesting;
 }
 
+void vPortAssertIfInISR(void)
+{
+    /* Assert if the interrupt nesting count is > 0 */
+    configASSERT(xPortInIsrContext() == 0);
+}
+
 BaseType_t IRAM_ATTR xPortInterruptedFromISRContext(void)
 {
     /* For single core, this can be the same as xPortInIsrContext() because reading it is atomic */

components/freertos/FreeRTOS-Kernel/portable/xtensa/include/freertos/portmacro.h

@@ -166,12 +166,9 @@ typedef uint32_t TickType_t;
 BaseType_t xPortInIsrContext(void);
 
 /**
- * @brief Asserts if in ISR context
+ * @brief Assert if in ISR context
  *
  * - Asserts on xPortInIsrContext() internally
- *
- * @note [refactor-todo] Check if this API is still required
- * @note [refactor-todo] Check if this should be inlined
  */
 void vPortAssertIfInISR(void);
 
@@ -431,6 +428,9 @@ FORCE_INLINE_ATTR BaseType_t xPortGetCoreID(void);
 #define portSET_INTERRUPT_MASK_FROM_ISR()                   xPortSetInterruptMaskFromISR()
 #define portCLEAR_INTERRUPT_MASK_FROM_ISR(prev_level)       vPortClearInterruptMaskFromISR(prev_level)
 
+/**
+ * @brief Assert if in ISR context
+ */
 #define portASSERT_IF_IN_ISR() vPortAssertIfInISR()
 
 /**

components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c

@@ -423,7 +423,7 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack,
     | Coproc Save Area          | (CPSA MUST BE FIRST)
     | ------------------------- |
     | TLS Variables             |
-    | ------------------------- | <- Start of useable stack
+    | ------------------------- | <- Start of usable stack
     | Starting stack frame      |
     | ------------------------- | <- pxTopOfStack on return (which is the tasks current SP)
     |             |             |
@@ -477,7 +477,8 @@ BaseType_t xPortInIsrContext(void)
 
 void vPortAssertIfInISR(void)
 {
-    configASSERT(xPortInIsrContext());
+    /* Assert if the interrupt nesting count is > 0 */
+    configASSERT(xPortInIsrContext() == 0);
 }
 
 BaseType_t IRAM_ATTR xPortInterruptedFromISRContext(void)
@@ -517,7 +518,7 @@ BaseType_t __attribute__((optimize("-O3"))) xPortEnterCriticalTimeout(portMUX_TY
 void __attribute__((optimize("-O3"))) vPortExitCritical(portMUX_TYPE *mux)
 {
     /* This function may be called in a nested manner. Therefore, we only need
-     * to reenable interrupts if this is the last call to exit the critical. We
+     * to re-enable interrupts if this is the last call to exit the critical. We
      * can use the nesting count to determine whether this is the last exit call.
      */
     spinlock_release(mux);

components/freertos/test_apps/freertos/port/test_freertos_isinisrcontext.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -62,3 +62,20 @@ TEST_CASE("xPortInIsrContext test", "[freertos]")
 
 
 #endif
+
+#if !CONFIG_FREERTOS_SMP // TODO: Enable when IDF-10540 is fixed
+
+static void testint_assert(void)
+{
+    esp_rom_printf("INT!\n");
+    portASSERT_IF_IN_ISR();
+}
+
+TEST_CASE("port must assert if in ISR context", "[ignore]")
+{
+    esp_err_t err = esp_register_freertos_tick_hook_for_cpu(testint_assert, xPortGetCoreID());
+    TEST_ASSERT_EQUAL_HEX32(ESP_OK, err);
+    vTaskDelay(100 / portTICK_PERIOD_MS);
+    esp_deregister_freertos_tick_hook_for_cpu(testint_assert, xPortGetCoreID());
+}
+#endif // !CONFIG_FREERTOS_SMP

components/freertos/test_apps/freertos/pytest_freertos.py

@@ -1,6 +1,5 @@
 # SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
 # SPDX-License-Identifier: Apache-2.0
-
 import pytest
 from pytest_embedded import Dut
 
@@ -40,3 +39,13 @@ def test_task_notify_wait_too_high_index_fails(dut: Dut) -> None:
     dut.expect('assert failed: xTaskGenericNotifyWait', timeout=5)
     dut.expect('uxIndexToWait < [0-9]+', timeout=5)
     dut.expect_exact('Rebooting...')
+
+
+@pytest.mark.supported_targets
+@pytest.mark.generic
+@pytest.mark.parametrize('config', ['default'], indirect=True)
+def test_port_must_assert_in_isr(dut: Dut) -> None:
+    dut.expect_exact('Press ENTER to see the list of tests.')
+    dut.write('\"port must assert if in ISR context\"')
+    dut.expect('assert failed: vPortAssertIfInISR', timeout=5)
+    dut.expect_exact('Rebooting...')