From 78c4080faf8ea49a7ce80d3afe048acd288fea09 Mon Sep 17 00:00:00 2001 From: Vincent Dupont Date: Thu, 22 Feb 2018 18:27:45 +0100 Subject: [PATCH 1/5] newlib: add thread safe syscalls --- core/include/thread.h | 7 +++ core/sched.c | 4 ++ core/thread.c | 5 ++ cpu/cortexm_common/include/cpu_conf_common.h | 8 +++ makefiles/arch/cortexm.inc.mk | 1 + sys/Makefile.include | 4 ++ sys/newlib_thread_safe/Makefile | 1 + sys/newlib_thread_safe/Makefile.include | 1 + sys/newlib_thread_safe/syscalls.c | 54 ++++++++++++++++++++ 9 files changed, 85 insertions(+) create mode 100644 sys/newlib_thread_safe/Makefile create mode 100644 sys/newlib_thread_safe/Makefile.include create mode 100644 sys/newlib_thread_safe/syscalls.c diff --git a/core/include/thread.h b/core/include/thread.h index d59cabb2bff4..80f918d23d56 100644 --- a/core/include/thread.h +++ b/core/include/thread.h @@ -119,6 +119,10 @@ #ifndef THREAD_H #define THREAD_H +#ifdef MODULE_NEWLIB_THREAD_SAFE +#include +#endif + #include "clist.h" #include "cib.h" #include "msg.h" @@ -201,6 +205,9 @@ struct _thread { msg_t *msg_array; /**< memory holding messages sent to this thread's message queue */ #endif +#if defined(MODULE_NEWLIB_THREAD_SAFE) || defined(DOXYGEN) + struct _reent newlib_reent; /**< thread's re-entrent object */ +#endif #if defined(DEVELHELP) || defined(SCHED_TEST_STACK) \ || defined(MODULE_MPU_STACK_GUARD) || defined(DOXYGEN) char *stack_start; /**< thread's stack start address */ diff --git a/core/sched.c b/core/sched.c index 84151433d041..15f5e074d7f9 100644 --- a/core/sched.c +++ b/core/sched.c @@ -146,6 +146,10 @@ int __attribute__((used)) sched_run(void) mpu_enable(); #endif +#ifdef MODULE_NEWLIB_THREAD_SAFE + _impure_ptr = &(next_thread->newlib_reent); +#endif + DEBUG("sched_run: done, changed sched_active_thread.\n"); return 1; diff --git a/core/thread.c b/core/thread.c index 6f7a71362e90..ba828b73f49c 100644 --- a/core/thread.c +++ b/core/thread.c @@ -235,6 +235,11 @@ kernel_pid_t thread_create(char *stack, int stacksize, char priority, int flags, cb->msg_array = NULL; #endif +#ifdef MODULE_NEWLIB_THREAD_SAFE + /* initialize the reent context */ + _REENT_INIT_PTR(&(cb->newlib_reent)); +#endif + sched_num_threads++; DEBUG("Created thread %s. PID: %" PRIkernel_pid ". Priority: %u.\n", name, cb->pid, priority); diff --git a/cpu/cortexm_common/include/cpu_conf_common.h b/cpu/cortexm_common/include/cpu_conf_common.h index 513d53da0955..b2c0406edd13 100644 --- a/cpu/cortexm_common/include/cpu_conf_common.h +++ b/cpu/cortexm_common/include/cpu_conf_common.h @@ -41,11 +41,19 @@ extern "C" { #define THREAD_EXTRA_STACKSIZE_PRINTF (512) #endif #ifndef THREAD_STACKSIZE_DEFAULT +#ifdef MODULE_NEWLIB_THREAD_SAFE +#define THREAD_STACKSIZE_DEFAULT (1152) +#else #define THREAD_STACKSIZE_DEFAULT (1024) #endif +#endif #ifndef THREAD_STACKSIZE_IDLE +#ifdef MODULE_NEWLIB_THREAD_SAFE +#define THREAD_STACKSIZE_IDLE (384) +#else #define THREAD_STACKSIZE_IDLE (256) #endif +#endif /** @} */ /** diff --git a/makefiles/arch/cortexm.inc.mk b/makefiles/arch/cortexm.inc.mk index 8c7377c738eb..a7ad89bc910b 100644 --- a/makefiles/arch/cortexm.inc.mk +++ b/makefiles/arch/cortexm.inc.mk @@ -96,6 +96,7 @@ include $(RIOTCPU)/cortexm_common/Makefile.include # use the nano-specs of Newlib when available USEMODULE += newlib_nano export USE_NANO_SPECS = 1 +USEMODULE += newlib_thread_safe # Avoid overriding the default rule: all: diff --git a/sys/Makefile.include b/sys/Makefile.include index 24b8c7259b11..3cee1cc3622f 100644 --- a/sys/Makefile.include +++ b/sys/Makefile.include @@ -69,6 +69,10 @@ ifneq (,$(filter newlib_syscalls_default,$(USEMODULE))) include $(RIOTBASE)/sys/newlib_syscalls_default/Makefile.include endif +ifneq (,$(filter newlib_thread_safe,$(USEMODULE))) + include $(RIOTBASE)/sys/newlib_thread_safe/Makefile.include +endif + ifneq (,$(filter arduino,$(USEMODULE))) include $(RIOTBASE)/sys/arduino/Makefile.include endif diff --git a/sys/newlib_thread_safe/Makefile b/sys/newlib_thread_safe/Makefile new file mode 100644 index 000000000000..48422e909a47 --- /dev/null +++ b/sys/newlib_thread_safe/Makefile @@ -0,0 +1 @@ +include $(RIOTBASE)/Makefile.base diff --git a/sys/newlib_thread_safe/Makefile.include b/sys/newlib_thread_safe/Makefile.include new file mode 100644 index 000000000000..32053238623a --- /dev/null +++ b/sys/newlib_thread_safe/Makefile.include @@ -0,0 +1 @@ +UNDEF += $(BINDIR)/newlib_thread_safe/syscalls.o diff --git a/sys/newlib_thread_safe/syscalls.c b/sys/newlib_thread_safe/syscalls.c new file mode 100644 index 000000000000..84a1cdb8de20 --- /dev/null +++ b/sys/newlib_thread_safe/syscalls.c @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2018 OTA keys S.A. + * + * This file is subject to the terms and conditions of the GNU Lesser + * General Public License v2.1. See the file LICENSE in the top level + * directory for more details. + */ + +/** + * @ingroup sys_newlib + * @{ + * + * @file + * @brief Newlib system calls for thread safety + * + * @author Vincent Dupont + * + * @} + */ + +#include + +#include "rmutex.h" + +static rmutex_t _env = RMUTEX_INIT; +static rmutex_t _malloc = RMUTEX_INIT; + +void __env_lock(struct _reent *_r) +{ + (void)_r; + + rmutex_lock(&_env); +} + +void __env_unlock(struct _reent *_r) +{ + (void)_r; + + rmutex_unlock(&_env); +} + +void __malloc_lock(struct _reent *_r) +{ + (void)_r; + + rmutex_lock(&_malloc); +} + +void __malloc_unlock(struct _reent *_r) +{ + (void)_r; + + rmutex_unlock(&_malloc); +} From 33860efeb5f2d26e984418c97804bae81c75684d Mon Sep 17 00:00:00 2001 From: Vincent Dupont Date: Tue, 27 Feb 2018 10:18:36 +0100 Subject: [PATCH 2/5] fixup! newlib: add thread safe syscalls --- core/include/thread.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/include/thread.h b/core/include/thread.h index 80f918d23d56..51e946c95400 100644 --- a/core/include/thread.h +++ b/core/include/thread.h @@ -206,7 +206,7 @@ struct _thread { to this thread's message queue */ #endif #if defined(MODULE_NEWLIB_THREAD_SAFE) || defined(DOXYGEN) - struct _reent newlib_reent; /**< thread's re-entrent object */ + struct _reent newlib_reent; /**< thread's re-entrant object */ #endif #if defined(DEVELHELP) || defined(SCHED_TEST_STACK) \ || defined(MODULE_MPU_STACK_GUARD) || defined(DOXYGEN) From d05c147c8ab7f12fa0cabf43573948def099168d Mon Sep 17 00:00:00 2001 From: Vincent Dupont Date: Tue, 27 Feb 2018 11:20:01 +0100 Subject: [PATCH 3/5] fixup! fixup! newlib: add thread safe syscalls --- sys/newlib_thread_safe/Makefile.include | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/sys/newlib_thread_safe/Makefile.include b/sys/newlib_thread_safe/Makefile.include index 32053238623a..5a781f73c9fe 100644 --- a/sys/newlib_thread_safe/Makefile.include +++ b/sys/newlib_thread_safe/Makefile.include @@ -1 +1,26 @@ UNDEF += $(BINDIR)/newlib_thread_safe/syscalls.o + +# define the compile test files +define __has_reent_test +#include \n +endef + +define __has_reent_small_test +#include \n +#ifndef _REENT_SMALL\n +#error wasting 1KB\n +#endif\n +endef + +# compile the tests +__has_reent := $(shell printf "$(__has_reent_test)" | $(CC) -E $(CFLAGS) $(INCLUDES) - > /dev/null 2>&1 ; echo $$?) +__has_reent_small := $(shell printf "$(__has_reent_small_test)" | $(CC) -E $(CFLAGS) $(INCLUDES) - > /dev/null 2>&1 ; echo $$?) + +# check if we use REENT_SMALL +ifeq (0, $(__has_reent)) + ifneq (0, $(__has_reent_small)) + $(error "MODULE_NEWLIB_THREAD_SAFE defined but _REENT_SMALL not defined") + endif +else + $(error "MODULE_NEWLIB_THREAD_SAFE defined but not found in toolchain path") +endif From e03823fe6fe4096e63b7c87ba78ce0919850bcb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joakim=20Nohlg=C3=A5rd?= Date: Tue, 27 Feb 2018 15:26:01 +0100 Subject: [PATCH 4/5] WIP proof of concept THREAD_CREATE_REENT --- core/include/thread.h | 10 +++++++++- core/kernel_init.c | 2 +- core/sched.c | 2 +- core/thread.c | 22 +++++++++++++++++++++- 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/core/include/thread.h b/core/include/thread.h index 51e946c95400..5250c2e6c3db 100644 --- a/core/include/thread.h +++ b/core/include/thread.h @@ -206,7 +206,7 @@ struct _thread { to this thread's message queue */ #endif #if defined(MODULE_NEWLIB_THREAD_SAFE) || defined(DOXYGEN) - struct _reent newlib_reent; /**< thread's re-entrant object */ + struct _reent *newlib_reent; /**< thread's re-entrant object */ #endif #if defined(DEVELHELP) || defined(SCHED_TEST_STACK) \ || defined(MODULE_MPU_STACK_GUARD) || defined(DOXYGEN) @@ -321,6 +321,14 @@ struct _thread { * debugging and profiling purposes) */ #define THREAD_CREATE_STACKTEST (8) + /** + * @brief Allocate thread local reentrancy structure + * + * This will be allocated at the top of the stack, near the thread control block. + * A thread local reentrancy structure is required for thread safety in certain + * libc functions, e.g. stdio in newlib. + */ +#define THREAD_CREATE_REENT (0x10u) /** @} */ /** diff --git a/core/kernel_init.c b/core/kernel_init.c index 7d4f3ebc3534..3e9622e5ed90 100644 --- a/core/kernel_init.c +++ b/core/kernel_init.c @@ -88,7 +88,7 @@ void kernel_init(void) thread_create(main_stack, sizeof(main_stack), THREAD_PRIORITY_MAIN, - THREAD_CREATE_WOUT_YIELD | THREAD_CREATE_STACKTEST, + THREAD_CREATE_WOUT_YIELD | THREAD_CREATE_STACKTEST | THREAD_CREATE_REENT, main_trampoline, NULL, main_name); cpu_switch_context_exit(); diff --git a/core/sched.c b/core/sched.c index 15f5e074d7f9..e049896d5ecb 100644 --- a/core/sched.c +++ b/core/sched.c @@ -147,7 +147,7 @@ int __attribute__((used)) sched_run(void) #endif #ifdef MODULE_NEWLIB_THREAD_SAFE - _impure_ptr = &(next_thread->newlib_reent); + _impure_ptr = next_thread->newlib_reent; #endif DEBUG("sched_run: done, changed sched_active_thread.\n"); diff --git a/core/thread.c b/core/thread.c index ba828b73f49c..2aa2d06769cf 100644 --- a/core/thread.c +++ b/core/thread.c @@ -175,6 +175,16 @@ kernel_pid_t thread_create(char *stack, int stacksize, char priority, int flags, /* allocate our thread control block at the top of our stackspace */ thread_t *cb = (thread_t *) (stack + stacksize); +#ifdef MODULE_NEWLIB_THREAD_SAFE + if (flags & THREAD_CREATE_REENT) { + /* Reduce stack space to make room for newlib's struct _reent and align + * to nearest 64 bit boundary */ + stacksize -= sizeof(*cb->newlib_reent); + stacksize &= ~(8 - 1); + cb->newlib_reent = (void *)(stack + stacksize); + } +#endif + #if defined(DEVELHELP) || defined(SCHED_TEST_STACK) if (flags & THREAD_CREATE_STACKTEST) { /* assign each int of the stack the value of it's address */ @@ -237,7 +247,17 @@ kernel_pid_t thread_create(char *stack, int stacksize, char priority, int flags, #ifdef MODULE_NEWLIB_THREAD_SAFE /* initialize the reent context */ - _REENT_INIT_PTR(&(cb->newlib_reent)); + if (flags & THREAD_CREATE_REENT) { + /* Reduce stack space by sizeof(struct _reent) and align to nearest 64 + * bit boundary */ + stacksize &= ~(8 - 1); + stacksize -= (sizeof(*cb->newlib_reent) + (8 - 1)) & ~(8 - 1); + /* Initialize reent struct */ + _REENT_INIT_PTR(cb->newlib_reent); + } + else { + cb->newlib_reent = _GLOBAL_REENT; + } #endif sched_num_threads++; From 3f9b5064e88a04ac3534f449136ccd9601ac65eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joakim=20Nohlg=C3=A5rd?= Date: Tue, 27 Feb 2018 15:31:54 +0100 Subject: [PATCH 5/5] WIP newlib reent print example --- tests/thread_msg/Makefile | 2 ++ tests/thread_msg/main.c | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/thread_msg/Makefile b/tests/thread_msg/Makefile index c6a5a0e70815..6cbdb150d4f5 100644 --- a/tests/thread_msg/Makefile +++ b/tests/thread_msg/Makefile @@ -4,6 +4,8 @@ BOARD_INSUFFICIENT_MEMORY := nucleo32-f031 nucleo32-f042 DISABLE_MODULE += auto_init +USEMODULE += ps + include $(RIOTBASE)/Makefile.include test: diff --git a/tests/thread_msg/main.c b/tests/thread_msg/main.c index 353238746ee8..27a2218dabf5 100644 --- a/tests/thread_msg/main.c +++ b/tests/thread_msg/main.c @@ -20,9 +20,16 @@ #include #include +#ifdef MODULE_NEWLIB +#include +#else +#include +#define _REENT NULL +#endif #include "thread.h" #include "msg.h" +#include "ps.h" char t1_stack[THREAD_STACKSIZE_MAIN]; char t2_stack[THREAD_STACKSIZE_MAIN]; @@ -34,6 +41,7 @@ void *thread1(void *arg) { (void) arg; puts("THREAD 1 start\n"); + printf("PID %4" PRIkernel_pid " reent: %p\n", thread_getpid(), _REENT); for (int i = 0; i < 3; ++i) { msg_t msg, reply; @@ -56,6 +64,7 @@ void *thread2(void *arg) { (void) arg; puts("THREAD 2 start\n"); + printf("PID %4" PRIkernel_pid " reent: %p\n", thread_getpid(), _REENT); for (int i = 0;; ++i) { msg_t msg, reply; @@ -73,6 +82,7 @@ void *thread3(void *arg) { (void) arg; puts("THREAD 3 start\n"); + printf("PID %4" PRIkernel_pid " reent: %p\n", thread_getpid(), _REENT); for (int i = 0;; ++i) { msg_t msg; @@ -86,6 +96,8 @@ void *thread3(void *arg) int main(void) { p_main = sched_active_pid; + printf("PID %4" PRIkernel_pid " reent: %p\n", thread_getpid(), _REENT); + ps(); p1 = thread_create(t1_stack, sizeof(t1_stack), THREAD_PRIORITY_MAIN - 1, THREAD_CREATE_WOUT_YIELD | THREAD_CREATE_STACKTEST, thread1, NULL, "nr1"); @@ -93,9 +105,10 @@ int main(void) THREAD_CREATE_WOUT_YIELD | THREAD_CREATE_STACKTEST, thread2, NULL, "nr2"); p3 = thread_create(t3_stack, sizeof(t3_stack), THREAD_PRIORITY_MAIN - 1, - THREAD_CREATE_WOUT_YIELD | THREAD_CREATE_STACKTEST, + THREAD_CREATE_WOUT_YIELD | THREAD_CREATE_STACKTEST | THREAD_CREATE_REENT, thread3, NULL, "nr3"); puts("THREADS CREATED\n"); + ps(); msg_t msg; /* Wait until thread 1 is done */