From 87847b1de4fe94befd251a921d9ae0c9d1b1c2cf Mon Sep 17 00:00:00 2001 From: chrysn Date: Tue, 11 Jan 2022 16:54:56 +0100 Subject: [PATCH 1/2] tests: Fix thread return with local message queue When a message queue is configured from the stack, that main function must never return -- otherwise, during sched_task_exit (which the thread's function "returns" to), message senders might still send messages into already freed stack space (which would be reused by sched_task_exit). Co-authored-by: Marian Buschsieweke --- tests/driver_nrf24l01p_lowlevel/main.c | 5 +++-- tests/posix_semaphore/main.c | 15 +++++++++------ tests/thread_msg_block_w_queue/main.c | 5 +++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/tests/driver_nrf24l01p_lowlevel/main.c b/tests/driver_nrf24l01p_lowlevel/main.c index 8e6a44f37a..6d30a0c146 100644 --- a/tests/driver_nrf24l01p_lowlevel/main.c +++ b/tests/driver_nrf24l01p_lowlevel/main.c @@ -109,12 +109,13 @@ void print_register(char reg, int num_bytes) char rx_handler_stack[THREAD_STACKSIZE_MAIN]; +static msg_t _msg_q[1]; + /* RX handler that waits for a message from the ISR */ void *nrf24l01p_rx_handler(void *arg) { (void)arg; - msg_t msg_q[1]; - msg_init_queue(msg_q, 1); + msg_init_queue(_msg_q, 1); unsigned int pid = thread_getpid(); char rx_buf[NRF24L01P_MAX_DATA_LENGTH]; diff --git a/tests/posix_semaphore/main.c b/tests/posix_semaphore/main.c index dedfacd536..2a8da0622f 100644 --- a/tests/posix_semaphore/main.c +++ b/tests/posix_semaphore/main.c @@ -129,10 +129,11 @@ static void test1(void) puts("first: end"); } +static msg_t _sema_thread_queue[SEMAPHORE_MSG_QUEUE_SIZE]; + static void *priority_sema_thread(void *name) { - msg_t msg_queue[SEMAPHORE_MSG_QUEUE_SIZE]; - msg_init_queue(msg_queue, SEMAPHORE_MSG_QUEUE_SIZE); + msg_init_queue(_sema_thread_queue, SEMAPHORE_MSG_QUEUE_SIZE); sem_wait(&s1); printf("Thread '%s' woke up.\n", (const char *) name); return NULL; @@ -176,11 +177,12 @@ void test2(void) } } +static msg_t _one_two_queue[SEMAPHORE_MSG_QUEUE_SIZE]; + static void *test3_one_two_thread(void *arg) { - msg_t msg_queue[SEMAPHORE_MSG_QUEUE_SIZE]; (void)arg; - msg_init_queue(msg_queue, SEMAPHORE_MSG_QUEUE_SIZE); + msg_init_queue(_one_two_queue, SEMAPHORE_MSG_QUEUE_SIZE); sem_wait(&s1); puts("Thread 1 woke up after waiting for s1."); sem_wait(&s2); @@ -188,11 +190,12 @@ static void *test3_one_two_thread(void *arg) return NULL; } +static msg_t _two_one_queue[SEMAPHORE_MSG_QUEUE_SIZE]; + static void *test3_two_one_thread(void *arg) { - msg_t msg_queue[SEMAPHORE_MSG_QUEUE_SIZE]; (void)arg; - msg_init_queue(msg_queue, SEMAPHORE_MSG_QUEUE_SIZE); + msg_init_queue(_two_one_queue, SEMAPHORE_MSG_QUEUE_SIZE); sem_wait(&s2); puts("Thread 2 woke up after waiting for s2."); sem_wait(&s1); diff --git a/tests/thread_msg_block_w_queue/main.c b/tests/thread_msg_block_w_queue/main.c index c77388867e..521d14f136 100644 --- a/tests/thread_msg_block_w_queue/main.c +++ b/tests/thread_msg_block_w_queue/main.c @@ -52,13 +52,14 @@ void *sender_thread(void *arg) return NULL; } +static msg_t _msg_q[1]; + int main(void) { msg_t msg; p_recv = thread_getpid(); - msg_t msg_q[1]; - msg_init_queue(msg_q, 1); + msg_init_queue(_msg_q, 1); p_send = thread_create(t1_stack, sizeof(t1_stack), THREAD_PRIORITY_MAIN - 1, THREAD_CREATE_WOUT_YIELD | THREAD_CREATE_STACKTEST, From b4a185132ab93e7b27bf7fe4cc97f81baf12d687 Mon Sep 17 00:00:00 2001 From: chrysn Date: Tue, 11 Jan 2022 17:18:38 +0100 Subject: [PATCH 2/2] core/msg: Document caution needed when having queue on the stack --- core/include/msg.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/include/msg.h b/core/include/msg.h index e56a0567aa..e81204d978 100644 --- a/core/include/msg.h +++ b/core/include/msg.h @@ -375,6 +375,9 @@ unsigned msg_avail(void); * not be NULL. * @param[in] num Number of ``msg_t`` structures in array. * **MUST BE POWER OF TWO!** + * + * If array resides on the stack, the containing stack frame must never be + * left, not even if it is the current thread's entry function. */ void msg_init_queue(msg_t *array, int num);