Allocate stack for threads in native code. NFC

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();

src/struct_info_internal.json

@@ -31,7 +31,6 @@
             ]
         },
         "defines": [
-          "__ATTRP_C11_THREAD",
           "EM_THREAD_NAME_MAX"
         ]
     },

system/lib/pthread/pthread_create.c

@@ -18,10 +18,13 @@
 // TODO(sbc): Should these be in their own header to avoid emmalloc here?
 #include <emscripten/emmalloc.h>
 
+#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);

tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports

@@ -14,7 +14,6 @@ M
 N
 O
 P
-u
 v
 w
 x

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

tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent

@@ -18,3 +18,4 @@ q
 r
 s
 t
+u

tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size

@@ -1 +1 @@
-17669
+17782

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,