Skip to content

Commit 85d58ff

Browse files
authored
Don't call emscripten_tls_init from static constructor. NFC (#14981)
This change makes handling of `emscripten_tls_init` the same on all threads, including the main thread and/or library loading thread. This is precursor to a change that makes use of the return value of emscripten_tls_init to do TLS relocations so making the call explicit is required. I've also removed the use of EM_ASM from library_pthread.c.
1 parent f169d66 commit 85d58ff

File tree

8 files changed

+39
-36
lines changed

8 files changed

+39
-36
lines changed

emcc.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1864,7 +1864,7 @@ def default_setting(name, new_default):
18641864
exit_with_error('USE_PTHREADS + BUILD_AS_WORKER require separate modes that don\'t work together, see https://github.com/emscripten-core/emscripten/issues/8854')
18651865
settings.JS_LIBRARIES.append((0, 'library_pthread.js'))
18661866
settings.EXPORTED_FUNCTIONS += [
1867-
'___emscripten_pthread_data_constructor',
1867+
'___emscripten_init_main_thread',
18681868
'__emscripten_call_on_thread',
18691869
'__emscripten_main_thread_futex',
18701870
'__emscripten_thread_init',

src/library_dylink.js

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -483,17 +483,20 @@ var LibraryDylink = {
483483

484484
// initialize the module
485485
#if USE_PTHREADS
486-
// The main thread does a full __wasm_call_ctors (which includes a call
487-
// to emscripten_tls_init), but secondary threads should not call static
488-
// constructors in general - emscripten_tls_init is the exception.
489-
if (ENVIRONMENT_IS_PTHREAD) {
490-
var init = moduleExports['emscripten_tls_init'];
491-
assert(init);
486+
// Only one thread (currently The main thread) should call
487+
// __wasm_call_ctors, but all threads need to call emscripten_tls_init
488+
var initTLS = moduleExports['emscripten_tls_init'];
489+
#if ASSERTIONS
490+
assert(initTLS);
491+
#endif
492492
#if DYLINK_DEBUG
493-
out("adding to tlsInitFunctions: " + init);
493+
out("adding to tlsInitFunctions: " + initTLS);
494494
#endif
495-
PThread.tlsInitFunctions.push(init);
496-
} else {
495+
PThread.tlsInitFunctions.push(initTLS);
496+
if (runtimeInitialized) {
497+
initTLS();
498+
}
499+
if (!ENVIRONMENT_IS_PTHREAD) {
497500
#endif
498501
var init = moduleExports['__wasm_call_ctors'];
499502
if (init) {

src/library_pthread.js

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,7 @@ var LibraryPThread = {
3737
}
3838
#endif
3939
},
40-
initRuntime: function(tb) {
41-
#if PTHREADS_PROFILING
42-
PThread.createProfilerBlock(tb);
43-
PThread.setThreadName(tb, "Browser main thread");
44-
PThread.setThreadStatus(tb, {{{ cDefine('EM_THREAD_STATUS_RUNNING') }}});
45-
#endif
4640

47-
// Pass the thread address to the native code where they stored in wasm
48-
// globals which act as a form of TLS. Global constructors trying
49-
// to access this value will read the wrong value, but that is UB anyway.
50-
__emscripten_thread_init(tb, /*isMainBrowserThread=*/!ENVIRONMENT_IS_WORKER, /*isMainRuntimeThread=*/1);
51-
#if ASSERTIONS
52-
PThread.mainRuntimeThread = true;
53-
#endif
54-
},
5541
initWorker: function() {
5642
#if USE_CLOSURE_COMPILER
5743
// worker.js is not compiled together with us, and must access certain
@@ -237,7 +223,7 @@ var LibraryPThread = {
237223
// Called by worker.js each time a thread is started.
238224
threadInit: function() {
239225
#if PTHREADS_DEBUG
240-
out("threadInit");
226+
out('Pthread 0x' + _pthread_self().toString(16) + ' threadInit.');
241227
#endif
242228
#if PTHREADS_PROFILING
243229
PThread.setThreadStatus(_pthread_self(), {{{ cDefine('EM_THREAD_STATUS_RUNNING') }}});
@@ -565,6 +551,22 @@ var LibraryPThread = {
565551
return navigator['hardwareConcurrency'];
566552
},
567553

554+
__emscripten_init_main_thread_js: function(tb) {
555+
#if PTHREADS_PROFILING
556+
PThread.createProfilerBlock(tb);
557+
PThread.setThreadName(tb, "Browser main thread");
558+
#endif
559+
560+
// Pass the thread address to the native code where they stored in wasm
561+
// globals which act as a form of TLS. Global constructors trying
562+
// to access this value will read the wrong value, but that is UB anyway.
563+
__emscripten_thread_init(tb, /*isMainBrowserThread=*/!ENVIRONMENT_IS_WORKER, /*isMainRuntimeThread=*/1);
564+
#if ASSERTIONS
565+
PThread.mainRuntimeThread = true;
566+
#endif
567+
PThread.threadInit();
568+
},
569+
568570
__pthread_create_js__sig: 'iiiii',
569571
__pthread_create_js__deps: ['$spawnThread', 'pthread_self', 'memalign', 'emscripten_sync_run_in_main_thread_4'],
570572
__pthread_create_js: function(pthread_ptr, attr, start_routine, arg) {

src/postamble_minimal.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ function initRuntime(asm) {
5858

5959
#if USE_PTHREADS
6060
// Export needed variables that worker.js needs to Module.
61-
Module['_emscripten_tls_init'] = _emscripten_tls_init;
6261
Module['HEAPU32'] = HEAPU32;
6362
Module['__emscripten_thread_init'] = __emscripten_thread_init;
6463
Module['_pthread_self'] = _pthread_self;
@@ -77,6 +76,10 @@ function initRuntime(asm) {
7776
#endif
7877
#endif
7978

79+
#if USE_PTHREADS
80+
PThread.tlsInitFunctions.push(asm['emscripten_tls_init']);
81+
#endif
82+
8083
#if hasExportedFunction('___wasm_call_ctors')
8184
asm['__wasm_call_ctors']();
8285
#endif

system/lib/README.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ These current set of static constructors in system libraries and their prioritie
1717
(lowest run first) are:
1818

1919
- 47: `initialize_emmalloc_heap` (emmalloc.c)
20-
- 48: `__emscripten_pthread_data_constructor` (pthread/emscripten_tls_init.c)
21-
- 49: `emscripten_tls_init` (pthread/library_pthread.c)
20+
- 48: `__emscripten_pthread_data_constructor` (pthread/library_pthread.c)
2221
- 50: asan init (??)
2322

2423
Priorities 0 - 100 are reserved for system libraries and user-level

system/lib/pthread/emscripten_tls_init.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ static void free_tls(void* tls_block) {
2828
emscripten_builtin_free(tls_block);
2929
}
3030

31-
//See system/lib/README.md for static constructor ordering.
32-
__attribute__((constructor(49)))
3331
void emscripten_tls_init(void) {
3432
size_t tls_size = __builtin_wasm_tls_size();
3533
size_t tls_align = __builtin_wasm_tls_align();

system/lib/pthread/library_pthread.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -869,14 +869,12 @@ int _emscripten_call_on_thread(int forceAsync, pthread_t targetThread, EM_FUNC_S
869869
// the main thread is waiting, we wake it up before waking up any workers.
870870
EMSCRIPTEN_KEEPALIVE void* _emscripten_main_thread_futex;
871871

872-
EM_JS(void, initPthreadsJS, (void* tb), {
873-
PThread.initRuntime(tb);
874-
})
872+
void __emscripten_init_main_thread_js(void* tb);
875873

876874
// See system/lib/README.md for static constructor ordering.
877875
__attribute__((constructor(48)))
878-
void __emscripten_pthread_data_constructor(void) {
879-
initPthreadsJS(&__main_pthread);
876+
void __emscripten_init_main_thread(void) {
877+
__emscripten_init_main_thread_js(&__main_pthread);
880878
// The pthread struct has a field that points to itself - this is used as
881879
// a magic ID to detect whether the pthread_t structure is 'alive'.
882880
__main_pthread.self = &__main_pthread;

tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.funcs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
$GetQueue
2-
$__emscripten_pthread_data_constructor
2+
$__emscripten_init_main_thread
33
$__errno_location
44
$__pthread_mutex_lock
55
$__pthread_mutex_trylock

0 commit comments

Comments
 (0)