Skip to content

Ensure static TLS region is free'd last #15086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sbc100 merged 1 commit into from
Sep 22, 2021
Merged

Ensure static TLS region is free'd last #15086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions system/lib/compiler-rt/lib/lsan/lsan_interceptors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,15 +562,14 @@ void InitializeInterceptors() {
LSAN_MAYBE_INTERCEPT_PTHREAD_ATFORK;

LSAN_MAYBE_INTERCEPT_STRERROR;
#endif // !SANITIZER_FUCHSIA && !SANITIZER_EMSCRIPTEN

@kripken kripken Sep 22, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the movement of the fuscia ifdef here a divergence from upstream?

@kripken kripken Sep 22, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(otherwise lgtm)

@sbc100 sbc100 Sep 22, 2021

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, I hadn't realized I had effect fuchsia here. Will follow up.


#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
Expand Down
4 changes: 0 additions & 4 deletions system/lib/libc/musl/src/thread/pthread_key_create.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 10 additions & 0 deletions system/lib/pthread/emscripten_thread_state.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
.globaltype __tls_base, i32

.globaltype thread_id, i32
thread_id:

Expand All @@ -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:
Expand Down
8 changes: 0 additions & 8 deletions system/lib/pthread/emscripten_tls_init.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
}
13 changes: 13 additions & 0 deletions system/lib/pthread/pthread_create.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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);
Expand All @@ -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()) {
Expand Down
2 changes: 1 addition & 1 deletion tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ M
N
O
P
Q
v
w
x
y
Expand Down
1 change: 0 additions & 1 deletion tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.funcs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.imports
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,3 @@ a.r
a.s
a.t
a.u
a.v
1 change: 0 additions & 1 deletion tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,3 @@ r
s
t
u
v
2 changes: 1 addition & 1 deletion tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
16977
16962