Skip to content

Commit

Permalink
Merge branch 'fix/freertos_port_assert_in_isr_bug_v5.3' into 'release…
Browse files Browse the repository at this point in the history
…/v5.3'

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

See merge request espressif/esp-idf!32372
  • Loading branch information
ESP-Marius committed Jul 29, 2024
2 parents d434587 + d2e4722 commit 39a3d54
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand Down Expand Up @@ -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 )
Expand Down
10 changes: 8 additions & 2 deletions components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
| | |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -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
*/
Expand Down
10 changes: 8 additions & 2 deletions components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/port.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 )
Expand Down Expand Up @@ -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)
| | |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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
*/
Expand Down
6 changes: 6 additions & 0 deletions components/freertos/FreeRTOS-Kernel/portable/riscv/port.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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()

/**
Expand Down
9 changes: 5 additions & 4 deletions components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
| | |
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down Expand Up @@ -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
10 changes: 10 additions & 0 deletions components/freertos/test_apps/freertos/pytest_freertos.py
Original file line number Diff line number Diff line change
Expand Up @@ -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...')

0 comments on commit 39a3d54

Please sign in to comment.