From 250e9e1dc712fb0b970f68bab54726656c2670e4 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Fri, 7 Jan 2022 13:58:01 -0800 Subject: [PATCH] Avoid proxying atexit calls back to main thread. (#15905) 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 --- embuilder.py | 3 +- emcc.py | 9 ++++- src/library.js | 15 -------- src/preamble.js | 3 ++ system/lib/libc/atexit_dummy.c | 18 +++++++++ system/lib/libc/musl/src/exit/atexit.c | 14 +++++-- tests/other/metadce/hello_libcxx_O2.jssize | 2 +- tests/other/metadce/hello_libcxx_O2.sent | 1 - .../hello_libcxx_O2_fexceptions.jssize | 2 +- .../metadce/hello_libcxx_O2_fexceptions.sent | 1 - ...cxx_O2_fexceptions_DEMANGLE_SUPPORT.jssize | 2 +- ...ibcxx_O2_fexceptions_DEMANGLE_SUPPORT.sent | 1 - .../pthread/test_pthread_busy_wait_atexit.cpp | 37 +++++++++++++++++++ .../pthread/test_pthread_busy_wait_atexit.out | 4 ++ tests/test_core.py | 6 +++ tools/system_libs.py | 12 +++++- 16 files changed, 100 insertions(+), 30 deletions(-) create mode 100644 system/lib/libc/atexit_dummy.c create mode 100644 tests/pthread/test_pthread_busy_wait_atexit.cpp create mode 100644 tests/pthread/test_pthread_busy_wait_atexit.out diff --git a/embuilder.py b/embuilder.py index 98de72397cba..14762767cfef 100755 --- a/embuilder.py +++ b/embuilder.py @@ -53,7 +53,8 @@ 'struct_info', 'libstandalonewasm', 'crt1', - 'libunwind-except' + 'libunwind-except', + 'libnoexit', ] # Variant builds that we want to support for certain ports diff --git a/emcc.py b/emcc.py index bfbf47c55172..a92a10e27964 100755 --- a/emcc.py +++ b/emcc.py @@ -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 @@ -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 \ diff --git a/src/library.js b/src/library.js index 6a1cccbfbeea..0816feaa4c83 100644 --- a/src/library.js +++ b/src/library.js @@ -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 diff --git a/src/preamble.js b/src/preamble.js index 5ba9b3967c38..dcdcc5335cce 100644 --- a/src/preamble.js +++ b/src/preamble.js @@ -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 diff --git a/system/lib/libc/atexit_dummy.c b/system/lib/libc/atexit_dummy.c new file mode 100644 index 000000000000..f7969fe72bb6 --- /dev/null +++ b/system/lib/libc/atexit_dummy.c @@ -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 + +int atexit(void (*function)(void)) { return 0; } + +int __cxa_atexit(void (*func)(void *), void *arg, void *dso) { return 0; } + +void __cxa_finalize(void *dso) { } diff --git a/system/lib/libc/musl/src/exit/atexit.c b/system/lib/libc/musl/src/exit/atexit.c index 854e9fddbe55..f2401780b30e 100644 --- a/system/lib/libc/musl/src/exit/atexit.c +++ b/system/lib/libc/musl/src/exit/atexit.c @@ -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); @@ -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); diff --git a/tests/other/metadce/hello_libcxx_O2.jssize b/tests/other/metadce/hello_libcxx_O2.jssize index a71fabff3923..a1de4ec547ad 100644 --- a/tests/other/metadce/hello_libcxx_O2.jssize +++ b/tests/other/metadce/hello_libcxx_O2.jssize @@ -1 +1 @@ -98361 +98236 diff --git a/tests/other/metadce/hello_libcxx_O2.sent b/tests/other/metadce/hello_libcxx_O2.sent index 943c0ca713dc..25e3f929f2fb 100644 --- a/tests/other/metadce/hello_libcxx_O2.sent +++ b/tests/other/metadce/hello_libcxx_O2.sent @@ -1,4 +1,3 @@ -__cxa_atexit abort emscripten_memcpy_big emscripten_resize_heap diff --git a/tests/other/metadce/hello_libcxx_O2_fexceptions.jssize b/tests/other/metadce/hello_libcxx_O2_fexceptions.jssize index 6d7849572b30..f02cad118882 100644 --- a/tests/other/metadce/hello_libcxx_O2_fexceptions.jssize +++ b/tests/other/metadce/hello_libcxx_O2_fexceptions.jssize @@ -1 +1 @@ -111833 +111708 diff --git a/tests/other/metadce/hello_libcxx_O2_fexceptions.sent b/tests/other/metadce/hello_libcxx_O2_fexceptions.sent index 21c54d195a50..354d1ea99aa9 100644 --- a/tests/other/metadce/hello_libcxx_O2_fexceptions.sent +++ b/tests/other/metadce/hello_libcxx_O2_fexceptions.sent @@ -1,5 +1,4 @@ __cxa_allocate_exception -__cxa_atexit __cxa_begin_catch __cxa_end_catch __cxa_find_matching_catch_2 diff --git a/tests/other/metadce/hello_libcxx_O2_fexceptions_DEMANGLE_SUPPORT.jssize b/tests/other/metadce/hello_libcxx_O2_fexceptions_DEMANGLE_SUPPORT.jssize index 6ae4b778e866..83b6b5bb3fa1 100644 --- a/tests/other/metadce/hello_libcxx_O2_fexceptions_DEMANGLE_SUPPORT.jssize +++ b/tests/other/metadce/hello_libcxx_O2_fexceptions_DEMANGLE_SUPPORT.jssize @@ -1 +1 @@ -112820 +112695 diff --git a/tests/other/metadce/hello_libcxx_O2_fexceptions_DEMANGLE_SUPPORT.sent b/tests/other/metadce/hello_libcxx_O2_fexceptions_DEMANGLE_SUPPORT.sent index 21c54d195a50..354d1ea99aa9 100644 --- a/tests/other/metadce/hello_libcxx_O2_fexceptions_DEMANGLE_SUPPORT.sent +++ b/tests/other/metadce/hello_libcxx_O2_fexceptions_DEMANGLE_SUPPORT.sent @@ -1,5 +1,4 @@ __cxa_allocate_exception -__cxa_atexit __cxa_begin_catch __cxa_end_catch __cxa_find_matching_catch_2 diff --git a/tests/pthread/test_pthread_busy_wait_atexit.cpp b/tests/pthread/test_pthread_busy_wait_atexit.cpp new file mode 100644 index 000000000000..9969940b27f8 --- /dev/null +++ b/tests/pthread/test_pthread_busy_wait_atexit.cpp @@ -0,0 +1,37 @@ +#include +#include +#include +#include +#include +#include + +_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; +} diff --git a/tests/pthread/test_pthread_busy_wait_atexit.out b/tests/pthread/test_pthread_busy_wait_atexit.out new file mode 100644 index 000000000000..0186ee703018 --- /dev/null +++ b/tests/pthread/test_pthread_busy_wait_atexit.out @@ -0,0 +1,4 @@ +in main +in thread +done main +exit_handler diff --git a/tests/test_core.py b/tests/test_core.py index 514b8f926920..c79abc5cefe7 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -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 diff --git a/tools/system_libs.py b/tools/system_libs.py index 9cd48739a222..47cad9824b7a 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -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' @@ -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', @@ -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): @@ -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: