Ensure static TLS region is free'd last

We were using `__cxa_thread_atexit` to register the funtion for free'ing
TLS data.  And because were were running the exit handlers
`__pthread_exit_run_handlers` before `__pthread_tsd_run_dtors` it meant
that any pthread_key descructor functions would not be able to access
TLS variables.

Instead we really need to delay free'ing of TLS data until all exit
handlers and pthread_key descructors have been run.

This was causing failure of `lsan.test_stdio_locking` due to the fact
that lsan uses TLS during pthread key destruction.  Specifically
`thread_finalize` (a pthread_key desctructor function) calls
`ThreadFinish` which in turn calls `GetCurrentThread` which uses a
thread local variable to store the current thread ID.

This change should allow #14489 to be dramatically simplified because
it will allow the use of TLS variables to implement the atexit queue.

Author: sbc100

.circleci/config.yml

@@ -369,7 +369,7 @@ jobs:
     steps:
       - run-tests:
           # also add a few asan tests
-          test_targets: "wasm2 asan.test_embind* asan.test_abort_on_exceptions asan.test_ubsan_full_left_shift_fsanitize_integer asan.test_pthread* asan.test_dyncall_specific_minimal_runtime asan.test_async_hello"
+          test_targets: "wasm2 asan.test_embind* asan.test_abort_on_exceptions asan.test_ubsan_full_left_shift_fsanitize_integer asan.test_pthread* asan.test_dyncall_specific_minimal_runtime asan.test_async_hello lsan.test_stdio_locking"
   test-wasm3:
     executor: bionic
     steps:

system/lib/compiler-rt/lib/lsan/lsan_interceptors.cpp

@@ -562,15 +562,14 @@ void InitializeInterceptors() {
   LSAN_MAYBE_INTERCEPT_PTHREAD_ATFORK;
 
   LSAN_MAYBE_INTERCEPT_STRERROR;
+#endif  // !SANITIZER_FUCHSIA && !SANITIZER_EMSCRIPTEN
 
 #if !SANITIZER_NETBSD && !SANITIZER_FREEBSD
   if (pthread_key_create(&g_thread_finalize_key, &thread_finalize)) {
     Report("LeakSanitizer: failed to create thread key.\n");
     Die();
   }
 #endif
-
-#endif  // !SANITIZER_FUCHSIA
 }
 
 } // namespace __lsan

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

@@ -39,11 +39,7 @@ int __pthread_key_delete(pthread_key_t k)
 	return 0;
 }
 
-#ifdef __EMSCRIPTEN__
-void EMSCRIPTEN_KEEPALIVE __pthread_tsd_run_dtors()
-#else
 void __pthread_tsd_run_dtors()
-#endif
 {
 	pthread_t self = __pthread_self();
 	int i, j, not_finished = self->tsd_used;

system/lib/pthread/emscripten_thread_state.s

@@ -1,3 +1,5 @@
+.globaltype __tls_base, i32
+
 .globaltype thread_id, i32
 thread_id:
 
@@ -24,6 +26,14 @@ _emscripten_thread_init:
   global.set is_runtime_thread
   end_function
 
+# Accessor for `__tls_base` symbol which is a wasm global an not directly
+# accessible from C/C++.
+.globl _emscripten_tls_base
+_emscripten_tls_base:
+  .functype _emscripten_tls_base () -> (i32)
+  global.get __tls_base
+  end_function
+
 # Semantically the same as testing "!ENVIRONMENT_IS_PTHREAD" in JS
 .globl emscripten_is_main_runtime_thread
 emscripten_is_main_runtime_thread:

system/lib/pthread/emscripten_tls_init.c

@@ -21,13 +21,6 @@ extern int __cxa_thread_atexit(void (*)(void *), void *, void *);
 
 extern int __dso_handle;
 
-static void free_tls(void* tls_block) {
-#ifdef DEBUG_TLS
-  printf("tls free: thread[%p] dso[%p] <- %p\n", pthread_self(), &__dso_handle, tls_block);
-#endif
-  emscripten_builtin_free(tls_block);
-}
-
 void* emscripten_tls_init(void) {
   size_t tls_size = __builtin_wasm_tls_size();
   size_t tls_align = __builtin_wasm_tls_align();
@@ -39,6 +32,5 @@ void* emscripten_tls_init(void) {
   printf("tls init: thread[%p] dso[%p] -> %p\n", pthread_self(), &__dso_handle, tls_block);
 #endif
   __wasm_init_tls(tls_block);
-  __cxa_thread_atexit(free_tls, tls_block, &__dso_handle);
   return tls_block;
 }

system/lib/pthread/pthread_create.c

@@ -22,6 +22,7 @@ extern int __pthread_create_js(struct pthread *thread, const pthread_attr_t *att
 extern void _emscripten_thread_init(int, int, int);
 extern void __pthread_exit_run_handlers();
 extern void __pthread_detached_exit();
+extern void* _emscripten_tls_base();
 extern int8_t __dso_handle;
 
 static void dummy_0()
@@ -104,6 +105,16 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
   return __pthread_create_js(new, attrp, entry, arg);
 }
 
+static void free_tls_data() {
+  void* tls_block = _emscripten_tls_base();
+  if (tls_block) {
+#ifdef DEBUG_TLS
+    printf("tls free: thread[%p] dso[%p] <- %p\n", pthread_self(), &__dso_handle, tls_block);
+#endif
+    emscripten_builtin_free(tls_block);
+  }
+}
+
 void _emscripten_thread_exit(void* result) {
   struct pthread *self = __pthread_self();
   assert(self);
@@ -117,6 +128,8 @@ void _emscripten_thread_exit(void* result) {
   // Call into the musl function that runs destructors of all thread-specific data.
   __pthread_tsd_run_dtors();
 
+  free_tls_data();
+
   __lock(self->exitlock);
 
   if (self == emscripten_main_browser_thread_id()) {

tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports

@@ -14,7 +14,7 @@ M
 N
 O
 P
-Q
+v
 w
 x
 y

tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.funcs

@@ -36,7 +36,6 @@ $emscripten_stack_set_limits
 $emscripten_sync_run_in_main_thread
 $emscripten_sync_run_in_main_thread
 $emscripten_tls_init
-$free_tls
 $init_file_lock
 $init_mparams
 $main

tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.imports

@@ -19,4 +19,3 @@ a.r
 a.s
 a.t
 a.u
-a.v

tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent

@@ -19,4 +19,3 @@ r
 s
 t
 u
-v

tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size

@@ -1 +1 @@
-16977
+16962