RIOT-OS/RIOT
RIOT-OS/RIOT
1
0
Fork 0
mirror of https://github.com/RIOT-OS/RIOT.git synced 2025-01-17 05:12:57 +01:00
Code Releases Activity

Merge pull request #18544 from gschorcht/cpu/esp/improve_thread_safety_of_malloc

Browse Source 
cpu/esp: improve thread safety in newlib locking functions
This commit is contained in:
Gunar Schorcht 2022-09-05 13:29:03 +02:00 committed by GitHub
commit a0a0b64f40
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 162 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
cpu/esp32/Kconfig.esp32
View File

@ -25,7 +25,6 @@ config CPU_FAM_ESP32
select PACKAGE_ESP32_SDK if TEST_KCONFIG
select MODULE_MALLOC_THREAD_SAFE if !MODULE_ESP_IDF_HEAP && TEST_KCONFIG
select MODULE_PERIPH_RTT if HAS_PERIPH_RTT && MODULE_PM_LAYERED
select MODULE_PS if MODULE_SHELL
select MODULE_PTHREAD if MODULE_CPP

1
cpu/esp32/Kconfig.esp32c3
View File

@ -26,7 +26,6 @@ config CPU_FAM_ESP32C3
select PACKAGE_ESP32_SDK if TEST_KCONFIG
select MODULE_MALLOC_THREAD_SAFE if !MODULE_ESP_IDF_HEAP && TEST_KCONFIG
select MODULE_PERIPH_RTT if HAS_PERIPH_RTT && MODULE_PM_LAYERED
select MODULE_PS if MODULE_SHELL
select MODULE_PTHREAD if MODULE_CPP

1
cpu/esp32/Kconfig.esp32s2
View File

@ -21,7 +21,6 @@ config CPU_FAM_ESP32S2
select PACKAGE_ESP32_SDK if TEST_KCONFIG
select MODULE_MALLOC_THREAD_SAFE if !MODULE_ESP_IDF_HEAP && TEST_KCONFIG
select MODULE_PERIPH_RTT if HAS_PERIPH_RTT && MODULE_PM_LAYERED
select MODULE_PS if MODULE_SHELL
select MODULE_PTHREAD if MODULE_CPP

1
cpu/esp32/Kconfig.esp32s3
View File

@ -27,7 +27,6 @@ config CPU_FAM_ESP32S3
select PACKAGE_ESP32_SDK if TEST_KCONFIG
select MODULE_MALLOC_THREAD_SAFE if !MODULE_ESP_IDF_HEAP && TEST_KCONFIG
select MODULE_PERIPH_RTT if HAS_PERIPH_RTT && MODULE_PM_LAYERED
select MODULE_PS if MODULE_SHELL
select MODULE_PTHREAD if MODULE_CPP

2
cpu/esp32/Makefile.dep
View File

@ -126,8 +126,6 @@ ifneq (,$(filter esp_idf_heap,$(USEMODULE)))
# the implementations of TLSF with the exception of heap poisoning, which
# is not configured.
USEPKG += tlsf
else
USEMODULE += malloc_thread_safe
endif
ifneq (,$(filter mtd,$(USEMODULE)))

47
cpu/esp32/syscalls.c
View File

@ -24,6 +24,8 @@
#include "div.h"
#include "esp/common_macros.h"
#include "irq_arch.h"
#include "mutex.h"
#include "rmutex.h"
#include "periph_cpu.h"
#include "periph/pm.h"
#include "syscalls.h"
@ -255,8 +257,53 @@ timer_hal_context_t sys_timer = {
.idx = TIMER_SYSTEM_INDEX,
};
#if defined(_RETARGETABLE_LOCKING)
/* all locking variables share the same mutex respectively the same rmutex */
static mutex_t s_shared_mutex = MUTEX_INIT;
static rmutex_t s_shared_rmutex = RMUTEX_INIT;
/* definition of locks required by the newlib if retargetable locking is used */
extern struct __lock __attribute__((alias("s_shared_rmutex"))) __lock___sinit_recursive_mutex;
extern struct __lock __attribute__((alias("s_shared_rmutex"))) __lock___sfp_recursive_mutex;
extern struct __lock __attribute__((alias("s_shared_rmutex"))) __lock___atexit_recursive_mutex;
extern struct __lock __attribute__((alias("s_shared_rmutex"))) __lock___malloc_recursive_mutex;
extern struct __lock __attribute__((alias("s_shared_rmutex"))) __lock___env_recursive_mutex;
extern struct __lock __attribute__((alias("s_shared_mutex"))) __lock___at_quick_exit_mutex;
extern struct __lock __attribute__((alias("s_shared_mutex"))) __lock___tz_mutex;
extern struct __lock __attribute__((alias("s_shared_mutex"))) __lock___dd_hash_mutex;
extern struct __lock __attribute__((alias("s_shared_mutex"))) __lock___arc4random_mutex;
#endif
void IRAM syscalls_init_arch(void)
{
#if defined(_RETARGETABLE_LOCKING)
/* initialization of static locking variables in ROM, different ROM
* versions of newlib use different locking variables */
#if defined(CPU_FAM_ESP32)
extern _lock_t __sfp_lock;
extern _lock_t __sinit_lock;
extern _lock_t __env_lock_object;
extern _lock_t __tz_lock_object;
__sfp_lock = (_lock_t)&s_shared_rmutex;
__sinit_lock = (_lock_t)&s_shared_rmutex;
__env_lock_object = (_lock_t)&s_shared_mutex;
__tz_lock_object = (_lock_t)&s_shared_rmutex;
#elif defined(CPU_FAM_ESP32S2)
extern _lock_t __sinit_recursive_mutex;
extern _lock_t __sfp_recursive_mutex;
__sinit_recursive_mutex = (_lock_t)&s_shared_rmutex;
__sfp_recursive_mutex = (_lock_t)&s_shared_rmutex;
#else
/* TODO: Other platforms don't provide access to these ROM variables.
* It could be necessary to call `esp_rom_newlib_init_common_mutexes`. For
* the moment it doesn't seems to be necessary to call this function. */
#endif
#endif /* _RETARGETABLE_LOCKING */
/* initialize and enable the system timer in us (TMG0 is enabled by default) */
timer_hal_init(&sys_timer, TIMER_SYSTEM_GROUP, TIMER_SYSTEM_INDEX);
timer_hal_set_divider(&sys_timer, rtc_clk_apb_freq_get() / MHZ);

11
cpu/esp8266/include/cpu_conf.h
View File

@ -65,16 +65,27 @@ extern "C" {
#ifndef THREAD_EXTRA_STACKSIZE_PRINTF
#define THREAD_EXTRA_STACKSIZE_PRINTF (0)
#endif
#ifndef THREAD_STACKSIZE_DEFAULT
#define THREAD_STACKSIZE_DEFAULT (1024)
#endif
#ifndef THREAD_STACKSIZE_IDLE
#define THREAD_STACKSIZE_IDLE (1024)
#endif
#ifndef THREAD_STACKSIZE_MAIN
#define THREAD_STACKSIZE_MAIN (3072)
#endif
#ifndef THREAD_STACKSIZE_SMALL
#define THREAD_STACKSIZE_SMALL (THREAD_STACKSIZE_MEDIUM * 3 / 2)
#endif
#ifndef THREAD_STACKSIZE_TINY
#define THREAD_STACKSIZE_TINY (THREAD_STACKSIZE_MEDIUM / 2)
#endif
#ifndef GNRC_IPV6_STACK_SIZE
#define GNRC_IPV6_STACK_SIZE (1536)
#endif

20
cpu/esp_common/freertos/semphr.c
View File

@ -66,6 +66,11 @@ BaseType_t xSemaphoreGive(SemaphoreHandle_t xSemaphore)
{
DEBUG("%s mutex=%p\n", __func__, xSemaphore);
/* if scheduler is not running, we must not lock the mutex */
if (thread_getpid() == KERNEL_PID_UNDEF) {
return pdPASS;
}
assert(xSemaphore != NULL);
_sem_t* sem = (_sem_t*)xSemaphore;
@ -91,6 +96,11 @@ BaseType_t xSemaphoreTake(SemaphoreHandle_t xSemaphore,
{
DEBUG("%s mutex=%p wait=%"PRIu32"\n", __func__, xSemaphore, xTicksToWait);
/* if scheduler is not running, we must not lock the mutex */
if (thread_getpid() == KERNEL_PID_UNDEF) {
return pdPASS;
}
assert(xSemaphore != NULL);
_sem_t* sem = (_sem_t*)xSemaphore;
@ -139,6 +149,11 @@ BaseType_t xSemaphoreGiveRecursive(SemaphoreHandle_t xSemaphore)
{
DEBUG("%s rmutex=%p\n", __func__, xSemaphore);
/* if scheduler is not running, we must not lock the mutex */
if (thread_getpid() == KERNEL_PID_UNDEF) {
return pdPASS;
}
_rsem_t* rsem = (_rsem_t*)xSemaphore;
assert(rsem != NULL);
@ -156,6 +171,11 @@ BaseType_t xSemaphoreTakeRecursive(SemaphoreHandle_t xSemaphore,
{
DEBUG("%s rmutex=%p wait=%"PRIu32"\n", __func__, xSemaphore, xTicksToWait);
/* if scheduler is not running, we must not lock the rmutex */
if (thread_getpid() == KERNEL_PID_UNDEF) {
return pdPASS;
}
_rsem_t* rsem = (_rsem_t*)xSemaphore;
assert(rsem != NULL);

124
cpu/esp_common/syscalls.c
View File

@ -59,7 +59,7 @@ int pthread_setcancelstate(int state, int *oldstate)
/**
* @name Locking functions
*
* Following functions implement the lock mechanism for newlib.
* Following functions implement the locking mechanism for newlib.
*/
#ifdef MCU_ESP8266
@ -76,24 +76,40 @@ static rmutex_t _malloc_rmtx = RMUTEX_INIT;
* the address of newlib's static variable __malloc_lock_object.
*/
static _lock_t *__malloc_static_object = NULL;
#define _lock_critical_enter()
#define _lock_critical_exit()
#else
/* Operations with recursive locking variable operations have to be guarded
* for the ESP32x SoCs by disabling interrupts to prevent unwanted context
* switches. */
#define _lock_critical_enter() uint32_t __lock_state = irq_disable();
#define _lock_critical_exit() irq_restore(__lock_state);
#endif
void IRAM_ATTR _lock_init(_lock_t *lock)
{
assert(lock != NULL);
assert(lock != NULL); /* lock must not be NULL */
/* prevent a context switch between the allocation and the initialization */
uint32_t state = irq_disable();
mutex_t* mtx = malloc(sizeof(mutex_t));
if (mtx) {
memset(mtx, 0, sizeof(mutex_t));
*lock = (_lock_t)mtx;
}
/* cppcheck-suppress memleak; mtx is stored in lock */
irq_restore(state);
}
void IRAM_ATTR _lock_init_recursive(_lock_t *lock)
{
assert(lock != NULL);
assert(lock != NULL); /* lock must not be NULL */
#ifdef MCU_ESP8266
/* _malloc_rmtx is static and has not to be allocated */
@ -102,57 +118,78 @@ void IRAM_ATTR _lock_init_recursive(_lock_t *lock)
}
#endif
rmutex_t* rmtx = malloc(sizeof(rmutex_t));
/* prevent a context switch between the allocation and the initialization */
uint32_t state = irq_disable();
rmutex_t* rmtx = malloc(sizeof(rmutex_t));
if (rmtx) {
memset(rmtx, 0, sizeof(rmutex_t));
*lock = (_lock_t)rmtx;
}
/* cppcheck-suppress memleak; rmtx is stored in lock */
irq_restore(state);
}
void IRAM_ATTR _lock_close(_lock_t *lock)
{
assert(lock != NULL);
/* locking variable has to be valid and initialized */
assert(lock != NULL && *lock != 0);
#ifdef MCU_ESP8266
assert(lock != __malloc_static_object);
#endif
/* prevent a context switch between freeing and resetting */
uint32_t state = irq_disable();
free((void*)*lock);
*lock = 0;
irq_restore(state);
}
void IRAM_ATTR _lock_close_recursive(_lock_t *lock)
{
assert(lock != NULL);
/* locking variable has to be valid and initialized */
assert(lock != NULL && *lock != 0);
#ifdef MCU_ESP8266
assert(lock != __malloc_static_object);
#endif
/* prevent a context switch between freeing and resetting */
uint32_t state = irq_disable();
free((void*)*lock);
*lock = 0;
irq_restore(state);
}
void IRAM_ATTR _lock_acquire(_lock_t *lock)
{
assert(lock != NULL); /* lock must not be NULL */
assert(!irq_is_in()); /* _lock_acquire must not be called in
interrupt context */
/* if scheduler is not running, we have not to lock the mutex */
if (thread_getpid() == KERNEL_PID_UNDEF) {
return;
}
assert(lock != NULL);
/* if the lock data structure is still not allocated, initialize it first */
/* if the locking variable is not initialized, initialize it implicitly */
if (*lock == 0) {
_lock_init(lock);
assert(*lock != 0);
}
assert(!irq_is_in());
mutex_lock((mutex_t*)*lock);
}
void IRAM_ATTR _lock_acquire_recursive(_lock_t *lock)
{
assert(lock != NULL); /* lock must not be NULL */
assert(!irq_is_in()); /* _lock_acquire must not be called in
interrupt context */
#ifdef MCU_ESP8266
/**
* Since we don't have direct access to newlib's static variable
@ -174,57 +211,67 @@ void IRAM_ATTR _lock_acquire_recursive(_lock_t *lock)
return;
}
assert(lock != NULL);
/* if the lock data structure is still not allocated, initialize it first */
/* if the locking variable is not initialized, initialize it implicitly */
if (*lock == 0) {
_lock_init_recursive(lock);
assert(*lock != 0);
}
assert(!irq_is_in());
_lock_critical_enter();
rmutex_lock((rmutex_t*)*lock);
_lock_critical_exit();
}
int IRAM_ATTR _lock_try_acquire(_lock_t *lock)
{
assert(lock != NULL); /* lock must not be NULL */
/* if scheduler is not running, we have not to lock the mutex */
if (thread_getpid() == KERNEL_PID_UNDEF) {
return 0;
}
assert(lock != NULL);
/* if the lock data structure is still not allocated, initialize it first */
if (*lock == 0) {
_lock_init(lock);
}
if (irq_is_in()) {
return 0;
}
/* if the locking variable is not initialized, initialize it implicitly */
if (*lock == 0) {
_lock_init(lock);
assert(*lock != 0);
}
return mutex_trylock((mutex_t*)*lock);
}
int IRAM_ATTR _lock_try_acquire_recursive(_lock_t *lock)
{
assert(lock != NULL); /* lock must not be NULL */
/* if scheduler is not running, we have not to lock the mutex */
if (thread_getpid() == KERNEL_PID_UNDEF) {
return 0;
}
assert(lock != NULL);
/* if the lock data structure is still not allocated, initialize it first */
if (*lock == 0) {
_lock_init_recursive(lock);
}
if (irq_is_in()) {
return 0;
}
return rmutex_trylock((rmutex_t*)*lock);
/* if the locking variable is not initialized, initialize it implicitly */
if (*lock == 0) {
_lock_init_recursive(lock);
assert(*lock != 0);
}
_lock_critical_enter();
int res = rmutex_trylock((rmutex_t*)*lock);
_lock_critical_exit();
return res;
}
void IRAM_ATTR _lock_release(_lock_t *lock)
@ -234,6 +281,7 @@ void IRAM_ATTR _lock_release(_lock_t *lock)
return;
}
/* the locking variable has to be valid and initialized */
assert(lock != NULL && *lock != 0);
mutex_unlock((mutex_t*)*lock);
@ -246,9 +294,14 @@ void IRAM_ATTR _lock_release_recursive(_lock_t *lock)
return;
}
/* the locking variable has to be valid and initialized */
assert(lock != NULL && *lock != 0);
_lock_critical_enter();
rmutex_unlock((rmutex_t*)*lock);
_lock_critical_exit();
}
#if defined(_RETARGETABLE_LOCKING)
@ -257,17 +310,6 @@ void IRAM_ATTR _lock_release_recursive(_lock_t *lock)
static_assert(sizeof(struct __lock) >= sizeof(rmutex_t),
"struct __lock is too small to hold a recursive mutex of type rmutex_t");
/* definition of locks required by the newlib if retargetable locking is used */
struct __lock __lock___sinit_recursive_mutex;
struct __lock __lock___sfp_recursive_mutex;
struct __lock __lock___atexit_recursive_mutex;
struct __lock __lock___at_quick_exit_mutex;
struct __lock __lock___malloc_recursive_mutex;
struct __lock __lock___env_recursive_mutex;
struct __lock __lock___tz_mutex;
struct __lock __lock___dd_hash_mutex;
struct __lock __lock___arc4random_mutex;
/* map newlib's `__retarget_*` functions to the existing `_lock_*` functions */
void __retarget_lock_init(_LOCK_T *lock)

4
dist/tools/doccheck/exclude_patterns vendored
View File

@ -5955,9 +5955,7 @@ cpu/esp8266/include/cpu_conf\.h:[0-9]+: warning: Member GNRC_IPV6_STACK_SIZE \(m
cpu/esp8266/include/cpu_conf\.h:[0-9]+: warning: Member GNRC_PKTDUMP_STACKSIZE \(macro definition\) of group cpu_esp8266_conf is not documented\.
cpu/esp8266/include/cpu_conf\.h:[0-9]+: warning: Member TCPIP_THREAD_STACKSIZE \(macro definition\) of group cpu_esp8266_conf is not documented\.
cpu/esp8266/include/cpu_conf\.h:[0-9]+: warning: Member THREAD_EXTRA_STACKSIZE_PRINTF \(macro definition\) of group cpu_esp8266_conf is not documented\.
cpu/esp8266/include/cpu_conf\.h:[0-9]+: warning: Member THREAD_STACKSIZE_DEFAULT \(macro definition\) of group cpu_esp8266_conf is not documented\.
cpu/esp8266/include/cpu_conf\.h:[0-9]+: warning: Member THREAD_STACKSIZE_IDLE \(macro definition\) of group cpu_esp8266_conf is not documented\.
cpu/esp8266/include/cpu_conf\.h:[0-9]+: warning: Member THREAD_STACKSIZE_MAIN \(macro definition\) of group cpu_esp8266_conf is not documented\.
cpu/esp8266/include/cpu_conf\.h:[0-9]+: warning: Member THREAD_STACKSIZE_[A-Z]+ \(macro definition\) of group cpu_esp8266_conf is not documented\.
cpu/esp8266/include/irq_arch\.h:[0-9]+: warning: Member ETS_SOFT_INT_HDL_MAC \(macro definition\) of file irq_arch\.h is not documented\.
cpu/esp8266/include/irq_arch\.h:[0-9]+: warning: Member ETS_SOFT_INT_NONE \(macro definition\) of file irq_arch\.h is not documented\.
cpu/esp8266/include/irq_arch\.h:[0-9]+: warning: Member ETS_SOFT_INT_YIELD \(macro definition\) of file irq_arch\.h is not documented\.

5
tests/thread_priority_inversion/Makefile
View File

@ -11,11 +11,6 @@ else
FANCY ?= 0
endif
# KNOWN ISSUE #18534
# Currently this is failing, causing unrelated errors to block other PRs.
# This issue will be looked into but for now we need to just close our eyes.
TEST_ON_CI_BLACKLIST += esp32-wroom-32
include $(RIOTBASE)/Makefile.include
CFLAGS += -DFANCY=$(FANCY)