Move parts of emscripten exception handling to native code. NFC by sbc100 · Pull Request #16627 · emscripten-core/emscripten

sbc100 · 2022-03-29T19:38:35Z

Specifically, this change moves the allocation and reference counting
functions.

This saves on code size but more importantly reduces the number and of
complexity of imports/exports, which in turn helps with the wasm64 work
I've been doing.

tests/test_core.py

system/lib/libcxxabi/src/cxa_noexception.cpp

system/lib/libcxxabi/src/cxa_emscripten.cpp

system/lib/libcxxabi/src/cxa_exception_emscripten.cpp

kripken · 2022-04-01T17:05:58Z

Diffing the cpp file to the file it is adapted from, I see stuff like this:

 //  Allocate a __cxa_exception object, and zero-fill it.
 //  Reserve "thrown_size" bytes on the end for the user's exception
 //  object. Zero-fill the object. If memory can't be allocated, call
 //  std::terminate. Return a pointer to the memory to be used for the
 //  user's exception object.
 void *__cxa_allocate_exception(size_t thrown_size) _NOEXCEPT {
     size_t actual_size = cxa_exception_size_from_exception_thrown_size(thrown_size);
 
-    // Allocate extra space before the __cxa_exception header to ensure the
-    // start of the thrown object is sufficiently aligned.
-    size_t header_offset = get_cxa_exception_offset();
     char *raw_buffer =
-        (char *)__aligned_malloc_with_fallback(header_offset + actual_size);
+        (char *)__aligned_malloc_with_fallback(actual_size);
     if (NULL == raw_buffer)
         std::terminate();
     __cxa_exception *exception_header =
-        static_cast<__cxa_exception *>((void *)(raw_buffer + header_offset));
+        static_cast<__cxa_exception *>((void *)(raw_buffer));
     ::memset(exception_header, 0, actual_size);
     return thrown_object_from_cxa_exception(exception_header);
 }

Why do we not have the header_offset? Documenting such changes would be good.

The other part of the diff that looks unclear to me is this:

 void __cxa_decrement_exception_refcount(void *thrown_object) _NOEXCEPT {
     if (thrown_object != NULL )
     {
         __cxa_exception* exception_header = cxa_exception_from_thrown_object(thrown_object);
-        if (std::__libcpp_atomic_add(&exception_header->referenceCount, size_t(-1)) == 0)
+        DEBUG("DEC: %p refcnt=%zu rethrown=%d\n", thrown_object, exception_header->referenceCount, exception_header->rethrown);
+        assert(exception_header->referenceCount > 0);
+        if (std::__libcpp_atomic_add(&exception_header->referenceCount, size_t(-1)) == 0 && !exception_header->rethrown)
         {
+            DEBUG("DEL: %p\n", thrown_object);
             if (NULL != exception_header->exceptionDestructor)
                 exception_header->exceptionDestructor(thrown_object);
             __cxa_free_exception(thrown_object);
         }
     }
 }

We add && !exception_header->rethrown but I'm not sure why?

sbc100 · 2022-04-15T00:04:53Z

Yes, those deltas are deliberate, and made to match the JS implementation. Honestly I'm not totally sure why they exist, but they match the existing JS code.

sbc100 · 2022-04-15T00:05:00Z

Added more comments

kripken

Ok, sgtm to match the old JS behavior even if we don't fully understand it.

@sbc100

I think we need a separate file in libc++abi for 'our' EH, meaning both Emscripten EH and Wasm EH. (I think they can be in the same file, as long as we have `ifdef`s) This also adds a file header. I think this method can be in that file, rather than having a separate file for this. I named this `cxa_emscripten.cpp` because it's the name emscripten-core#16627 used and I hope we can put all Emscripten specific stuff there, but maybe @sbc100 had another thought; if so let me know.

@sbc100

I think we need a separate file in libc++abi for 'our' EH, meaning both Emscripten EH and Wasm EH. (I think they can be in the same file, as long as we have `ifdef`s) This also adds a file header. I think this method can be in that file, rather than having a separate file for this. I named this `cxa_emscripten.cpp` because it's the name emscripten-core#16627 used and I hope we can put all Emscripten specific stuff there, but maybe @sbc100 had another thought; if so let me know.

RReverser · 2023-03-27T16:47:51Z

Oh I started working on something similar but then decided to search PRs just in case, and glad I did. There seems to be a lot of conflicts - mainly in tests - but otherwise seems like this was in mergeable state?

sbc100 · 2023-03-27T16:53:10Z

I landed a smaller version of this patch in #18292. I'm not sure its worth revisiting this larger change. Would be good to have but not sure if its high enough priority given that it works, and we have native exception handling coming down the pipe soon.

RReverser · 2023-03-27T16:59:54Z

I'm not sure its worth revisiting this larger change. Would be good to have but not sure if its high enough priority given that it works, and we have native exception handling coming down the pipe soon.

Unfortunately, one doesn't contradict the other. I still need to work on something like this for project I'm working on at the moment, because neither Wasmtime nor Wasmer support Wasm exceptions yet, so the only chance for STANDALONE_WASM is manually shimming out Emscripten exception support in the custom runtime we have.

Moving parts of the Emscripten EH that just modify C++ exception structures from JS to C++ would make this much easier, as in the end I'd only need to shim out invoke_* and __cxa_throw (and maybe few other small things) rather than the whole API.

RReverser · 2023-03-27T17:00:38Z

I guess, looking at the number of conflicts, I should just try & start anew after all... (unless you think this PR is still salvageable)

RReverser · 2023-03-27T17:02:59Z

I'll try to rebase it first after all.

sbc100 · 2023-03-27T17:05:08Z

Sounds good. I would still love to see this land if you can make it happen.

system/lib/libcxxabi/src/cxa_exception_emscripten.cpp

sbc100 · 2023-03-27T21:34:31Z

system/lib/libcxxabi/src/cxa_exception_emscripten.cpp

+static
+inline
+__cxa_exception*
+cxa_exception_from_thrown_object(void* thrown_object)


These utilities seem to be duplicated here and in the new utils file.

Yeah, just like in cxa_exception they were originally borrowed from, they're static local helpers everywhere.

test/other/metadce/test_metadce_cxx_mangle.exports

RReverser · 2023-03-27T21:41:53Z

@sbc100 FWIW this is WIP, I just wanted to push as I got far enough in my conflict resolution & new changes that I didn't want to lose.

RReverser · 2023-03-27T21:46:23Z

I'll continue working on this tomorrow. For now I managed to rebase most of this stuff, although had to deal with new helpers exposed to JS that were added since this PR.

Since they're shared between backends, I've moved them to a new .cpp file together with (yet another) copy of used static function helpers.

Ideal end state I think would be to reuse cxa_exception.cpp between both backends with some ifdef sprinkled to exclude bits implemented in JS, but that would be out of scope of this particular PR - for now just trying to get it to do what it did originally.

RReverser · 2023-03-27T21:47:34Z

emcc.py

+      # case because that only works for user-level code, and __cxa_begin_catch
+      # can be used by the standard library.
+      '__cxa_increment_exception_refcount',
+      # Same for __cxa_end_catch


Also had to add __cxa_decrement_exception_refcount here to make some tests pass; I'm not sure why it wasn't necessary for the original PR but became now - maybe I'm missing a better place where it should be added, or maybe something changed in tests.

RReverser · 2023-03-28T12:55:09Z

One more thing I noticed is that this part

emscripten/emcc.py

Lines 2785 to 2806 in b5c68f6

    
           if not settings.DISABLE_EXCEPTION_CATCHING: 
        
             settings.REQUIRED_EXPORTS += [ 
        
               # For normal builds the entries in deps_info.py are enough to include 
        
               # these symbols whenever __cxa_find_matching_catch_* functions are 
        
               # found.  However, under LTO these symbols don't exist prior to linking 
        
               # so we include then unconditionally when exceptions are enabled. 
        
               '__cxa_is_pointer_type', 
        
               '__cxa_can_catch', 
        
               # __cxa_begin_catch depends on this but we can't use deps info in this 
        
               # case because that only works for user-level code, and __cxa_begin_catch 
        
               # can be used by the standard library. 
        
               '__cxa_increment_exception_refcount', 
        
               # Same for __cxa_end_catch 
        
               '__cxa_decrement_exception_refcount', 
        
               # Emscripten exception handling can generate invoke calls, and they call 
        
               # setThrew(). We cannot handle this using deps_info as the invokes are not 
        
               # emitted because of library function usage, but by codegen itself. 
        
               'setThrew', 
        
               '__cxa_free_exception', 
        
             ]

seems to overlap in terms of intent with this one

emscripten/emcc.py

Lines 2897 to 2911 in b5c68f6

    
           # Make `getExceptionMessage` and other necessary functions available for use. 
        
           if settings.EXPORT_EXCEPTION_HANDLING_HELPERS: 
        
             # If the user explicitly gave EXPORT_EXCEPTION_HANDLING_HELPERS=1 without 
        
             # enabling EH, errors out. 
        
             if settings.DISABLE_EXCEPTION_CATCHING and not settings.WASM_EXCEPTIONS: 
        
               exit_with_error('EXPORT_EXCEPTION_HANDLING_HELPERS requires either of -fexceptions or -fwasm-exceptions') 
        
             # We also export refcount increasing and decreasing functions because if you 
        
             # catch an exception, be it an Emscripten exception or a Wasm exception, in 
        
             # JS, you may need to manipulate the refcount manually not to leak memory. 
        
             # What you need to do is different depending on the kind of EH you use 
        
             # (https://github.com/emscripten-core/emscripten/issues/17115). 
        
             settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$getExceptionMessage', '$incrementExceptionRefcount', '$decrementExceptionRefcount'] 
        
             settings.EXPORTED_FUNCTIONS += ['getExceptionMessage', '___get_exception_message', '_free'] 
        
             if settings.WASM_EXCEPTIONS: 
        
               settings.EXPORTED_FUNCTIONS += ['___cpp_exception', '___cxa_increment_exception_refcount', '___cxa_decrement_exception_refcount', '___thrown_object_from_unwind_exception']

Should they be merged somehow or are they responsible for different things? (one is under if settings.EXPORT_EXCEPTION_HANDLING_HELPERS and another says REQUIRED_EXPORTS, but not sure if the difference is intentional)

RReverser · 2023-04-12T17:47:11Z

From a quick look, the signature seems the same as before despite moving func from JS to C++. I'm curious why it complains...

sbc100 · 2023-04-12T17:48:18Z

Its particularly interesting that it only happens for LTO..

sbc100 · 2023-04-12T17:54:35Z

I think I have a fix.

RReverser · 2023-04-12T17:59:47Z

Could it be related to _LIBCXXABI_NO_CFI?

This test started failing after #16627. Prior to that change the exceptions destructors were called via the following code in JS: ``` // In Wasm, destructors return 'this' as in ARM {{{ makeDynCall('pp', 'destructor') }}}(info.excPtr); ``` I'm not sure why this test only started failing under LTO. Its seems like it should be failing under non-LTO too.

This test started failing after #16627. Prior to that change the exceptions destructors were called via the following code in JS: ``` // In Wasm, destructors return 'this' as in ARM {{{ makeDynCall('pp', 'destructor') }}}(info.excPtr); ``` For some reason the LTO build produces the following for for the call to `exception_header->exceptionDestructor(thrown_object)` in `__cxa_decrement_exception_refcount`: ``` call_indirect 0 (type 0) ``` Where as the normal non-LTO build produces: ``` call 13 <invoke_ii> ``` Because invoke_ii goes via JS it uses the sloppy type checking and doesn't trap, but `call_indirect` has strict type checking and so does trap.

This was recently introduced in #16627 but when to a different deps system in #18905 I noticed it is not necessary.

dschuff · 2023-04-14T17:04:22Z

This is still causing a failure in the LLVM testsuite: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8783849429524138417/+/u/LLVM_testsuite/stdout

wasm-ld: error: lib/libgtest.a(gtest-all.cc.o): undefined symbol: __cxa_free_exception
em++: error: '/b/s/w/ir/x/w/install/bin/wasm-ld -o MicroBenchmarks/libs/benchmark/test/perf_counters_gtest.wasm -L/b/s/w/ir/cache/builder/emscripten-releases/build/llvmtest-out MicroBenchmarks/libs/benchmark/test/CMakeFiles/perf_counters_gtest.dir/perf_counters_gtest.cc.o MicroBenchmarks/libs/benchmark/src/libbenchmark.a lib/libgmock_main.a lib/libgmock.a lib/libgtest.a -L/b/s/w/ir/x/w/install/emscripten/cache/sysroot/lib/wasm32-emscripten -lGL -lal -lhtml5 -lstubs -lc -ldlmalloc -lcompiler_rt -lc++-noexcept -lc++abi-noexcept -lsockets -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr --allow-undefined-file=/b/s/w/ir/x/t/tmp2wer4hv6.undefined --strip-debug --export-if-defined=main --export-if-defined=__start_em_asm --export-if-defined=__stop_em_asm --export-if-defined=__start_em_lib_deps --export-if-defined=__stop_em_lib_deps --export-if-defined=__start_em_js --export-if-defined=__stop_em_js --export-if-defined=__main_argc_argv --export-if-defined=fflush --export=stackSave --export=stackRestore --export=stackAlloc --export=__errno_location --export=__get_temp_ret --export=__set_temp_ret --export=__funcs_on_exit --export=__wasm_call_ctors --export=__cxa_is_pointer_type --export=malloc --export=__cxa_increment_exception_refcount --export=setThrew --export=__cxa_decrement_exception_refcount --export=__cxa_can_catch --export=setTempRet0 --export-table -z stack-size=131072 --initial-memory=1073741824 --no-entry --max-memory=1073741824 --global-base=1024' failed (returned 1)

dschuff · 2023-04-14T17:05:51Z

I have it reproing locally too, let me know if you want more detail or the files.

sbc100 · 2023-04-14T17:13:41Z

I'm guessing that test is building with the default settings? (i.e. DISABLE_EXCEPTION_CATCHING=1 + DISABLE_EXCEPTION_THROWING=0).. hense the linking with -lc++-noexcept -lc++abi-noexcept. Are you sure it was this change that caused the failure to start?

sbc100 · 2023-04-14T17:15:36Z

I'm guessing the change to system/lib/libcxxabi/src/cxa_noexception.cpp that was part of this change needs to have __cxa_free_exception as well as the __cxa_allocate_exception

sbc100 · 2023-04-14T17:19:21Z

Yup that looks like it the problem.. the sad thing is I have no idea why non of our core tests required that. Any idea what gtest-all.cc.o is doing to depend on that symbol? (so we can add a test?)

dschuff · 2023-04-14T18:08:22Z

According to https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html#cxx-destroy it seems like __cxa_end_catch should usually call __cxa_free_exception (and indeed this seems to be the case in cxa_exception.cpp where it's called via cxa_decrement_exception_refcount and this is mirrored in cxa_exception_emscripten.cpp. So it seems that basically anything with a catch block should depend on it?

dschuff · 2023-04-14T18:11:05Z

Oh wait, I see what you're saying. If catching is disabled, then why are we still getting this dependency? Yeah I don't quite get it yet either.

dschuff · 2023-04-14T18:23:05Z

I disassembled gtest-all.cc.o and found that __cxa_free_exception is actually being called directly by testing::UnitTest::AddTestPartResult(testing::TestPartResult::Type, char const*, int, std::__2::basic_string<char, std::__2::char_traits<char>, std::__2::allocator<char> > const&, std::__2::basic_string<char, std::__2::char_traits<char>, std::__2::allocator<char> > const&) and not by decrement_exception_refcount. The source is https://chromium.googlesource.com/native_client/pnacl-llvm-testsuite/+/edc9be64895f43c79aa34fad32801ada216ce229/MicroBenchmarks/libs/benchmark/googletest/googletest/src/gtest.cc#5351 but there's nothing obvious to me just by looking.

sbc100 · 2023-04-14T18:25:48Z

In emscripten we default to throwing being enabled but catching being disabled.. which is a little unusual.

sbc100 · 2023-04-14T18:27:57Z

I don't see __cxa_free_exception in the source code, so I assume you don't mean they the are actually calling it directly from their source, but that clang is generating a call to it?

sbc100 · 2023-04-14T18:32:46Z

I looks like it must be generated here: https://github.com/llvm/llvm-project/blob/64ae7669a7cefa12ea1117680cf9bfb5c3f6084c/clang/lib/CodeGen/CGException.cpp#L380-L388

Maybe we can add a test that simply calls it directly? Or figure out how to trigger the above code indirectly in C++?

dschuff · 2023-04-14T22:10:08Z

Yeah, if a generated call is possible, then a test that calls it directly probably makes sense.

When we enable (Wasm / Emscripten) EH and don't use it, we still pulled in a lot of JS library functions + libc++abi library functions that were not DCE'ed. The reason was we mandatorily exported many functions whenever some EH options were enabled, regardless of they were used or not. 1. We used to enable `EXPORT_EXCEPTION_HANDLING_HELPERS`, which exports `getExceptionMessage` and `in/decrementExceptionRefcount`, whenever `EXCEPTION_STACK_TRACES` was true. And `EXCEPTION_STACK_TRACES` is true whenever `ASSERTIONS` is true, which is the default at `-O0`. And those exported functions can pull in many libc++abi functions. As a result, at `-O0`, we pull in a lot of functions even when we are not using any exceptions. This PR removes that `EXCEPTION_STACK_TRACES` -> `EXPORT_EXCEPTION_HANDLING_HELPERS` link. Without this link, we can still use see stack traces with an exception message with no problem, because `__cxa_throw` -> `__throw_exception_with_stack_trace` -> `getExceptionMessage` dependencies: https://github.com/emscripten-core/emscripten/blob/6ad2f5e03021a39377428e9d476985fc967014d4/system/lib/libcxxabi/src/cxa_exception.cpp#L302-L304 https://github.com/emscripten-core/emscripten/blob/6ad2f5e03021a39377428e9d476985fc967014d4/src/lib/libexceptions.js#L311 2. If we do 1, Emscripten EH's `getExceptionMessage` does not work, because unlike Wasm EH, its `getExceptionMessage` dependency is within `runtime_exceptions.js`, where we can't attach `__deps`, so we can't track it: https://github.com/emscripten-core/emscripten/blob/6ad2f5e03021a39377428e9d476985fc967014d4/src/runtime_exceptions.js#L20 So, this adds `getExceptionMessage` as a dependency of `libexception.js`'s `__cxa_throw` directly, when `EXCEPTION_STACK_TRACES` is enabled. 3. This removes several functions from `REQUIRED_EXPORTS` when Emscripten EH is used. Previously, the comment said, in LTO mode, `_cxa_find_matching_catch_*` -> `__cxa_can_catch` dependency is was not tracked. But now, in Emscripten EH, we preemptively add several `__cxa_find_matching_catch_n`s, and it depends on `findMatchingCatch`, which depends on `__cxa_end_catch`: https://github.com/emscripten-core/emscripten/blob/78403050f355085104175499224ad0f6bccb5fb1/src/lib/libexceptions.js#L371-L405 https://github.com/emscripten-core/emscripten/blob/78403050f355085104175499224ad0f6bccb5fb1/src/lib/libexceptions.js#L217 So we don't need to require exporting `__cxa_end_catch` anymore. Also, other required exports (`__cxa_in/decrement_exception_count` and `__cxa_free_exceptions`) are dependencies that can be naturally referred within libc++abi. I think these were added here in emscripten-core#16627 when we were using `deps_info.py`, which we don't use anymore. After this commit, when you compile an empty `int main { return 0; }` with `-O0` and `-fexceptions`/`-fwasm-exceptions`, the size decreases to: - `-fexceptions`: 113212 -> 1168 - `-fwasm-exceptions`: 109177 -> 1106

When we enabled (Wasm / Emscripten) EH and don't use it, we still pulled in a lot of JS library functions + libc++abi library functions that were not DCE'ed. The reason was we mandatorily exported many functions whenever some EH options were enabled, regardless of whether they were used or not. 1. We used to enable `EXPORT_EXCEPTION_HANDLING_HELPERS`, which exports `getExceptionMessage` and `in/decrementExceptionRefcount`, whenever `EXCEPTION_STACK_TRACES` was true. And `EXCEPTION_STACK_TRACES` is true whenever `ASSERTIONS` is true, which is the default at `-O0`. And those exported functions can pull in many libc++abi functions. As a result, at `-O0`, we pulled in a lot of functions even when we were not using any exceptions. This PR removes that `EXCEPTION_STACK_TRACES` -> `EXPORT_EXCEPTION_HANDLING_HELPERS` link. Without this link, we can still see stack traces with exception messages with no problem, because `__cxa_throw` -> `__throw_exception_with_stack_trace` -> `getExceptionMessage` dependencies: https://github.com/emscripten-core/emscripten/blob/6ad2f5e03021a39377428e9d476985fc967014d4/system/lib/libcxxabi/src/cxa_exception.cpp#L302-L304 https://github.com/emscripten-core/emscripten/blob/6ad2f5e03021a39377428e9d476985fc967014d4/src/lib/libexceptions.js#L311 2. If we do 1, Emscripten EH's `getExceptionMessage` does not work, because unlike Wasm EH, its `getExceptionMessage` dependency is within `runtime_exceptions.js`, where we can't attach `__deps`, so we can't track it: https://github.com/emscripten-core/emscripten/blob/6ad2f5e03021a39377428e9d476985fc967014d4/src/runtime_exceptions.js#L20 So, this adds `getExceptionMessage` as a dependency of `libexception.js`'s `__cxa_throw` directly, when `EXCEPTION_STACK_TRACES` is enabled. 3. This removes several functions from `REQUIRED_EXPORTS` when Emscripten EH is used. Previously, the comment said, in LTO mode, `_cxa_find_matching_catch_*` -> `__cxa_can_catch` dependency was not tracked. But now, in Emscripten EH, we preemptively add several `__cxa_find_matching_catch_n`s, and it depends on `findMatchingCatch`, which depends on `__cxa_end_catch`: https://github.com/emscripten-core/emscripten/blob/78403050f355085104175499224ad0f6bccb5fb1/src/lib/libexceptions.js#L371-L405 https://github.com/emscripten-core/emscripten/blob/78403050f355085104175499224ad0f6bccb5fb1/src/lib/libexceptions.js#L217 So we don't need to require exporting `__cxa_end_catch` anymore. Also, other required exports (`__cxa_in/decrement_exception_count` and `__cxa_free_exceptions`) are dependencies that can be naturally referred within libc++abi. I think these were added here in emscripten-core#16627 when we were using `deps_info.py`, which we don't use anymore. After this commit, when you compile an empty `int main { return 0; }` with `-O0` and `-fexceptions`/`-fwasm-exceptions`, the size decreases to: - `-fexceptions`: 113212 -> 1168 - `-fwasm-exceptions`: 109177 -> 1106

When we enabled (Wasm / Emscripten) EH and didn't use it, we still pulled in a lot of JS library functions + libc++abi library functions that were not DCE'ed. The reason was we mandatorily exported many functions whenever some EH options were enabled, regardless of whether they were used or not. 1. We used to enable `EXPORT_EXCEPTION_HANDLING_HELPERS`, which exports `getExceptionMessage` and `in/decrementExceptionRefcount`, whenever `EXCEPTION_STACK_TRACES` was true. And `EXCEPTION_STACK_TRACES` is true whenever `ASSERTIONS` is true, which is the default at `-O0`. And those exported functions can pull in many libc++abi functions. As a result, at `-O0`, we pulled in a lot of functions even when we were not using any exceptions. This PR removes that `EXCEPTION_STACK_TRACES` -> `EXPORT_EXCEPTION_HANDLING_HELPERS` link. Without this link, we can still see stack traces with exception messages with no problem, because `__cxa_throw` -> `__throw_exception_with_stack_trace` -> `getExceptionMessage` dependencies: https://github.com/emscripten-core/emscripten/blob/6ad2f5e03021a39377428e9d476985fc967014d4/system/lib/libcxxabi/src/cxa_exception.cpp#L302-L304 https://github.com/emscripten-core/emscripten/blob/6ad2f5e03021a39377428e9d476985fc967014d4/src/lib/libexceptions.js#L311 2. If we do 1, Emscripten EH's `getExceptionMessage` does not work, because unlike Wasm EH, its `getExceptionMessage` dependency is within `runtime_exceptions.js`, where we can't attach `__deps`, so we can't track it: https://github.com/emscripten-core/emscripten/blob/6ad2f5e03021a39377428e9d476985fc967014d4/src/runtime_exceptions.js#L20 So, this adds `getExceptionMessage` as a dependency of `libexception.js`'s `__cxa_throw` directly, when `EXCEPTION_STACK_TRACES` is enabled. 3. This removes several functions from `REQUIRED_EXPORTS` when Emscripten EH is used. Previously, the comment said, in LTO mode, `_cxa_find_matching_catch_*` -> `__cxa_can_catch` dependency was not tracked. But now, in Emscripten EH, we preemptively add several `__cxa_find_matching_catch_n`s, and it depends on `findMatchingCatch`, which depends on `__cxa_end_catch`: https://github.com/emscripten-core/emscripten/blob/78403050f355085104175499224ad0f6bccb5fb1/src/lib/libexceptions.js#L371-L405 https://github.com/emscripten-core/emscripten/blob/78403050f355085104175499224ad0f6bccb5fb1/src/lib/libexceptions.js#L217 So we don't need to require exporting `__cxa_end_catch` anymore. Also, other required exports (`__cxa_in/decrement_exception_count` and `__cxa_free_exceptions`) are dependencies that can be naturally referred within libc++abi. I think these were added here in #16627 when we were using `deps_info.py`, which we don't use anymore. After this commit, when you compile an empty `int main { return 0; }` with `-O0` and `-fexceptions`/`-fwasm-exceptions`, the size decreases to: - `-fexceptions`: 113212 -> 1168 - `-fwasm-exceptions`: 109177 -> 1106

sbc100 force-pushed the emscripten_exceptions_native branch from 8cb6dae to 225496d Compare March 29, 2022 21:24

sbc100 requested a review from kripken March 29, 2022 21:56

sbc100 force-pushed the emscripten_exceptions_native branch 3 times, most recently from 6befd1b to ed6e571 Compare March 30, 2022 19:35

kripken reviewed Mar 30, 2022

View reviewed changes

tests/test_core.py Show resolved Hide resolved

system/lib/libcxxabi/src/cxa_noexception.cpp Outdated Show resolved Hide resolved

system/lib/libcxxabi/src/cxa_emscripten.cpp Outdated Show resolved Hide resolved

system/lib/libcxxabi/src/cxa_emscripten.cpp Show resolved Hide resolved

sbc100 force-pushed the emscripten_exceptions_native branch from ed6e571 to c7c5836 Compare March 30, 2022 20:48

kripken reviewed Apr 1, 2022

View reviewed changes

system/lib/libcxxabi/src/cxa_exception_emscripten.cpp Outdated Show resolved Hide resolved

sbc100 force-pushed the emscripten_exceptions_native branch from c7c5836 to 3efb144 Compare April 15, 2022 00:05

kripken approved these changes Apr 18, 2022

View reviewed changes

sbc100 mentioned this pull request May 20, 2022

[EH] Tidy up format_exception.cpp (NFC) #16995

Merged

aheejin mentioned this pull request May 24, 2022

[EH] Rename format_exception.cpp #17045

Merged

RReverser force-pushed the emscripten_exceptions_native branch from 3efb144 to a113c6b Compare March 27, 2023 21:25

sbc100 commented Mar 27, 2023

View reviewed changes

RReverser reviewed Mar 27, 2023

View reviewed changes

RReverser force-pushed the emscripten_exceptions_native branch from c563002 to b5c68f6 Compare March 28, 2023 13:29

sbc100 mentioned this pull request Apr 12, 2023

Fix lto0.test_exceptions_allowed_uncaught failure #19165

Merged

sbc100 added a commit that referenced this pull request Apr 13, 2023

Remove unnecessary dependency from deps_info.py. NFC

f68dc41

This was recently introduced in #16627 but when to a different deps system in #18905 I noticed it is not necessary.

sbc100 mentioned this pull request Apr 13, 2023

Remove unnecessary dependency from deps_info.py. NFC #19166

Merged

sbc100 added a commit that referenced this pull request Apr 13, 2023

Remove unnecessary dependency from deps_info.py. NFC (#19166)

5394838

This was recently introduced in #16627 but when to a different deps system in #18905 I noticed it is not necessary.

dschuff mentioned this pull request Apr 14, 2023

Add a stub __cxa_free_exception to cxa_noexception.cpp #19183

Merged

aheejin mentioned this pull request Mar 19, 2026

[EH] Don't export unnecessary functions for EH when not used #26493

Merged

Conversation

sbc100 commented Mar 29, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kripken commented Apr 1, 2022

Uh oh!

sbc100 commented Apr 15, 2022

Uh oh!

sbc100 commented Apr 15, 2022

Uh oh!

kripken left a comment

Choose a reason for hiding this comment

Uh oh!

RReverser commented Mar 27, 2023

Uh oh!

sbc100 commented Mar 27, 2023

Uh oh!

RReverser commented Mar 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RReverser commented Mar 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RReverser commented Mar 27, 2023

Uh oh!

sbc100 commented Mar 27, 2023

Uh oh!

Uh oh!

sbc100 Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

RReverser Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RReverser commented Mar 27, 2023

Uh oh!

RReverser commented Mar 27, 2023

Uh oh!

RReverser Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

RReverser commented Mar 28, 2023

Uh oh!

RReverser commented Apr 12, 2023

Uh oh!

sbc100 commented Apr 12, 2023

Uh oh!

sbc100 commented Apr 12, 2023

Uh oh!

RReverser commented Apr 12, 2023

Uh oh!

dschuff commented Apr 14, 2023

Uh oh!

dschuff commented Apr 14, 2023

Uh oh!

sbc100 commented Apr 14, 2023

Uh oh!

sbc100 commented Apr 14, 2023

Uh oh!

sbc100 commented Apr 14, 2023

Uh oh!

dschuff commented Apr 14, 2023

Uh oh!

dschuff commented Apr 14, 2023

Uh oh!

dschuff commented Apr 14, 2023

Uh oh!

sbc100 commented Apr 14, 2023

Uh oh!

sbc100 commented Apr 14, 2023

Uh oh!

sbc100 commented Apr 14, 2023

Uh oh!

dschuff commented Apr 14, 2023

Uh oh!

RReverser commented Mar 27, 2023 •

edited

Loading

RReverser commented Mar 27, 2023 •

edited

Loading