From f9f0208daa41bf46032536be82bd6db0b4bebe2b Mon Sep 17 00:00:00 2001 From: Paul Hollinsky Date: Sun, 20 Oct 2024 10:19:56 -0700 Subject: [PATCH 1/2] POSIX Port: Round up stack sizes correctly on macOS On macOS, pthread_attr_setstacksize requires the stack size to be a multiple of the system page size. The previous changes here assumed the use of pthread_attr_setstack, however the transition to pthread_attr_setstacksize means we can avoid rounding the pxEndOfStack pointer. This is additionally positive because pxEndOfStack was actually being rounded in the wrong direction -- it ought to have been rounded down (_trunc) instead of up due to the stack growth direction. In my case this actually caused pxEndOfStack to end up after pxTopOfStack, which underflowed and created a huge stack size request + EXC_BAD_ACCESS in pthread_create. Signed-off-by: Paul Hollinsky --- portable/ThirdParty/GCC/Posix/port.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index 94e80cc4d6b..5785d425be0 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -165,14 +165,14 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack, thread = ( Thread_t * ) ( pxTopOfStack + 1 ) - 1; pxTopOfStack = ( StackType_t * ) thread - 1; - #ifdef __APPLE__ - pxEndOfStack = ( StackType_t * ) mach_vm_round_page( pxEndOfStack ); - #endif - ulStackSize = ( size_t ) ( pxTopOfStack + 1 - pxEndOfStack ) * sizeof( *pxTopOfStack ); #ifdef __APPLE__ - ulStackSize = mach_vm_trunc_page( ulStackSize ); + /* + * On macOS, pthread_attr_setstacksize requires the stack to be a multiple of the system page size. + * Round up to the next page boundary. + */ + ulStackSize = mach_vm_round_page( ulStackSize ); #endif thread->pxCode = pxCode; From 4a8daaaab33fa2b841deb03be1677ab3b1aca10e Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Thu, 24 Oct 2024 06:33:55 +0000 Subject: [PATCH 2/2] Code review suggestions Signed-off-by: Gaurav Aggarwal --- portable/ThirdParty/GCC/Posix/port.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index 5785d425be0..b11e9017fcf 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -165,30 +165,15 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack, thread = ( Thread_t * ) ( pxTopOfStack + 1 ) - 1; pxTopOfStack = ( StackType_t * ) thread - 1; + /* Ensure that there is enough space to store Thread_t on the stack. */ ulStackSize = ( size_t ) ( pxTopOfStack + 1 - pxEndOfStack ) * sizeof( *pxTopOfStack ); - - #ifdef __APPLE__ - /* - * On macOS, pthread_attr_setstacksize requires the stack to be a multiple of the system page size. - * Round up to the next page boundary. - */ - ulStackSize = mach_vm_round_page( ulStackSize ); - #endif + configASSERT( ulStackSize > sizeof( Thread_t ) ); thread->pxCode = pxCode; thread->pvParams = pvParameters; thread->xDying = pdFALSE; - /* Ensure ulStackSize is at least PTHREAD_STACK_MIN */ - ulStackSize = (ulStackSize < ( size_t ) ( PTHREAD_STACK_MIN ) ) ? ( size_t ) ( PTHREAD_STACK_MIN ) : ulStackSize; - pthread_attr_init( &xThreadAttributes ); - iRet = pthread_attr_setstacksize( &xThreadAttributes, ulStackSize ); - - if( iRet != 0 ) - { - fprintf( stderr, "[WARN] pthread_attr_setstacksize failed with return value: %d. Default stack size will be used.\n", iRet ); - } thread->ev = event_create();