Use musl/native version of `atexit` #15905

sbc100 · 2022-01-07T00:24:26Z

This implementation uses shared memory to store the list of atexit functions
and avoids proxying atexit calls back to main thread.

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

kripken

Maybe the PR title could be "Move atexit() to C, and run them on each thread" or such?

emcc.py

sbc100 · 2022-01-07T16:29:08Z

Maybe the PR title could be "Move atexit() to C, and run them on each thread" or such?

But the atexit function still only run on the main thread. I will update the title though.

emcc.py

tests/other/metadce/hello_libcxx_O2.size

tests/pthread/test_pthread_busy_wait_atexit.cpp

kripken

Ah, good idea to link in stubs for atexit. Yes, I think that's the right approach.

lgtm, but also please delete these 2 lines:

emscripten/emcc.py

Lines 540 to 541 in 8988358

    
           if not settings.EXIT_RUNTIME: 
        
             passes += ['--no-exit-runtime']

That pass removes calls to atexit as JS imports. Now that we have no more JS imports for them it does not do anything. We can remove it from Binaryen after this lands actually.

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

Since emscripten-core/emscripten#15905 landed emscripten now includes its own dummy atexit function when building with EXIT_RUNTIME=0. This dummy function conflicts with the emscripten-provided one: ``` wasm-ld: error: duplicate symbol: atexit >>> defined in CMakeFiles/binaryen_wasm.dir/src/binaryen-c.cpp.o >>> defined in ...wasm32-emscripten/lto/libnoexit.a(atexit_dummy.o) ``` Normally overriding symbols from libc does not causes issues but one needs to be sure to override all the symbols in a given object file so that the object in question (atexit_dummy.o) does not get linked in. In this case some other symbol being defined in in atexit_dummy.o (e.g. __cxa_atexit) is likely the cause of the conflict. Overriding symbols from libc is likely to break in this way as the libc evolves, and since emscripten is now providing a dummy, just as we want, its better/safer to simply remove our dummy.

After emscripten-core/emscripten#15905 lands Emscripten will no longer use it, and nothing else needs it AFAIK.

Runtime callbacks can only be functions, not numbers. In the past we used to handle C atexit using this mechansim which is why this used to be needed but that changd in #15905.

Followup to #17303. The last usage of the func/arg type of callback was removed in #15905 and I don't think we ever advertised this as part of the API.

Runtime callbacks can only be functions, not numbers. In the past we used to handle C atexit using this mechansim which is why this used to be needed but that changd in #15905.

Followup to #17303. The last usage of the func/arg type of callback was removed in #15905 and I don't think we ever advertised this as part of the API.

sbc100 requested a review from kripken January 7, 2022 00:58

sbc100 force-pushed the atexit_no_proxy branch 3 times, most recently from e89185d to ee67165 Compare January 7, 2022 02:14

bakkot mentioned this pull request Jan 7, 2022

wasm build: satisfiable problem produces unknown, Z3_solver_get_reason_unknown returns empty string Z3Prover/z3#5746

Closed

sbc100 force-pushed the atexit_no_proxy branch 2 times, most recently from 7215d83 to 76e5833 Compare January 7, 2022 11:49

sbc100 mentioned this pull request Jan 7, 2022

thread can't spawn synchronously even with PTHREAD_POOL_SIZE=4 #15868

Closed

kripken reviewed Jan 7, 2022

View reviewed changes

emcc.py Show resolved Hide resolved

sbc100 changed the title ~~Avoid proxying atexit calls back to main thread.~~ Use musl/native version of atexit Jan 7, 2022

sbc100 requested a review from kripken January 7, 2022 16:30

kripken reviewed Jan 7, 2022

View reviewed changes

emcc.py Show resolved Hide resolved

tests/other/metadce/hello_libcxx_O2.size Outdated Show resolved Hide resolved

tests/pthread/test_pthread_busy_wait_atexit.cpp Outdated Show resolved Hide resolved

sbc100 force-pushed the atexit_no_proxy branch 5 times, most recently from 7e6b02c to 129fe7e Compare January 7, 2022 19:01

kripken approved these changes Jan 7, 2022

View reviewed changes

sbc100 force-pushed the atexit_no_proxy branch from 129fe7e to 217252a Compare January 7, 2022 19:43

kripken mentioned this pull request Jan 7, 2022

Remove NoExitRuntime pass WebAssembly/binaryen#4431

Merged

sbc100 enabled auto-merge (squash) January 7, 2022 20:18

sbc100 merged commit 250e9e1 into main Jan 7, 2022

sbc100 deleted the atexit_no_proxy branch January 7, 2022 21:58

sbc100 mentioned this pull request Jan 8, 2022

Fix emscripten build by removing dummy atexit function WebAssembly/binaryen#4435

Merged

kripken added a commit to WebAssembly/binaryen that referenced this pull request Jan 26, 2022

Remove NoExitRuntime pass (#4431)

97bb0bd

After emscripten-core/emscripten#15905 lands Emscripten will no longer use it, and nothing else needs it AFAIK.

duxovni mentioned this pull request May 17, 2022

binaryen: 102 -> 105; emscripten: 3.0.0 -> 3.1.10 NixOS/nixpkgs#172741

Merged

13 tasks

sbc100 mentioned this pull request Jun 23, 2022

Remove dead code from callRuntimeCallbacks. NFC #17303

Merged

sbc100 added a commit that referenced this pull request Jun 23, 2022

More dead code removal from callRuntimeCallbacks. NFC.

fee9da2

Followup to #17303. The last usage of the func/arg type of callback was removed in #15905 and I don't think we ever advertised this as part of the API.

sbc100 mentioned this pull request Jun 23, 2022

More dead code removal from callRuntimeCallbacks. NFC. #17304

Merged

sbc100 added a commit that referenced this pull request Jun 23, 2022

More dead code removal from callRuntimeCallbacks. NFC.

c91b7f2

Followup to #17303. The last usage of the func/arg type of callback was removed in #15905 and I don't think we ever advertised this as part of the API.

sbc100 added a commit that referenced this pull request Jun 23, 2022

More dead code removal from callRuntimeCallbacks. NFC. (#17304)

e98bac3

Followup to #17303. The last usage of the func/arg type of callback was removed in #15905 and I don't think we ever advertised this as part of the API.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use musl/native version of `atexit` #15905

Use musl/native version of `atexit` #15905

sbc100 commented Jan 7, 2022 •

edited

kripken left a comment

sbc100 commented Jan 7, 2022

kripken left a comment

	if not settings.EXIT_RUNTIME:
	passes += ['--no-exit-runtime']

Use musl/native version of atexit #15905

Use musl/native version of atexit #15905

Conversation

sbc100 commented Jan 7, 2022 • edited

kripken left a comment

Choose a reason for hiding this comment

sbc100 commented Jan 7, 2022

kripken left a comment

Choose a reason for hiding this comment

Use musl/native version of `atexit` #15905

Use musl/native version of `atexit` #15905

sbc100 commented Jan 7, 2022 •

edited