From 4a59036ab12dc6384d84c7805024fb82e82ec2ce Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Mon, 9 May 2022 15:45:38 +0530 Subject: [PATCH 1/2] freertos: fix allocation order for stack and TCB per portSTACK_GROWTH This is as per FreeRTOS recommendation and allows to protect task TCB in case task stack has overflowed. --- components/freertos/port/port_common.c | 42 ++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/components/freertos/port/port_common.c b/components/freertos/port/port_common.c index ffca3d5429..e2edf04bc6 100644 --- a/components/freertos/port/port_common.c +++ b/components/freertos/port/port_common.c @@ -161,9 +161,24 @@ void vApplicationGetIdleTaskMemory(StaticTask_t **ppxIdleTaskTCBBuffer, { StaticTask_t *pxTCBBufferTemp; StackType_t *pxStackBufferTemp; - //Allocate TCB and stack buffer in internal memory - pxTCBBufferTemp = pvPortMallocTcbMem(sizeof(StaticTask_t)); - pxStackBufferTemp = pvPortMallocStackMem(configIDLE_TASK_STACK_SIZE); + + /* If the stack grows down then allocate the stack then the TCB so the stack + * does not grow into the TCB. Likewise if the stack grows up then allocate + * the TCB then the stack. */ + #if (portSTACK_GROWTH > 0) + { + //Allocate TCB and stack buffer in internal memory + pxTCBBufferTemp = pvPortMallocTcbMem(sizeof(StaticTask_t)); + pxStackBufferTemp = pvPortMallocStackMem(configIDLE_TASK_STACK_SIZE); + } + #else /* portSTACK_GROWTH */ + { + //Allocate TCB and stack buffer in internal memory + pxStackBufferTemp = pvPortMallocStackMem(configIDLE_TASK_STACK_SIZE); + pxTCBBufferTemp = pvPortMallocTcbMem(sizeof(StaticTask_t)); + } + #endif /* portSTACK_GROWTH */ + assert(pxTCBBufferTemp != NULL); assert(pxStackBufferTemp != NULL); //Write back pointers @@ -186,9 +201,24 @@ void vApplicationGetTimerTaskMemory(StaticTask_t **ppxTimerTaskTCBBuffer, { StaticTask_t *pxTCBBufferTemp; StackType_t *pxStackBufferTemp; - //Allocate TCB and stack buffer in internal memory - pxTCBBufferTemp = pvPortMallocTcbMem(sizeof(StaticTask_t)); - pxStackBufferTemp = pvPortMallocStackMem(configTIMER_TASK_STACK_DEPTH); + + /* If the stack grows down then allocate the stack then the TCB so the stack + * does not grow into the TCB. Likewise if the stack grows up then allocate + * the TCB then the stack. */ + #if (portSTACK_GROWTH > 0) + { + //Allocate TCB and stack buffer in internal memory + pxTCBBufferTemp = pvPortMallocTcbMem(sizeof(StaticTask_t)); + pxStackBufferTemp = pvPortMallocStackMem(configTIMER_TASK_STACK_DEPTH); + } + #else /* portSTACK_GROWTH */ + { + //Allocate TCB and stack buffer in internal memory + pxStackBufferTemp = pvPortMallocStackMem(configTIMER_TASK_STACK_DEPTH); + pxTCBBufferTemp = pvPortMallocTcbMem(sizeof(StaticTask_t)); + } + #endif /* portSTACK_GROWTH */ + assert(pxTCBBufferTemp != NULL); assert(pxStackBufferTemp != NULL); //Write back pointers From 5b6d9d87a37b4eb280046cca99bb1c5231d97b63 Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Mon, 9 May 2022 15:47:13 +0530 Subject: [PATCH 2/2] freertos: add return value to API `vTaskGetSnapshot` `vTaskGetSnapshot` is being used in coredump module to collect diagnostic information. It is possible that input arguments are invalid and `assert` in this situation is not correct. This commit modifies API signature to return pdTRUE in case of success, and pdFALSE otherwise. Caller can verify return value and then take appropriate decision. --- components/freertos/esp_additions/task_snapshot.c | 10 ++++++---- .../include/esp_additions/freertos/task_snapshot.h | 3 ++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/components/freertos/esp_additions/task_snapshot.c b/components/freertos/esp_additions/task_snapshot.c index 1244118b60..8c394126e9 100644 --- a/components/freertos/esp_additions/task_snapshot.c +++ b/components/freertos/esp_additions/task_snapshot.c @@ -116,13 +116,15 @@ return NULL; } - void vTaskGetSnapshot( TaskHandle_t pxTask, TaskSnapshot_t *pxTaskSnapshot ) - { - configASSERT( portVALID_TCB_MEM(pxTask) ); - configASSERT( pxTaskSnapshot != NULL ); + BaseType_t vTaskGetSnapshot( TaskHandle_t pxTask, TaskSnapshot_t *pxTaskSnapshot ) +{ + if (portVALID_TCB_MEM(pxTask) == false || pxTaskSnapshot == NULL) { + return pdFALSE; + } pxTaskSnapshot->pxTCB = (void*) pxTask; pxTaskSnapshot->pxTopOfStack = pxTCBGetTopOfStack((void*) pxTask); pxTaskSnapshot->pxEndOfStack = pxTCBGetEndOfStack((void*) pxTask); + return pdTRUE; } TaskHandle_t pxTaskGetNext( TaskHandle_t pxTask ) diff --git a/components/freertos/include/esp_additions/freertos/task_snapshot.h b/components/freertos/include/esp_additions/freertos/task_snapshot.h index 1ad04cce69..635a515025 100644 --- a/components/freertos/include/esp_additions/freertos/task_snapshot.h +++ b/components/freertos/include/esp_additions/freertos/task_snapshot.h @@ -80,8 +80,9 @@ TaskHandle_t pxTaskGetNext( TaskHandle_t pxTask ); * @note This function should not be used while FreeRTOS is running (as it doesn't acquire any locks). * @param pxTask task handle. * @param pxTaskSnapshot address of TaskSnapshot_t structure to fill. + * @return pdTRUE if operation was successful else pdFALSE */ -void vTaskGetSnapshot( TaskHandle_t pxTask, TaskSnapshot_t *pxTaskSnapshot ); +BaseType_t vTaskGetSnapshot( TaskHandle_t pxTask, TaskSnapshot_t *pxTaskSnapshot ); #ifdef __cplusplus }