Merge branch 'bugfix/freertos_include_coreid' into 'master'

freertos: Fix include coreid regression, add new UT configs

See merge request espressif/esp-idf!10864

components/freertos/queue.c

@@ -770,6 +770,11 @@ Queue_t * const pxQueue = xQueue;
 	}
 	#endif
 
+#if ( configUSE_MUTEXES == 1 && configCHECK_MUTEX_GIVEN_BY_OWNER == 1)
+	configASSERT(pxQueue->uxQueueType != queueQUEUE_IS_MUTEX
+				 || pxQueue->u.xSemaphore.xMutexHolder == NULL
+				 || pxQueue->u.xSemaphore.xMutexHolder == xTaskGetCurrentTaskHandle());
+#endif
 
 	/*lint -save -e904 This function relaxes the coding standard somewhat to
 	allow return statements within the function itself.  This is done in the

components/freertos/tasks.c

@@ -637,8 +637,9 @@ void taskYIELD_OTHER_CORE( BaseType_t xCoreID, UBaseType_t uxPriority )
 	TCB_t *pxNewTCB;
 	TaskHandle_t xReturn;
 
-		configASSERT( pxStackBuffer != NULL );
-		configASSERT( pxTaskBuffer != NULL );
+		configASSERT( portVALID_TCB_MEM(pxTaskBuffer) );
+		configASSERT( portVALID_STACK_MEM(pxStackBuffer) );
+		configASSERT( (xCoreID>=0 && xCoreID<portNUM_PROCESSORS) || (xCoreID==tskNO_AFFINITY) );
 
 		#if( configASSERT_DEFINED == 1 )
 		{
@@ -4134,7 +4135,7 @@ static void prvCheckTasksWaitingTermination( void )
 		pxTaskStatus->xTaskNumber = pxTCB->uxTCBNumber;
 
 		#if ( configTASKLIST_INCLUDE_COREID == 1 )
-		pxTaskStatus.xCoreID = pxTCB->xCoreID;
+		pxTaskStatus->xCoreID = pxTCB->xCoreID;
 		#endif /* configTASKLIST_INCLUDE_COREID */
 
 		#if ( configUSE_MUTEXES == 1 )

components/freertos/test/test_context_save_clobber.c

@@ -1,7 +1,8 @@
+#include "sdkconfig.h"
 #include "unity.h"
 #include "esp_intr_alloc.h"
 
-#if defined(__XTENSA__)
+#if defined(__XTENSA__) && CONFIG_FREERTOS_CORETIMER_0
 #include "xtensa/config/core-isa.h"
 #include "xtensa/hal.h"
 #if defined(XCHAL_HAVE_WINDOWED)
@@ -40,4 +41,4 @@ TEST_CASE("context save doesn't corrupt return address register", "[freertos]")
 }
 
 #endif // XCHAL_HAVE_WINDOWED
-#endif // __XTENSA__
+#endif // __XTENSA__ && CONFIG_FREERTOS_CORETIMER_0

components/freertos/test/test_freertos_backported_functions.c

@@ -216,6 +216,16 @@ TEST_CASE("Test FreeRTOS backported eventgroup functions", "[freertos]")
 //The variables pointed to by Thread Local Storage Pointer
 static uint32_t task_storage[portNUM_PROCESSORS][NO_OF_TLSP] = {0};
 
+/* If static task cleanup is defined, can't set index 0 even if the calling task is not a pthread,
+   as the cleanup is called for every task.
+*/
+#if defined(CONFIG_FREERTOS_ENABLE_STATIC_TASK_CLEAN_UP)
+static const int skip_index = 0; /*PTHREAD_TLS_INDEX*/
+#else
+static const int skip_index = -1;
+#endif
+
+
 static void del_cb(int index, void *ptr)
 {
     *((uint32_t *)ptr) = (TLSP_DEL_BASE << index);   //Indicate deletion by setting task storage element to a unique value
@@ -225,11 +235,19 @@ static void task_cb(void *arg)
 {
     int core = xPortGetCoreID();
     for(int i = 0; i < NO_OF_TLSP; i++){
+        if (i == skip_index) {
+            continue;
+        }
+
         task_storage[core][i] = (TLSP_SET_BASE << i);   //Give each element of task_storage a unique number
         vTaskSetThreadLocalStoragePointerAndDelCallback(NULL, i, (void *)&task_storage[core][i], del_cb);   //Set each TLSP to point to a task storage element
     }
 
     for(int i = 0; i < NO_OF_TLSP; i++){
+        if (i == skip_index) {
+            continue;
+        }
+
         uint32_t * tlsp = (uint32_t *)pvTaskGetThreadLocalStoragePointer(NULL, i);
         TEST_ASSERT_EQUAL(*tlsp, (TLSP_SET_BASE << i)); //Check if TLSP points to the correct task storage element by checking unique value
     }
@@ -247,6 +265,9 @@ TEST_CASE("Test FreeRTOS thread local storage pointers and del cb", "[freertos]"
 
     for(int core = 0; core < portNUM_PROCESSORS; core++){
         for(int i = 0; i < NO_OF_TLSP; i++){
+            if (i == skip_index) {
+                continue;
+            }
             TEST_ASSERT_EQUAL((TLSP_DEL_BASE << i), task_storage[core][i]);     //Check del_cb ran by checking task storage for unique value
         }
     }

components/freertos/test/test_freertos_isinisrcontext.c

@@ -15,6 +15,8 @@
 #include "xtensa/hal.h"
 #include "esp_rom_sys.h"
 
+#if CONFIG_FREERTOS_CORETIMER_0
+
 static volatile int in_int_context, int_handled;
 
 
@@ -52,3 +54,5 @@ TEST_CASE("xPortInIsrContext test", "[freertos]")
 #endif
 }
 
+
+#endif

components/freertos/test/test_freertos_mutex.c

@@ -4,6 +4,9 @@
 #include "unity.h"
 #include "test_utils.h"
 
+/* If assertions aren't set to fail this code still crashes, but not with an abort... */
+#if CONFIG_FREERTOS_CHECK_MUTEX_GIVEN_BY_OWNER && CONFIG_FREERTOS_ASSERT_FAIL_ABORT
+
 static void mutex_release_task(void* arg)
 {
     SemaphoreHandle_t mutex = (SemaphoreHandle_t) arg;
@@ -18,3 +21,5 @@ TEST_CASE("mutex released not by owner causes an assert", "[freertos][reset=abor
     xTaskCreate(&mutex_release_task, "mutex_release", 2048, mutex, UNITY_FREERTOS_PRIORITY + 1, NULL);
     vTaskDelay(1);
 }
+
+#endif

components/freertos/test/test_legacy_hooks.c

@@ -0,0 +1,78 @@
+/*
+ Test of FreeRTOS "legacy" hook functions, and delete hook
+
+ Only compiled in if the relevant options are enabled.
+*/
+#include <stdio.h>
+#include <stdlib.h>
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
+#include <freertos/semphr.h>
+#include "driver/timer.h"
+#include "unity.h"
+#include "test_utils.h"
+
+#if CONFIG_FREERTOS_LEGACY_HOOKS
+
+static volatile unsigned idle_count;
+
+void vApplicationIdleHook(void)
+{
+    idle_count++;
+}
+
+TEST_CASE("legacy idle hook is correctly called based on config", "[freertos]")
+{
+    idle_count = 0;
+    vTaskDelay(10);
+    TEST_ASSERT_NOT_EQUAL(0, idle_count); // The legacy idle hook should be called at least once
+}
+
+static volatile unsigned tick_count;
+
+void IRAM_ATTR vApplicationTickHook(void)
+{
+    tick_count++;
+}
+
+TEST_CASE("legacy tick hook is called based on config", "[freertos]")
+{
+    unsigned before = xTaskGetTickCount();
+    const unsigned SLEEP_FOR = 20;
+    tick_count = before;
+    vTaskDelay(SLEEP_FOR);
+    TEST_ASSERT_UINT32_WITHIN_MESSAGE(3 * portNUM_PROCESSORS, before + SLEEP_FOR * portNUM_PROCESSORS, tick_count,
+                                      "The legacy tick hook should have been called approx 1 time per tick per CPU");
+}
+
+#endif // CONFIG_FREERTOS_LEGACY_HOOKS
+
+#if CONFIG_FREERTOS_ENABLE_STATIC_TASK_CLEAN_UP
+
+static volatile void *deleted_tcb;
+
+static void taskDeletesItself(void *ignored)
+{
+    vTaskDelete(NULL);
+}
+
+void vPortCleanUpTCB(void *pxTCB)
+{
+    deleted_tcb = pxTCB;
+}
+
+TEST_CASE("static task cleanup hook is called based on config", "[freertos]")
+{
+    for(int i = 0; i < portNUM_PROCESSORS; i++) {
+        printf("Creating task CPU %d\n", i);
+        TaskHandle_t new_task = NULL;
+        deleted_tcb = NULL;
+        xTaskCreatePinnedToCore(taskDeletesItself, "delete", 2048, NULL, UNITY_FREERTOS_PRIORITY, &new_task, i);
+        vTaskDelay(5);
+        TEST_ASSERT_EQUAL_PTR(deleted_tcb, new_task); // TCB & TaskHandle are the same in FreeRTOS
+    }
+}
+
+#endif // CONFIG_FREERTOS_ENABLE_STATIC_TASK_CLEAN_UP
+
+

components/freertos/test/test_task_suspend_resume.c

@@ -129,6 +129,7 @@ void IRAM_ATTR timer_group0_isr(void *vp_arg)
     timer_group_clr_intr_status_in_isr(TIMER_GROUP_0, TIMER_0);
 
     timer_isr_fired = true;
+
     TaskHandle_t handle = vp_arg;
     BaseType_t higherPriorityTaskWoken = pdFALSE;
     higherPriorityTaskWoken = xTaskResumeFromISR(handle);
@@ -137,44 +138,70 @@ void IRAM_ATTR timer_group0_isr(void *vp_arg)
     }
 }
 
+/* Task suspends itself, then sets parameter value to the current timer group counter and deletes itself */
+static void task_suspend_self_with_timer(void *vp_resumed)
+{
+    volatile uint64_t *resumed_counter = vp_resumed;
+    *resumed_counter = 0;
+    vTaskSuspend(NULL);
+    timer_get_counter_value(TIMER_GROUP_0, TIMER_0, vp_resumed);
+    vTaskDelete(NULL);
+}
+
+
 /* Create a task which suspends itself, then resume it from a timer
  * interrupt. */
 static void test_resume_task_from_isr(int target_core)
 {
-    volatile bool resumed = false;
+    volatile uint64_t resumed_counter = 99;
     TaskHandle_t suspend_task;
 
-    xTaskCreatePinnedToCore(task_suspend_self, "suspend_self", 2048,
-                            (void *)&resumed, UNITY_FREERTOS_PRIORITY + 1,
+    xTaskCreatePinnedToCore(task_suspend_self_with_timer, "suspend_self", 2048,
+                            (void *)&resumed_counter, UNITY_FREERTOS_PRIORITY + 1,
                             &suspend_task, target_core);
 
     vTaskDelay(1);
-    TEST_ASSERT_FALSE(resumed);
+    TEST_ASSERT_EQUAL(0, resumed_counter);
+
+    const unsigned APB_CYCLES_PER_TICK = TIMER_BASE_CLK / configTICK_RATE_HZ;
+    const unsigned TEST_TIMER_DIV = 2;
+    const unsigned TEST_TIMER_CYCLES_PER_TICK = APB_CYCLES_PER_TICK / TEST_TIMER_DIV;
+    const unsigned TEST_TIMER_CYCLES_PER_MS = TIMER_BASE_CLK / 1000 / TEST_TIMER_DIV;
+    const unsigned TEST_TIMER_ALARM = TEST_TIMER_CYCLES_PER_TICK / 2; // half an RTOS tick
 
     /* Configure timer ISR */
+    timer_isr_handle_t isr_handle;
     const timer_config_t config = {
         .alarm_en = 1,
         .auto_reload = 0,
         .counter_dir = TIMER_COUNT_UP,
-        .divider = 2,       //Range is 2 to 65536
+        .divider = TEST_TIMER_DIV,
         .intr_type = TIMER_INTR_LEVEL,
         .counter_en = TIMER_PAUSE,
     };
     /*Configure timer*/
-    timer_init(TIMER_GROUP_0, TIMER_0, &config);
-    timer_pause(TIMER_GROUP_0, TIMER_0);
-    timer_set_counter_value(TIMER_GROUP_0, TIMER_0, 0);
-    timer_set_alarm_value(TIMER_GROUP_0, TIMER_0, 1000);
-    timer_enable_intr(TIMER_GROUP_0, TIMER_0);
-    timer_isr_register(TIMER_GROUP_0, TIMER_0, timer_group0_isr, (void*)suspend_task, ESP_INTR_FLAG_IRAM, NULL);
+    ESP_ERROR_CHECK( timer_init(TIMER_GROUP_0, TIMER_0, &config) );
+    ESP_ERROR_CHECK( timer_pause(TIMER_GROUP_0, TIMER_0) );
+    ESP_ERROR_CHECK( timer_set_counter_value(TIMER_GROUP_0, TIMER_0, 0) );
+    ESP_ERROR_CHECK( timer_set_alarm_value(TIMER_GROUP_0, TIMER_0, TEST_TIMER_ALARM) );
+    ESP_ERROR_CHECK( timer_enable_intr(TIMER_GROUP_0, TIMER_0) );
+    ESP_ERROR_CHECK( timer_isr_register(TIMER_GROUP_0, TIMER_0, timer_group0_isr, (void*)suspend_task, ESP_INTR_FLAG_IRAM, &isr_handle) );
     timer_isr_fired = false;
-    timer_start(TIMER_GROUP_0, TIMER_0);
+    vTaskDelay(1); // Make sure we're at the start of a new tick
 
-    vTaskDelay(1);
+    ESP_ERROR_CHECK( timer_start(TIMER_GROUP_0, TIMER_0) );
+
+    vTaskDelay(1); // We expect timer group will fire half-way through this delay
 
-    timer_deinit(TIMER_GROUP_0, TIMER_0);
     TEST_ASSERT_TRUE(timer_isr_fired);
-    TEST_ASSERT_TRUE(resumed);
+    TEST_ASSERT_NOT_EQUAL(0, resumed_counter);
+    // The task should have woken within 500us of the timer interrupt event (note: task may be a flash cache miss)
+    printf("alarm value %u task resumed at %u\n", TEST_TIMER_ALARM, (unsigned)resumed_counter);
+    TEST_ASSERT_UINT32_WITHIN(TEST_TIMER_CYCLES_PER_MS/2, TEST_TIMER_ALARM, (unsigned)resumed_counter);
+
+    // clean up
+    timer_deinit(TIMER_GROUP_0, TIMER_0);
+    ESP_ERROR_CHECK( esp_intr_free(isr_handle) );
 }
 
 TEST_CASE("Resume task from ISR (same core)", "[freertos]")
@@ -293,11 +320,11 @@ TEST_CASE("Test the waiting task not missed due to scheduler suspension on one C
     test_scheduler_suspend1(1);
 }
 
-static uint32_t count_tick[2];
+static uint32_t tick_hook_ms[2];
 
 static void IRAM_ATTR tick_hook(void)
 {
-    ++count_tick[xPortGetCoreID()];
+    tick_hook_ms[xPortGetCoreID()] += portTICK_PERIOD_MS;
 }
 
 static void test_scheduler_suspend2(int cpu)
@@ -305,7 +332,7 @@ static void test_scheduler_suspend2(int cpu)
     esp_register_freertos_tick_hook_for_cpu(tick_hook, 0);
     esp_register_freertos_tick_hook_for_cpu(tick_hook, 1);
 
-    memset(count_tick, 0, sizeof(count_tick));
+    memset(tick_hook_ms, 0, sizeof(tick_hook_ms));
 
     printf("Test for CPU%d\n", cpu);
     xTaskCreatePinnedToCore(&control_task, "control_task", 8192, NULL, 5, NULL, cpu);
@@ -313,10 +340,10 @@ static void test_scheduler_suspend2(int cpu)
     vTaskDelay(waiting_ms * 2 / portTICK_PERIOD_MS);
     esp_deregister_freertos_tick_hook(tick_hook);
 
-    printf("count_tick[cpu0] = %d, count_tick[cpu1] = %d\n", count_tick[0], count_tick[1]);
+    printf("tick_hook_ms[cpu0] = %d, tick_hook_ms[cpu1] = %d\n", tick_hook_ms[0], tick_hook_ms[1]);
 
-    TEST_ASSERT_INT_WITHIN(1, waiting_ms * 2, count_tick[0]);
-    TEST_ASSERT_INT_WITHIN(1, waiting_ms * 2, count_tick[1]);
+    TEST_ASSERT_INT_WITHIN(portTICK_PERIOD_MS, waiting_ms * 2, tick_hook_ms[0]);
+    TEST_ASSERT_INT_WITHIN(portTICK_PERIOD_MS, waiting_ms * 2, tick_hook_ms[1]);
     printf("\n");
 }
 
@@ -337,7 +364,7 @@ static int duration_timer_ms;
 
 static void timer_callback(void *arg)
 {
-    ++duration_timer_ms;
+    duration_timer_ms += portTICK_PERIOD_MS;
 }
 
 static void test_scheduler_suspend3(int cpu)

docs/en/api-reference/system/freertos_additions.rst

@@ -506,6 +506,9 @@ and ``vApplicationTickHook()`` can only be defined once. However, the ESP32 is d
 in nature, therefore same Idle Hook and Tick Hook are used for both cores (in other words,
 the hooks are symmetrical for both cores).
 
+In a dual core system, ``vApplicationTickHook()`` must be located in IRAM (for example
+by adding the IRAM_ATTR attribute).
+
 ESP-IDF Idle and Tick Hooks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 Due to the the dual core nature of the ESP32, it may be necessary for some

tools/unit-test-app/configs/freertos_options

@@ -0,0 +1,24 @@
+# This is a small set of tests where we enable as many as possible of the optional features
+# in FreeRTOS that are gated behind config
+
+CONFIG_IDF_TARGET="esp32"
+TEST_COMPONENTS=freertos
+
+CONFIG_FREERTOS_CORETIMER_1=y
+CONFIG_FREERTOS_OPTIMIZED_SCHEDULER=n
+CONFIG_FREERTOS_HZ=500
+CONFIG_FREERTOS_CHECK_STACKOVERFLOW_PTRVAL=y
+CONFIG_FREERTOS_WATCHPOINT_END_OF_STACK=y
+CONFIG_FREERTOS_INTERRUPT_BACKTRACE=n
+CONFIG_FREERTOS_ASSERT_FAIL_PRINT_CONTINUE=y
+CONFIG_FREERTOS_LEGACY_HOOKS=y
+CONFIG_FREERTOS_SUPPORT_STATIC_ALLOCATION=y
+CONFIG_FREERTOS_ENABLE_STATIC_TASK_CLEAN_UP=y
+CONFIG_FREERTOS_QUEUE_REGISTRY_SIZE=10
+CONFIG_FREERTOS_USE_TRACE_FACILITY=y
+CONFIG_FREERTOS_USE_STATS_FORMATTING_FUNCTIONS=y
+CONFIG_FREERTOS_VTASKLIST_INCLUDE_COREID=y
+CONFIG_FREERTOS_GENERATE_RUN_TIME_STATS=y
+CONFIG_FREERTOS_PLACE_FUNCTIONS_INTO_FLASH=y
+CONFIG_FREERTOS_FPU_IN_ISR=y
+CONFIG_FREERTOS_THREAD_LOCAL_STORAGE_POINTERS=16

tools/unit-test-app/configs/freertos_options_s2

@@ -0,0 +1,23 @@
+# This is a small set of tests where we enable as many as possible of the optional features
+# in FreeRTOS that are gated behind config
+
+CONFIG_IDF_TARGET="esp32s2"
+TEST_COMPONENTS=freertos
+
+CONFIG_FREERTOS_CORETIMER_1=y
+CONFIG_FREERTOS_OPTIMIZED_SCHEDULER=n
+CONFIG_FREERTOS_HZ=500
+CONFIG_FREERTOS_CHECK_STACKOVERFLOW_PTRVAL=y
+CONFIG_FREERTOS_WATCHPOINT_END_OF_STACK=y
+CONFIG_FREERTOS_INTERRUPT_BACKTRACE=n
+CONFIG_FREERTOS_ASSERT_FAIL_PRINT_CONTINUE=y
+CONFIG_FREERTOS_LEGACY_HOOKS=y
+CONFIG_FREERTOS_SUPPORT_STATIC_ALLOCATION=y
+CONFIG_FREERTOS_ENABLE_STATIC_TASK_CLEAN_UP=y
+CONFIG_FREERTOS_QUEUE_REGISTRY_SIZE=10
+CONFIG_FREERTOS_USE_TRACE_FACILITY=y
+CONFIG_FREERTOS_USE_STATS_FORMATTING_FUNCTIONS=y
+CONFIG_FREERTOS_VTASKLIST_INCLUDE_COREID=y
+CONFIG_FREERTOS_GENERATE_RUN_TIME_STATS=y
+CONFIG_FREERTOS_PLACE_FUNCTIONS_INTO_FLASH=y
+CONFIG_FREERTOS_FPU_IN_ISR=y