From 4b689851dee5c00cfdc390dac4b0937675d3add1 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 1 Dec 2023 13:28:08 -0500 Subject: [PATCH 1/2] Fix #1429, adjust pthread stack to account for TCB+TLS The glibc pthread implementation puts additional data structures at the base of the stack. The size of these is not known, but the only thing to go by is PTHREAD_MIN_STACK -- which should always be enough to hold the required objects. Instead of simply assuring that the stack is at least PTHREAD_MIN_STACK, add this to the requested stack instead. This in turn will ensure that the user always has at least their requested stack size available for actual use. If the pthread implementation does not advertise a PTHREAD_MIN_STACK value, then just reserve 1 extra page. --- src/os/posix/src/os-impl-tasks.c | 37 ++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/os/posix/src/os-impl-tasks.c b/src/os/posix/src/os-impl-tasks.c index f41428661..a64fec5cc 100644 --- a/src/os/posix/src/os-impl-tasks.c +++ b/src/os/posix/src/os-impl-tasks.c @@ -37,10 +37,26 @@ #include "os-shared-idmap.h" /* - * Defines + * Extra Stack Space for overhead - + * + * glibc pthreads implicitly puts its TCB/TLS structures at the base of the stack. + * The actual size of these structures is highly system and configuration dependent. + * Experimentation/measurement on an x64-64 system with glibc shows this to consume + * between 9-10kB of stack space off the top. + * + * There does not seem to be a reliable/standardized way of determining how large this + * is going to be, but PTHREAD_STACK_MIN (if defined) should include adequate space for it. + * If this is not defined, then just assume 1 page. + * + * Importantly - when the user passes a stack size to OS_TaskCreate() - the expectation is + * that there will be at least this amount of real _usable_ space for the task. If 10kB + * is used before the entry point is even called, this needs to be accounted for, or else + * the stack might end up being too small. */ -#ifndef PTHREAD_STACK_MIN -#define PTHREAD_STACK_MIN (8 * 1024) +#ifdef PTHREAD_STACK_MIN +#define OS_IMPL_STACK_EXTRA PTHREAD_STACK_MIN +#else +#define OS_IMPL_STACK_EXTRA POSIX_GlobalVars.PageSize #endif /* Tables where the OS object information is stored */ @@ -451,20 +467,9 @@ int32 OS_Posix_InternalTaskCreate_Impl(pthread_t *pthr, osal_priority_t priority } /* - * Adjust the stack size parameter. - * - * POSIX has additional restrictions/limitations on the stack size of tasks that - * other RTOS environments may not have. Specifically POSIX says that the stack - * size must be at least PTHREAD_STACK_MIN and may also need to be a multiple of the - * system page size. - * - * Rounding up means the user might get a bigger stack than they requested, but - * that should not break anything aside from consuming extra memory. + * Adjust the stack size parameter, add budget for TCB/TLS overhead. */ - if (stacksz < PTHREAD_STACK_MIN) - { - stacksz = PTHREAD_STACK_MIN; - } + stacksz += OS_IMPL_STACK_EXTRA; stacksz += POSIX_GlobalVars.PageSize - 1; stacksz -= stacksz % POSIX_GlobalVars.PageSize; From 9efb7c1d41ea7b288fd218913daa9fc956737fd0 Mon Sep 17 00:00:00 2001 From: Dylan Date: Tue, 12 Dec 2023 08:31:56 -0500 Subject: [PATCH 2/2] Updating documentation and version numbers for v6.0.0-rc4+dev247 --- CHANGELOG.md | 6 +++++- src/os/inc/osapi-version.h | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc33a0ca9..7e6119278 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ # Changelog -##Development Build: v6.0.0-rc4+dev243 +## Development Build: v6.0.0-rc4+dev247 +- adjust pthread stack to account for TCB+TLS +- See + +## Development Build: v6.0.0-rc4+dev243 - Wrong memory alignment calculation - See diff --git a/src/os/inc/osapi-version.h b/src/os/inc/osapi-version.h index 95d2ab90c..84ac26ce8 100644 --- a/src/os/inc/osapi-version.h +++ b/src/os/inc/osapi-version.h @@ -34,7 +34,7 @@ /* * Development Build Macro Definitions */ -#define OS_BUILD_NUMBER 243 +#define OS_BUILD_NUMBER 247 #define OS_BUILD_BASELINE "v6.0.0-rc4" /*