mirror of
https://github.com/espressif/esp-idf.git
synced 2026-04-27 19:13:21 +00:00
Merge branch 'fix/freertos_mutex_not_given_by_owner' into 'master'
fix(freertos): hide obsolete mutex-owner config and harden assert test Closes IDFCI-4743, IDFCI-4271, IDFCI-3883, and IDFCI-3594 See merge request espressif/esp-idf!47210
This commit is contained in:
@@ -417,14 +417,11 @@ menu "FreeRTOS"
|
||||
hook function in their application.
|
||||
|
||||
config FREERTOS_CHECK_MUTEX_GIVEN_BY_OWNER
|
||||
# This feature is innately supported in FreeRTOS SMP, and hence not available as a config option when
|
||||
# FreeRTOS SMP is enabled.
|
||||
# TODO(IDF-15523): Remove this hidden compatibility option entirely.
|
||||
# This feature is innately supported in FreeRTOS SMP, and IDF FreeRTOS always enables it.
|
||||
depends on !FREERTOS_SMP
|
||||
bool "Check that mutex semaphore is given by owner task"
|
||||
bool
|
||||
default y
|
||||
help
|
||||
If enabled, assert that when a mutex semaphore is given, the task giving the semaphore is the task
|
||||
which is currently holding the mutex.
|
||||
|
||||
config FREERTOS_ISR_STACKSIZE
|
||||
int "ISR stack size"
|
||||
|
||||
@@ -289,9 +289,6 @@
|
||||
#ifdef CONFIG_FREERTOS_TLSP_DELETION_CALLBACKS
|
||||
#define configTHREAD_LOCAL_STORAGE_DELETE_CALLBACKS 1
|
||||
#endif /* CONFIG_FREERTOS_TLSP_DELETION_CALLBACKS */
|
||||
#if CONFIG_FREERTOS_CHECK_MUTEX_GIVEN_BY_OWNER
|
||||
#define configCHECK_MUTEX_GIVEN_BY_OWNER 1
|
||||
#endif /* CONFIG_FREERTOS_CHECK_MUTEX_GIVEN_BY_OWNER */
|
||||
#endif /* !CONFIG_FREERTOS_SMP */
|
||||
|
||||
/* ------------------------------------------------ ESP-IDF Additions --------------------------------------------------
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
|
||||
* SPDX-FileCopyrightText: 2022-2026 Espressif Systems (Shanghai) CO LTD
|
||||
*
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
@@ -7,35 +7,73 @@
|
||||
#include "freertos/FreeRTOS.h"
|
||||
#include "freertos/task.h"
|
||||
#include "freertos/semphr.h"
|
||||
#include <stdbool.h>
|
||||
#include "esp_attr.h"
|
||||
#include "soc/soc_caps.h"
|
||||
#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
|
||||
#include "hal/cache_ll.h"
|
||||
#endif
|
||||
#include "unity.h"
|
||||
#include "test_utils.h"
|
||||
#include "esp_system.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_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE
|
||||
#if !CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE
|
||||
|
||||
/* Persists across reset so stage 2 can confirm stage 1 reached the failure path. */
|
||||
static __NOINIT_ATTR bool s_restart_marker;
|
||||
/* Reinitialized on reboot so stage 2 can distinguish a real restart from normal return. */
|
||||
static bool s_runtime_marker;
|
||||
|
||||
static void mutex_release_task(void* arg)
|
||||
{
|
||||
SemaphoreHandle_t mutex = (SemaphoreHandle_t) arg;
|
||||
xSemaphoreGive(mutex);
|
||||
/* The spawned task gives a mutex it does not own. Because the mutex is still held by the
|
||||
main task, the give flows through xSemaphoreGive(), xQueueGenericSend(),
|
||||
prvCopyDataToQueue(), and xTaskPriorityDisinherit(), where configASSERT(pxTCB ==
|
||||
pxCurrentTCB) fires and the panic resets the chip. */
|
||||
TEST_FAIL_MESSAGE("should not be reached");
|
||||
}
|
||||
|
||||
static void check_reset_reason_panic(void)
|
||||
static void setup_restart_markers(void)
|
||||
{
|
||||
TEST_ASSERT_EQUAL(ESP_RST_PANIC, esp_reset_reason());
|
||||
s_restart_marker = true;
|
||||
s_runtime_marker = true;
|
||||
|
||||
#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
|
||||
/* If internal data is behind a cache it might not be written to the physical memory when we crash
|
||||
force a full writeback here to ensure this */
|
||||
cache_ll_writeback_all(CACHE_LL_LEVEL_INT_MEM, CACHE_TYPE_DATA, CACHE_LL_ID_ALL);
|
||||
#endif
|
||||
}
|
||||
|
||||
static void check_system_restarted(void)
|
||||
{
|
||||
/* s_restart_marker is NOINIT so it still holds the true value set in stage 1 across the
|
||||
* software reset; s_runtime_marker is BSS so it is re-initialized to false on every boot. */
|
||||
TEST_ASSERT_TRUE(s_restart_marker);
|
||||
TEST_ASSERT_FALSE(s_runtime_marker);
|
||||
|
||||
s_restart_marker = false;
|
||||
}
|
||||
|
||||
static void do_mutex_release_assert(void)
|
||||
{
|
||||
setup_restart_markers();
|
||||
|
||||
SemaphoreHandle_t mutex = xSemaphoreCreateMutex();
|
||||
xSemaphoreTake(mutex, portMAX_DELAY);
|
||||
xTaskCreate(&mutex_release_task, "mutex_release", 2048, mutex, UNITY_FREERTOS_PRIORITY + 1, NULL);
|
||||
vTaskDelay(1);
|
||||
/* Stage 1 must not return to Unity before the releaser trips the panic. */
|
||||
vTaskSuspend(NULL);
|
||||
}
|
||||
|
||||
TEST_CASE_MULTIPLE_STAGES("mutex released not by owner causes an assert", "[freertos][reset=assert,SW_CPU_RESET]",
|
||||
TEST_CASE_MULTIPLE_STAGES("mutex released not by owner causes an assert", "[freertos]",
|
||||
/* On dual-core ESP32 the panic handler can deadlock on S32C1I if the
|
||||
* stalled core was mid-atomic, causing an RTC WDT reset (ESP_RST_WDT)
|
||||
* instead of ESP_RST_PANIC. Stage 2 therefore verifies a real reboot
|
||||
* via NOINIT and BSS state instead of checking the reset reason. */
|
||||
do_mutex_release_assert,
|
||||
check_reset_reason_panic)
|
||||
check_system_restarted)
|
||||
|
||||
#endif
|
||||
|
||||
@@ -96,8 +96,5 @@ CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER=y
|
||||
# Thread local storage deletion callbacks
|
||||
CONFIG_FREERTOS_TLSP_DELETION_CALLBACKS=y
|
||||
|
||||
# Mutex owner check
|
||||
CONFIG_FREERTOS_CHECK_MUTEX_GIVEN_BY_OWNER=y
|
||||
|
||||
# Port critical compliance check
|
||||
CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE=y
|
||||
|
||||
Reference in New Issue
Block a user