Minor cleanups

Author: kleisauke

src/library_pthread.js

@@ -269,7 +269,7 @@ var LibraryPThread = {
         worker.on('error', function(e) {
           worker.onerror(e);
         });
-        worker.on('detachedExit', function() {
+        worker.on('exit', function() {
           // TODO: update the worker queue?
           // See: https://github.com/emscripten-core/emscripten/issues/9763
         });

src/struct_info_internal.json

@@ -24,10 +24,10 @@
               "_a_transferredcanvases"
             ],
             "thread_profiler_block": [
-                "threadStatus",
-                "currentStatusStartTime",
-                "timeSpentInStatus",
-                "name"
+              "threadStatus",
+              "currentStatusStartTime",
+              "timeSpentInStatus",
+              "name"
             ]
         },
         "defines": [

system/lib/libc/musl/src/internal/pthread_impl.h

@@ -10,10 +10,12 @@
 #include "syscall.h"
 #include "atomic.h"
 #ifdef __EMSCRIPTEN__
+#include <math.h>
 #include <emscripten/threading.h>
 #include "threading_internal.h"
-#endif
+#else
 #include "futex.h"
+#endif
 
 #include "pthread_arch.h"
 
@@ -186,11 +188,12 @@ hidden int __timedwait_cp(volatile int *, int, clockid_t, const struct timespec
 hidden void __wait(volatile int *, volatile int *, int, int);
 static inline void __wake(volatile void *addr, int cnt, int priv)
 {
-	if (priv) priv = FUTEX_PRIVATE;
-	if (cnt<0) cnt = INT_MAX;
 #ifdef __EMSCRIPTEN__
-	emscripten_futex_wake(addr, cnt);
+	(void)priv;
+	emscripten_futex_wake(addr, cnt < 0 ? INT_MAX : cnt);
 #else
+	if (priv) priv = FUTEX_PRIVATE;
+	if (cnt<0) cnt = INT_MAX;
 	__syscall(SYS_futex, addr, FUTEX_WAKE|priv, cnt) != -ENOSYS ||
 	__syscall(SYS_futex, addr, FUTEX_WAKE, cnt);
 #endif

system/lib/libc/musl/src/thread/__timedwait.c

@@ -2,12 +2,11 @@
 #include <time.h>
 #include <errno.h>
 #ifdef __EMSCRIPTEN__
-#include <math.h>
 #include <emscripten/emscripten.h>  // for emscripten_get_now()
 #else
 #include "futex.h"
-#endif
 #include "syscall.h"
+#endif
 #include "pthread_impl.h"
 
 #ifndef __EMSCRIPTEN__
@@ -42,7 +41,9 @@ int __timedwait_cp(volatile int *addr, int val,
 	int r;
 	struct timespec to, *top=0;
 
+#ifndef __EMSCRIPTEN__
 	if (priv) priv = FUTEX_PRIVATE;
+#endif
 
 	if (at) {
 		if (at->tv_nsec >= 1000000000UL) return EINVAL;

system/lib/libc/musl/src/thread/__wait.c

@@ -1,14 +1,11 @@
-#ifdef __EMSCRIPTEN__
-#include <math.h>
-#include <emscripten/threading.h>
-#endif
-
 #include "pthread_impl.h"
 
 void __wait(volatile int *addr, volatile int *waiters, int val, int priv)
 {
 	int spins=100;
+#ifndef __EMSCRIPTEN__
 	if (priv) priv = FUTEX_PRIVATE;
+#endif
 	while (spins-- && (!waiters || !*waiters)) {
 		if (*addr==val) a_spin();
 		else return;

system/lib/libc/musl/src/thread/pthread_barrier_wait.c

@@ -1,7 +1,3 @@
-#ifdef __EMSCRIPTEN__
-#include <math.h>
-#endif
-
 #include "pthread_impl.h"
 
 static int pshared_barrier_wait(pthread_barrier_t *b)

system/lib/libc/musl/src/thread/pthread_detach.c

@@ -3,10 +3,11 @@
 
 static int __pthread_detach(pthread_t t)
 {
+#ifdef __EMSCRIPTEN__
 	// XXX EMSCRIPTEN: Add check for invalid (already joined) thread.  Again
 	// for the benefit of the conformance tests.
-	if (t->self != t)
-		return ESRCH;
+	if (t->self != t) return ESRCH;
+#endif
 	/* If the cas fails, detach state is either already-detached
 	 * or exiting/exited, and pthread_join will trap or cleanup. */
 	if (a_cas(&t->detach_state, DT_JOINABLE, DT_DETACHED) != DT_JOINABLE)
@@ -16,5 +17,6 @@ static int __pthread_detach(pthread_t t)
 
 weak_alias(__pthread_detach, pthread_detach);
 weak_alias(__pthread_detach, thrd_detach);
-// XXX EMSCRIPTEN: add extra alias for asan.
+#ifdef __EMSCRIPTEN__ // XXX Emscripten add an extra alias for ASan/LSan.
 weak_alias(__pthread_detach, emscripten_builtin_pthread_detach);
+#endif

system/lib/libc/musl/src/thread/pthread_key_create.c

@@ -1,9 +1,5 @@
 #include "pthread_impl.h"
 
-#ifdef __EMSCRIPTEN__
-#include <emscripten.h>
-#endif
-
 volatile size_t __pthread_tsd_size = sizeof(void *) * PTHREAD_KEYS_MAX;
 void *__pthread_tsd_main[PTHREAD_KEYS_MAX] = { 0 };
 

system/lib/libc/musl/src/thread/thrd_create.c

@@ -6,8 +6,6 @@
 // if we call the internal __pthread_create function here to don't the wrapping
 // See pthread_create wrapper in compiler-rt/lib/lsan/lsan_interceptors.cpp.
 #define __pthread_create pthread_create
-#else
-int __pthread_create(pthread_t *restrict, const pthread_attr_t *restrict, void *(*)(void *), void *restrict);
 #endif
 
 int thrd_create(thrd_t *thr, thrd_start_t func, void *arg)

system/lib/pthread/library_pthread.c

@@ -5,30 +5,16 @@
  * found in the LICENSE file.
  */
 
-#define _GNU_SOURCE
-#include "../internal/libc.h"
-#include "../internal/pthread_impl.h"
+#include "libc.h"
+#include "pthread_impl.h"
 #include <assert.h>
-#include <dirent.h>
-#include <errno.h>
-#include <fcntl.h>
 #include <math.h>
-#include <poll.h>
 #include <pthread.h>
 #include <stdatomic.h>
 #include <stdarg.h>
 #include <stdbool.h>
-#include <stdlib.h>
-#include <sys/ioctl.h>
-#include <sys/mman.h>
-#include <sys/socket.h>
-#include <sys/stat.h>
-#include <sys/statvfs.h>
-#include <sys/time.h>
-#include <termios.h>
 #include <threads.h>
 #include <unistd.h>
-#include <utime.h>
 
 #include <emscripten.h>
 #include <emscripten/threading.h>
@@ -834,13 +820,16 @@ void __emscripten_init_main_thread(void) {
   __main_pthread.stack = (void*)emscripten_stack_get_base();
   __main_pthread.stack_size = emscripten_stack_get_base() - emscripten_stack_get_end();
   __main_pthread.detach_state = DT_JOINABLE;
-  // pthread struct robust_list head should point to itself.
-  __main_pthread.robust_list.head = &__main_pthread.robust_list.head;
   // Main thread ID is always 1.  It can't be 0 because musl assumes
   // tid is always non-zero.
   __main_pthread.tid = getpid();
   __main_pthread.locale = &libc.global_locale;
+  // Initialize thread-specific data area.
+  __main_pthread.tsd = (void **)__pthread_tsd_main;
+  // pthread struct robust_list head should point to itself.
+  __main_pthread.robust_list.head = &__main_pthread.robust_list.head;
+  // pthread struct prev and next should initially point to itself (see __init_tp),
+  // this is used by pthread_key_delete for deleting thread-specific data.
   // TODO(sbc): Implement circular list of threads
   //__main_pthread.next = __main_pthread.prev = &__main_pthread;
-  __main_pthread.tsd = (void **)__pthread_tsd_main;
 }

system/lib/pthread/pthread_create.c

@@ -5,7 +5,6 @@
  * found in the LICENSE file.
  */
 
-#define _GNU_SOURCE
 #include "pthread_impl.h"
 #include "stdio_impl.h"
 #include "assert.h"
@@ -22,7 +21,7 @@
 
 // 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 int __pthread_create_js(pthread_t thread, const pthread_attr_t *attr, void *(*start_routine) (void *), void *arg);
 extern void __set_thread_state(pthread_t ptr, int is_main, int is_runtime, int can_block);
 extern int _emscripten_default_pthread_stack_size();
 extern void __emscripten_thread_cleanup(pthread_t thread);
@@ -76,7 +75,7 @@ int __pthread_create(pthread_t* restrict res,
                      void* (*entry)(void*),
                      void* restrict arg) {
   // Note on LSAN: lsan intercepts/wraps calls to pthread_create so any
-  // allocation we we do here should be considered leaks.
+  // allocation we do here should be considered as leak.
   // See: lsan_interceptors.cpp.
   if (!res) {
     return EINVAL;
@@ -143,8 +142,6 @@ int __pthread_create(pthread_t* restrict res,
   _emscripten_thread_profiler_init(new);
 #endif
 
-  //printf("start __pthread_create: %p\n", self);
-
   // Set libc.need_locks before calling __pthread_create_js since
   // by the time it returns the thread could be running and we
   // want libc.need_locks to be set from the moment it starts.
@@ -169,7 +166,6 @@ int __pthread_create(pthread_t* restrict res,
   */
 
   *res = new;
-  //printf("done __pthread_create self=%p next=%p prev=%p new=%p\n", self, self->next, self->prev, new);
   return 0;
 }
 
@@ -202,11 +198,11 @@ static void free_tls_data() {
 }
 
 void _emscripten_thread_exit(void* result) {
-  struct pthread *self = __pthread_self();
+  pthread_t self = __pthread_self();
   assert(self);
 
-  self->canceldisable = PTHREAD_CANCEL_DISABLE;
-  self->cancelasync = PTHREAD_CANCEL_DEFERRED;
+  self->canceldisable = 1;
+  self->cancelasync = 0;
   self->result = result;
 
   // Run any handlers registered with pthread_cleanup_push
@@ -235,16 +231,16 @@ void _emscripten_thread_exit(void* result) {
     return;
   }
 
-  // We have the call the buildin free here since lsan handling for this thread
+  // We have the call the builtin free here since lsan handling for this thread
   // gets shut down during __pthread_tsd_run_dtors.
   emscripten_builtin_free(self->tsd);
   self->tsd = NULL;
 
   // Not hosting a pthread anymore in this worker set __pthread_self to NULL
   __set_thread_state(NULL, 0, 0, 1);
 
-  /* This atomic potentially competes with a concurrent pthread_detach
-   * call; the loser is responsible for freeing thread resources. */
+  // This atomic potentially competes with a concurrent pthread_detach
+  // call; the loser is responsible for freeing thread resources.
   int state = a_cas(&self->detach_state, DT_JOINABLE, DT_EXITING);
 
   if (state == DT_DETACHED) {
@@ -258,7 +254,7 @@ void _emscripten_thread_exit(void* result) {
   }
 }
 
-// Mark as `no_sanitize("address"` since emscripten_pthread_exit destroys
+// Mark as `no_sanitize("address")` since emscripten_pthread_exit destroys
 // the current thread and runs its exit handlers.  Without this asan injects
 // a call to __asan_handle_no_return before emscripten_unwind_to_js_event_loop
 // which seem to cause a crash later down the line.

tests/pthread/main_thread_join.cpp

@@ -33,7 +33,7 @@ void *ThreadMain(void *arg) {
   // succeeding.
   while (tries.load() < EXPECTED_TRIES) {}
 #endif
-	pthread_exit((void*)0);
+  pthread_exit((void*)0);
 }
 
 pthread_t CreateThread() {

tests/test_posixtest.py

@@ -7,6 +7,15 @@
    ./tests/third_party/posixtestsuite
 See
    https://github.com/emscripten-core/posixtestsuite
+
+Verify that it runs properly in musl:
+   cd tests/third_party/posixtestsuite
+   docker run -v $(pwd):/app -w /app -it alpine:latest sh -c "\
+      apk add build-base && \
+      POSIX_TARGET=conformance/interfaces/[INTERFACE_DIR] make LDFLAGS='-pthread' CFLAGS='-D__EMSCRIPTEN__'"
+   cat logfile
+
+Where [INTERFACE_DIR] is for e.g.: pthread_detach
 """
 
 import glob
@@ -153,6 +162,15 @@ def f(self):
     if name in disabled:
       self.skipTest(disabled[name])
     args = ['-I' + os.path.join(testsuite_root, 'include'),
+            # TODO(kleisauke): Run with ASan. Note that this requires matching signatures, i.e:
+            # void *thread_func() -> void *thread_func(void *unused)
+            # void *a_thread_func() -> void *a_thread_func(void *unused)
+            # void *a_thread_function() -> void *a_thread_function(void *unused)
+            # void a_cleanup_func() -> void a_cleanup_func(void *unused)
+            # etc, etc.
+            # '-O0',
+            # '-g3',
+            # '-fsanitize=address',
             '-Werror',
             '-Wno-format-security',
             '-Wno-int-conversion',