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

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

See merge request espressif/esp-idf!32372

Author: ESP-Marius

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

@@ -113,6 +113,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 --------------------
 
 #if ( configNUMBER_OF_CORES > 1 )
@@ -187,6 +194,15 @@ void vPortTCBPreDeleteHook( 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 --------------------
 
 #if ( configNUMBER_OF_CORES > 1 )

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

@@ -168,6 +168,12 @@ BaseType_t xPortCheckIfInISR(void)
     return uxInterruptNesting;
 }
 
+void vPortAssertIfInISR(void)
+{
+    /* Assert if the interrupt nesting count is > 0 */
+    configASSERT(xPortCheckIfInISR() == 0);
+}
+
 // ------------------ Critical Sections --------------------
 
 #if ( configNUMBER_OF_CORES > 1 )
@@ -373,7 +379,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);
@@ -440,7 +446,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 );
@@ -161,6 +168,14 @@ void vPortTCBPreDeleteHook( void *pxTCB );
 #define portSET_INTERRUPT_MASK_FROM_ISR()   portSET_INTERRUPT_MASK()
 #define portDISABLE_INTERRUPTS()            portSET_INTERRUPT_MASK()
 
+/**
+ * @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 to set the value of the INTLEVEL field ourselves
 */

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

@@ -139,7 +139,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);
@@ -204,6 +204,12 @@ BaseType_t xPortCheckIfInISR(void)
     return ret;
 }
 
+void vPortAssertIfInISR(void)
+{
+    /* Assert if the interrupt nesting count is > 0 */
+    configASSERT(xPortCheckIfInISR() == 0);
+}
+
 // ------------------ Critical Sections --------------------
 
 #if ( configNUMBER_OF_CORES > 1 )
@@ -614,7 +620,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

@@ -166,6 +166,13 @@ void vPortClearInterruptMaskFromISR(UBaseType_t prev_int_level);
  */
 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
  *
@@ -478,6 +485,11 @@ void vPortTCBPreDeleteHook( void *pxTCB );
 #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()
+
 /**
  * @brief Used by FreeRTOS functions to call the correct version of critical section API
  */

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

@@ -462,6 +462,12 @@ BaseType_t xPortInIsrContext(void)
 #endif /* (configNUM_CORES > 1) */
 }
 
+void vPortAssertIfInISR(void)
+{
+    /* Assert if the interrupt nesting count is > 0 */
+    configASSERT(xPortInIsrContext() == 0);
+}
+
 BaseType_t IRAM_ATTR xPortInterruptedFromISRContext(void)
 {
     /* Return the interrupt nesting counter for this core */

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

@@ -141,12 +141,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);
 
@@ -427,6 +424,9 @@ void vPortTCBPreDeleteHook( void *pxTCB );
 #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

@@ -8,7 +8,7 @@
  *
  * SPDX-License-Identifier: MIT
  *
- * SPDX-FileContributor: 2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileContributor: 2023-2024 Espressif Systems (Shanghai) CO LTD
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy of
  * this software and associated documentation files (the "Software"), to deal in
@@ -395,7 +395,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)
     |             |             |
@@ -449,7 +449,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)
@@ -489,7 +490,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
  */
@@ -58,3 +58,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

@@ -39,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...')