From d294ac381f988efdf09f2404e200292db5ccd87d Mon Sep 17 00:00:00 2001 From: Marius Vikhammer Date: Tue, 9 Feb 2021 08:57:35 +0800 Subject: [PATCH 1/2] freertos: fix errors reported by PVS-Studio Removed leftover code-paths that were never taken. Upstream freertos uses vTaskSuspendAll() and xTaskResumeAll(), and therefor check if the task already yielded. In the IDF port of freertos we use critcal sections instead, so xAlreadyYielded will never be set. Partially addresses https://github.com/espressif/esp-idf/issues/6440 --- components/freertos/event_groups.c | 10 +--------- components/freertos/tasks.c | 29 ++++++----------------------- 2 files changed, 7 insertions(+), 32 deletions(-) diff --git a/components/freertos/event_groups.c b/components/freertos/event_groups.c index 9467d7c9c9..f1e1b362e6 100644 --- a/components/freertos/event_groups.c +++ b/components/freertos/event_groups.c @@ -198,7 +198,6 @@ EventBits_t xEventGroupSync( EventGroupHandle_t xEventGroup, const EventBits_t u { EventBits_t uxOriginalBitValue, uxReturn; EventGroup_t *pxEventBits = xEventGroup; -BaseType_t xAlreadyYielded = pdFALSE; BaseType_t xTimeoutOccurred = pdFALSE; configASSERT( ( uxBitsToWaitFor & eventEVENT_BITS_CONTROL_BYTES ) == 0 ); @@ -257,14 +256,7 @@ BaseType_t xTimeoutOccurred = pdFALSE; if( xTicksToWait != ( TickType_t ) 0 ) { - if( xAlreadyYielded == pdFALSE ) - { - portYIELD_WITHIN_API(); - } - else - { - mtCOVERAGE_TEST_MARKER(); - } + portYIELD_WITHIN_API(); /* The task blocked to wait for its required bits to be set - at this point either the required bits were set or the block time expired. If diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c index 7476014f2a..585224ca9a 100644 --- a/components/freertos/tasks.c +++ b/components/freertos/tasks.c @@ -1406,7 +1406,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode void vTaskDelayUntil( TickType_t * const pxPreviousWakeTime, const TickType_t xTimeIncrement ) { TickType_t xTimeToWake; - BaseType_t xAlreadyYielded = pdFALSE, xShouldDelay = pdFALSE; + BaseType_t xShouldDelay = pdFALSE; configASSERT( pxPreviousWakeTime ); configASSERT( ( xTimeIncrement > 0U ) ); @@ -1470,16 +1470,8 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode } taskEXIT_CRITICAL( &xTaskQueueMutex ); - /* Force a reschedule if xTaskResumeAll has not already done so, we may - have put ourselves to sleep. */ - if( xAlreadyYielded == pdFALSE ) - { - portYIELD_WITHIN_API(); - } - else - { - mtCOVERAGE_TEST_MARKER(); - } + /* Force a reschedule, we may have put ourselves to sleep. */ + portYIELD_WITHIN_API(); } #endif /* INCLUDE_vTaskDelayUntil */ @@ -1489,8 +1481,6 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode void vTaskDelay( const TickType_t xTicksToDelay ) { - BaseType_t xAlreadyYielded = pdFALSE; - /* A delay time of zero just forces a reschedule. */ if( xTicksToDelay > ( TickType_t ) 0U ) { @@ -1515,18 +1505,11 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode mtCOVERAGE_TEST_MARKER(); } - /* Force a reschedule if xTaskResumeAll has not already done so, we may - have put ourselves to sleep. */ - if( xAlreadyYielded == pdFALSE ) - { - portYIELD_WITHIN_API(); - } - else - { - mtCOVERAGE_TEST_MARKER(); - } + /* Force a reschedule, we may have put ourselves to sleep. */ + portYIELD_WITHIN_API(); } + #endif /* INCLUDE_vTaskDelay */ /*-----------------------------------------------------------*/ From 4eb9cc68a6136b84cac4fe68a5aadfcb936bcae8 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 11 Feb 2021 17:00:55 +1100 Subject: [PATCH 2/2] esp_event test: Disable linker relaxations in this component, to temporarily workaround a linker bug --- components/esp_event/test/CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/esp_event/test/CMakeLists.txt b/components/esp_event/test/CMakeLists.txt index 44e4b7ec84..b6690eb027 100644 --- a/components/esp_event/test/CMakeLists.txt +++ b/components/esp_event/test/CMakeLists.txt @@ -1,3 +1,8 @@ idf_component_register(SRC_DIRS "." PRIV_INCLUDE_DIRS . ../private_include PRIV_REQUIRES cmock test_utils esp_event driver) + +if(CONFIG_IDF_TARGET_ARCH_RISCV) + # Temporary workaround for a linker issue on RISC-V that should be resolved in binutils 2.35 (internal ref: GCC-101) + target_compile_options(${COMPONENT_LIB} PRIVATE -mno-relax) +endif()