From 3179de23bb49578b09aeb2472ccdec46c776b100 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Mon, 15 Nov 2021 18:21:24 -0800 Subject: [PATCH] Allocate stack for threads in native code. NFC Followup to #15533 --- src/library_pthread.js | 45 +++---------------- src/struct_info_internal.json | 1 - system/lib/pthread/pthread_create.c | 23 ++++++++-- ...n_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports | 1 - ...n_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.imports | 1 + ...main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent | 1 + ...main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size | 2 +- tests/reference_struct_info.json | 1 - 8 files changed, 29 insertions(+), 46 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index e8c6886aa91df..0488a4794572d 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -546,21 +546,12 @@ var LibraryPThread = { PThread.runningWorkers.push(worker); - var stackHigh = threadParams.stackBase + threadParams.stackSize; - // Create a pthread info object to represent this thread. var pthread = PThread.pthreads[threadParams.pthread_ptr] = { worker: worker, - stackBase: threadParams.stackBase, - stackSize: threadParams.stackSize, // Info area for this thread in Emscripten HEAP (shared) threadInfoStruct: threadParams.pthread_ptr }; - var tis = pthread.threadInfoStruct >> 2; - // spawnThread is always called with a zero-initialized thread struct so - // no need to set any valudes to zero here. - Atomics.store(HEAPU32, tis + ({{{ C_STRUCTS.pthread.stack_size }}} >> 2), threadParams.stackSize); - Atomics.store(HEAPU32, tis + ({{{ C_STRUCTS.pthread.stack }}} >> 2), stackHigh); #if PTHREADS_PROFILING PThread.createProfilerBlock(pthread.threadInfoStruct); @@ -746,34 +737,6 @@ var LibraryPThread = { // with the detected error. if (error) return error; - var stackSize = 0; - var stackBase = 0; - // When musl creates C11 threads it passes __ATTRP_C11_THREAD (-1) which - // treat as if it was NULL. - if (attr && attr != {{{ cDefine('__ATTRP_C11_THREAD') }}}) { - stackSize = {{{ makeGetValue('attr', 0/*_a_stacksize*/, 'i32') }}}; - stackBase = {{{ makeGetValue('attr', 8/*_a_stackaddr*/, 'i32') }}}; - } else { - // According to - // http://man7.org/linux/man-pages/man3/pthread_create.3.html, default - // stack size if not specified is 2 MB, so follow that convention. - stackSize = {{{ DEFAULT_PTHREAD_STACK_SIZE }}}; - } - // If stackBase is zero then we allocate a new stack region and mark it as - // owned. - if (stackBase == 0) { - // Allocate a stack if the user doesn't want to place the stack in a - // custom memory area. - stackBase = _memalign({{{ STACK_ALIGN }}}, stackSize); - Atomics.store(HEAPU32, (pthread_ptr + {{{ C_STRUCTS.pthread.stack_owned }}}) >> 2, 1); - } else { - // Musl stores the stack base address assuming stack grows downwards, so - // adjust it to Emscripten convention that the - // stack grows upwards instead. - stackBase -= stackSize; - assert(stackBase > 0); - } - #if OFFSCREENCANVAS_SUPPORT // Register for each of the transferred canvases that the new thread now // owns the OffscreenCanvas. @@ -784,8 +747,6 @@ var LibraryPThread = { #endif var threadParams = { - stackBase: stackBase, - stackSize: stackSize, startRoutine: start_routine, pthread_ptr: pthread_ptr, arg: arg, @@ -1227,6 +1188,12 @@ var LibraryPThread = { return func.apply(null, _emscripten_receive_on_main_thread_js_callArgs); }, + // TODO(sbc): Do we really need this to be dynamically settable from JS like this? + // See https://github.com/emscripten-core/emscripten/issues/15101. + _emscripten_default_pthread_stack_size: function() { + return {{{ DEFAULT_PTHREAD_STACK_SIZE }}}; + }, + $establishStackSpace__internal: true, $establishStackSpace: function() { var pthread_ptr = _pthread_self(); diff --git a/src/struct_info_internal.json b/src/struct_info_internal.json index f61a60d1b9d6a..6ff037c182952 100644 --- a/src/struct_info_internal.json +++ b/src/struct_info_internal.json @@ -31,7 +31,6 @@ ] }, "defines": [ - "__ATTRP_C11_THREAD", "EM_THREAD_NAME_MAX" ] }, diff --git a/system/lib/pthread/pthread_create.c b/system/lib/pthread/pthread_create.c index b6336ba7d6ef3..6edd7da6346f4 100644 --- a/system/lib/pthread/pthread_create.c +++ b/system/lib/pthread/pthread_create.c @@ -18,10 +18,13 @@ // TODO(sbc): Should these be in their own header to avoid emmalloc here? #include +#define STACK_ALIGN 16 + // See musl's pthread_create.c extern int __pthread_create_js(struct pthread *thread, const pthread_attr_t *attr, void *(*start_routine) (void *), void *arg); extern void _emscripten_thread_init(int, int, int); +extern int _emscripten_default_pthread_stack_size(); extern void __pthread_detached_exit(); extern void* _emscripten_tls_base(); extern int8_t __dso_handle; @@ -116,10 +119,24 @@ int __pthread_create(pthread_t* restrict res, new->tsd = malloc(PTHREAD_KEYS_MAX * sizeof(void*)); memset(new->tsd, 0, PTHREAD_KEYS_MAX * sizeof(void*)); - if (attrp && attrp != __ATTRP_C11_THREAD && attrp->_a_detach) { - new->detach_state = DT_DETACHED; + new->detach_state = DT_JOINABLE; + + if (attrp && attrp != __ATTRP_C11_THREAD) { + if (attrp->_a_detach) { + new->detach_state = DT_DETACHED; + } + new->stack_size = attrp->_a_stacksize; + new->stack = (void*)attrp->_a_stackaddr; } else { - new->detach_state = DT_JOINABLE; + new->stack_size = _emscripten_default_pthread_stack_size(); + } + + if (!new->stack) { + char* stackBase = memalign(STACK_ALIGN, new->stack_size); + // musl stores top of the stack in pthread_t->stack (i.e. the high + // end from which it grows down). + new->stack = stackBase + new->stack_size; + new->stack_owned = 1; } //printf("start __pthread_create: %p\n", self); diff --git a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports index 292e15a7fc463..8bbfd29999464 100644 --- a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports +++ b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports @@ -14,7 +14,6 @@ M N O P -u v w x diff --git a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.imports b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.imports index 9479133a56d79..014f2e2290126 100644 --- a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.imports +++ b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.imports @@ -18,3 +18,4 @@ a.q a.r a.s a.t +a.u diff --git a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent index b64b08c98af4d..ad11ff431eda4 100644 --- a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent +++ b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent @@ -18,3 +18,4 @@ q r s t +u diff --git a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size index 0b581918e6535..9d3fb083e57a7 100644 --- a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size +++ b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size @@ -1 +1 @@ -17669 +17782 diff --git a/tests/reference_struct_info.json b/tests/reference_struct_info.json index ea55e9d7d8306..663cf29f7ce01 100644 --- a/tests/reference_struct_info.json +++ b/tests/reference_struct_info.json @@ -358,7 +358,6 @@ "UUID_VARIANT_DCE": 1, "W_OK": 2, "X_OK": 1, - "__ATTRP_C11_THREAD": -1, "__WASI_CLOCKID_MONOTONIC": 1, "__WASI_CLOCKID_PROCESS_CPUTIME_ID": 2, "__WASI_CLOCKID_REALTIME": 0,