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

This commit fixes an issue where in the FreeRTOS port layer would cause
the portASSERT_IF_IN_ISR() assert check to fail even when the system is
not in an interrupt context.
This commit is contained in:
Sudeep Mohanty 2024-07-24 16:56:17 +02:00
parent e40386bf4a
commit 1012172436
10 changed files with 102 additions and 13 deletions

View File

@ -113,6 +113,13 @@ typedef spinlock_t portMUX_TYPE; /**< Spi
BaseType_t xPortCheckIfInISR(void); BaseType_t xPortCheckIfInISR(void);
/**
* @brief Assert if in ISR context
*
* - Asserts on xPortCheckIfInISR() internally
*/
void vPortAssertIfInISR(void);
// ------------------ Critical Sections -------------------- // ------------------ Critical Sections --------------------
#if ( configNUMBER_OF_CORES > 1 ) #if ( configNUMBER_OF_CORES > 1 )
@ -187,6 +194,15 @@ void vPortTCBPreDeleteHook( void *pxTCB );
#define portENABLE_INTERRUPTS() vPortClearInterruptMask(1) #define portENABLE_INTERRUPTS() vPortClearInterruptMask(1)
#define portRESTORE_INTERRUPTS(x) vPortClearInterruptMask(x) #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 -------------------- // ------------------ Critical Sections --------------------
#if ( configNUMBER_OF_CORES > 1 ) #if ( configNUMBER_OF_CORES > 1 )

View File

@ -168,6 +168,12 @@ BaseType_t xPortCheckIfInISR(void)
return uxInterruptNesting; return uxInterruptNesting;
} }
void vPortAssertIfInISR(void)
{
/* Assert if the interrupt nesting count is > 0 */
configASSERT(xPortCheckIfInISR() == 0);
}
// ------------------ Critical Sections -------------------- // ------------------ Critical Sections --------------------
#if ( configNUMBER_OF_CORES > 1 ) #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 #if CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER
static void vPortTaskWrapper(TaskFunction_t pxCode, void *pvParameters) 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); extern void __attribute__((noreturn)) panic_abort(const char *details);
static char DRAM_ATTR msg[80] = "FreeRTOS: FreeRTOS Task \"\0"; static char DRAM_ATTR msg[80] = "FreeRTOS: FreeRTOS Task \"\0";
pxCode(pvParameters); pxCode(pvParameters);
@ -440,7 +446,7 @@ StackType_t *pxPortInitialiseStack(StackType_t *pxTopOfStack, TaskFunction_t pxC
HIGH ADDRESS HIGH ADDRESS
|---------------------------| <- pxTopOfStack on entry |---------------------------| <- pxTopOfStack on entry
| TLS Variables | | TLS Variables |
| ------------------------- | <- Start of useable stack | ------------------------- | <- Start of usable stack
| Starting stack frame | | Starting stack frame |
| ------------------------- | <- pxTopOfStack on return (which is the tasks current SP) | ------------------------- | <- pxTopOfStack on return (which is the tasks current SP)
| | | | | |

View File

@ -95,6 +95,13 @@ typedef spinlock_t portMUX_TYPE; /**< Spi
BaseType_t xPortCheckIfInISR(void); BaseType_t xPortCheckIfInISR(void);
/**
* @brief Assert if in ISR context
*
* - Asserts on xPortCheckIfInISR() internally
*/
void vPortAssertIfInISR(void);
// ------------------ Critical Sections -------------------- // ------------------ Critical Sections --------------------
UBaseType_t uxPortEnterCriticalFromISR( void ); UBaseType_t uxPortEnterCriticalFromISR( void );
@ -161,6 +168,14 @@ void vPortTCBPreDeleteHook( void *pxTCB );
#define portSET_INTERRUPT_MASK_FROM_ISR() portSET_INTERRUPT_MASK() #define portSET_INTERRUPT_MASK_FROM_ISR() portSET_INTERRUPT_MASK()
#define portDISABLE_INTERRUPTS() 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 Note: XTOS_RESTORE_INTLEVEL() will overwrite entire PS register on XEA2. So we need to set the value of the INTLEVEL field ourselves
*/ */

View File

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

View File

@ -166,6 +166,13 @@ void vPortClearInterruptMaskFromISR(UBaseType_t prev_int_level);
*/ */
BaseType_t xPortInIsrContext(void); 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 * @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 portSET_INTERRUPT_MASK_FROM_ISR() xPortSetInterruptMaskFromISR()
#define portCLEAR_INTERRUPT_MASK_FROM_ISR(prev_level) vPortClearInterruptMaskFromISR(prev_level) #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 * @brief Used by FreeRTOS functions to call the correct version of critical section API
*/ */

View File

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

View File

@ -141,12 +141,9 @@ typedef uint32_t TickType_t;
BaseType_t xPortInIsrContext(void); BaseType_t xPortInIsrContext(void);
/** /**
* @brief Asserts if in ISR context * @brief Assert if in ISR context
* *
* - Asserts on xPortInIsrContext() internally * - 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); void vPortAssertIfInISR(void);
@ -427,6 +424,9 @@ void vPortTCBPreDeleteHook( void *pxTCB );
#define portSET_INTERRUPT_MASK_FROM_ISR() xPortSetInterruptMaskFromISR() #define portSET_INTERRUPT_MASK_FROM_ISR() xPortSetInterruptMaskFromISR()
#define portCLEAR_INTERRUPT_MASK_FROM_ISR(prev_level) vPortClearInterruptMaskFromISR(prev_level) #define portCLEAR_INTERRUPT_MASK_FROM_ISR(prev_level) vPortClearInterruptMaskFromISR(prev_level)
/**
* @brief Assert if in ISR context
*/
#define portASSERT_IF_IN_ISR() vPortAssertIfInISR() #define portASSERT_IF_IN_ISR() vPortAssertIfInISR()
/** /**

View File

@ -8,7 +8,7 @@
* *
* SPDX-License-Identifier: MIT * 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 * 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 * 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) | Coproc Save Area | (CPSA MUST BE FIRST)
| ------------------------- | | ------------------------- |
| TLS Variables | | TLS Variables |
| ------------------------- | <- Start of useable stack | ------------------------- | <- Start of usable stack
| Starting stack frame | | Starting stack frame |
| ------------------------- | <- pxTopOfStack on return (which is the tasks current SP) | ------------------------- | <- pxTopOfStack on return (which is the tasks current SP)
| | | | | |
@ -449,7 +449,8 @@ BaseType_t xPortInIsrContext(void)
void vPortAssertIfInISR(void) void vPortAssertIfInISR(void)
{ {
configASSERT(xPortInIsrContext()); /* Assert if the interrupt nesting count is > 0 */
configASSERT(xPortInIsrContext() == 0);
} }
BaseType_t IRAM_ATTR xPortInterruptedFromISRContext(void) 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) void __attribute__((optimize("-O3"))) vPortExitCritical(portMUX_TYPE *mux)
{ {
/* This function may be called in a nested manner. Therefore, we only need /* 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. * can use the nesting count to determine whether this is the last exit call.
*/ */
spinlock_release(mux); spinlock_release(mux);

View File

@ -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 * SPDX-License-Identifier: Apache-2.0
*/ */
@ -58,3 +58,20 @@ TEST_CASE("xPortInIsrContext test", "[freertos]")
} }
#endif #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

View File

@ -41,3 +41,13 @@ def test_task_notify_wait_too_high_index_fails(dut: Dut) -> None:
dut.expect('assert failed: xTaskGenericNotifyWait', timeout=5) dut.expect('assert failed: xTaskGenericNotifyWait', timeout=5)
dut.expect('uxIndexToWait < [0-9]+', timeout=5) dut.expect('uxIndexToWait < [0-9]+', timeout=5)
dut.expect_exact('Rebooting...') 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...')