Skip to content

Commit

Permalink
Avoid proxying atexit calls back to main thread. (#15905)
Browse files Browse the repository at this point in the history
To achieve this we manage the `atexit` functions in native code using
existing musl code.

This is small step towards a large change to just use musl for all
`atexit` handling: #14479.

The codesize implications of this change are a mixed bag.  In some
places we see saving but in other cases the extra export causes a small
regression (only when EXIT_RUNTIME=1).  In the long, once we land #14479
there should be more code size saving to be had by doing everything on
the native side.

Fixes #15868
  • Loading branch information
sbc100 authored Jan 7, 2022
1 parent a310f16 commit 250e9e1
Show file tree
Hide file tree
Showing 16 changed files with 100 additions and 30 deletions.
3 changes: 2 additions & 1 deletion embuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@
'struct_info',
'libstandalonewasm',
'crt1',
'libunwind-except'
'libunwind-except',
'libnoexit',
]

# Variant builds that we want to support for certain ports
Expand Down
9 changes: 7 additions & 2 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,8 +537,6 @@ def get_binaryen_passes():
passes += ['--memory64-lowering']
if run_binaryen_optimizer:
passes += ['--post-emscripten']
if not settings.EXIT_RUNTIME:
passes += ['--no-exit-runtime']
if run_binaryen_optimizer:
passes += [building.opt_level_to_str(settings.OPT_LEVEL, settings.SHRINK_LEVEL)]
# when optimizing, use the fact that low memory is never used (1024 is a
Expand Down Expand Up @@ -2336,6 +2334,13 @@ def check_memory_setting(setting):
# always does.
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$callRuntimeCallbacks']

if settings.EXIT_RUNTIME and not settings.STANDALONE_WASM:
# Internal function implemented in musl that calls any functions registered
# via `atexit` et al. With STANDALONE_WASM this is all taken care of via
# _start and exit handling in musl, but with the normal emscripten ABI we
# need to be able to call these explicitly.
settings.REQUIRED_EXPORTS += ['__funcs_on_exit']

# various settings require malloc/free support from JS
if settings.RELOCATABLE or \
settings.BUILD_AS_WORKER or \
Expand Down
15 changes: 0 additions & 15 deletions src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -361,21 +361,6 @@ LibraryManager.library = {
// stdlib.h
// ==========================================================================

#if MINIMAL_RUNTIME && !EXIT_RUNTIME
atexit__sig: 'v', // atexit unsupported in MINIMAL_RUNTIME
atexit: function(){},
__cxa_atexit: function(){},
#else
atexit__proxy: 'sync',
atexit__sig: 'iii',
atexit: function(func, arg) {
#if EXIT_RUNTIME
__ATEXIT__.unshift({ func: func, arg: arg });
#endif
},
__cxa_atexit: 'atexit',
#endif

// TODO: There are currently two abort() functions that get imported to asm
// module scope: the built-in runtime function abort(), and this library
// function _abort(). Remove one of these, importing two functions for the
Expand Down
3 changes: 3 additions & 0 deletions src/preamble.js
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,9 @@ function exitRuntime() {
if (ENVIRONMENT_IS_PTHREAD) return; // PThreads reuse the runtime from the main thread.
#endif
#if EXIT_RUNTIME
#if !STANDALONE_WASM
___funcs_on_exit(); // Native atexit() functions
#endif
callRuntimeCallbacks(__ATEXIT__);
<<< ATEXITS >>>
#endif
Expand Down
18 changes: 18 additions & 0 deletions system/lib/libc/atexit_dummy.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright 2022 The Emscripten Authors. All rights reserved.
* Emscripten is available under two separate licenses, the MIT license and the
* University of Illinois/NCSA Open Source License. Both these licenses can be
* found in the LICENSE file.
*/

// Stub implementations of atexit function. These will be included
// in favor of the regular ones in system/lib/libc/musl/src/exit/atexit.c
// when EXIT_RUNTIME == 0.

#include <stdlib.h>

int atexit(void (*function)(void)) { return 0; }

int __cxa_atexit(void (*func)(void *), void *arg, void *dso) { return 0; }

void __cxa_finalize(void *dso) { }
14 changes: 10 additions & 4 deletions system/lib/libc/musl/src/exit/atexit.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ void __funcs_on_exit()
}
}

void __cxa_finalize(void *dso)
void ___cxa_finalize(void *dso)
{
}

int __cxa_atexit(void (*func)(void *), void *arg, void *dso)
int ___cxa_atexit(void (*func)(void *), void *arg, void *dso)
{
LOCK(lock);

Expand Down Expand Up @@ -73,7 +73,13 @@ static void call(void *p)
((void (*)(void))(uintptr_t)p)();
}

int atexit(void (*func)(void))
int __atexit(void (*func)(void))
{
return __cxa_atexit(call, (void *)(uintptr_t)func, 0);
return ___cxa_atexit(call, (void *)(uintptr_t)func, 0);
}

// XXX: EMSCRIPTEN: Use weak aliases here so that we can override these symbols
// in when EXIT_RUNTIME is set to 0.
weak_alias(__atexit, atexit);
weak_alias(___cxa_atexit, __cxa_atexit);
weak_alias(___cxa_finalize, __cxa_finalize);
2 changes: 1 addition & 1 deletion tests/other/metadce/hello_libcxx_O2.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
98361
98236
1 change: 0 additions & 1 deletion tests/other/metadce/hello_libcxx_O2.sent
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
__cxa_atexit
abort
emscripten_memcpy_big
emscripten_resize_heap
Expand Down
2 changes: 1 addition & 1 deletion tests/other/metadce/hello_libcxx_O2_fexceptions.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
111833
111708
1 change: 0 additions & 1 deletion tests/other/metadce/hello_libcxx_O2_fexceptions.sent
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
__cxa_allocate_exception
__cxa_atexit
__cxa_begin_catch
__cxa_end_catch
__cxa_find_matching_catch_2
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
112820
112695
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
__cxa_allocate_exception
__cxa_atexit
__cxa_begin_catch
__cxa_end_catch
__cxa_find_matching_catch_2
Expand Down
37 changes: 37 additions & 0 deletions tests/pthread/test_pthread_busy_wait_atexit.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#include <stdatomic.h>
#include <stdbool.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <emscripten/console.h>

_Atomic bool done = false;

void exit_handler() {
printf("exit_handler\n");
}

void* thread_main(void*) {
// Avoid using printf here since stdio is proxied back to the
// main thread which is busy looping
_emscripten_out("in thread");
atexit(exit_handler);
done = true;
return NULL;
}

// Similar to test_pthread_busy_wait.cpp but with lower level pthreads
// API and explcit use of atexit before setting done to true.
// We also don't make any calls during the busy loop which means that
// proxied calls are *not* processed.
int main() {
printf("in main\n");
pthread_t t;
pthread_create(&t, NULL, thread_main, NULL);

while (!done) { }

pthread_join(t, NULL);
printf("done main\n");
return 0;
}
4 changes: 4 additions & 0 deletions tests/pthread/test_pthread_busy_wait_atexit.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
in main
in thread
done main
exit_handler
6 changes: 6 additions & 0 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -8488,6 +8488,12 @@ def test_pthread_busy_wait(self):
self.set_setting('EXIT_RUNTIME')
self.do_run_in_out_file_test('pthread/test_pthread_busy_wait.cpp')

@node_pthreads
def test_pthread_busy_wait_atexit(self):
self.set_setting('PTHREAD_POOL_SIZE', 1)
self.set_setting('EXIT_RUNTIME')
self.do_run_in_out_file_test('pthread/test_pthread_busy_wait_atexit.cpp')

@node_pthreads
def test_pthread_create_pool(self):
# with a pool, we can synchronously depend on workers being available
Expand Down
12 changes: 10 additions & 2 deletions tools/system_libs.py
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,12 @@ class libcompiler_rt(MTLibrary, SjLjLibrary):
])


class libnoexit(Library):
name = 'libnoexit'
src_dir = 'system/lib/libc'
src_files = ['atexit_dummy.c']


class libc(DebugLibrary, AsanInstrumentedLibrary, MuslInternalLibrary, MTLibrary):
name = 'libc'

Expand Down Expand Up @@ -918,7 +924,7 @@ def get_files(self):

libc_files += files_in_path(
path='system/lib/libc/musl/src/exit',
filenames=['_Exit.c'])
filenames=['_Exit.c', 'atexit.c'])

libc_files += files_in_path(
path='system/lib/libc/musl/src/ldso',
Expand Down Expand Up @@ -1562,7 +1568,7 @@ def get_files(self):
# including fprintf etc.
files += files_in_path(
path='system/lib/libc/musl/src/exit',
filenames=['assert.c', 'atexit.c', 'exit.c'])
filenames=['assert.c', 'exit.c'])
return files

def can_use(self):
Expand Down Expand Up @@ -1728,6 +1734,8 @@ def add_library(libname):

if settings.ALLOW_UNIMPLEMENTED_SYSCALLS:
add_library('libstubs')
if not settings.EXIT_RUNTIME:
add_library('libnoexit')
add_library('libc')
add_library('libcompiler_rt')
if settings.LINK_AS_CXX:
Expand Down

0 comments on commit 250e9e1

Please sign in to comment.