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

[aheejin]

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:

   emscripten/system/lib/libcxxabi/src/cxa_exception.cpp

   Lines 302 to 304 in 6ad2f5e

   	// In debug mode, call a JS library function to use WebAssembly.Exception JS
   	// API, which enables us to include stack traces
   	__throw_exception_with_stack_trace(&exception_header->unwindHeader);

   emscripten/src/lib/libexceptions.js

   Line 311 in 6ad2f5e

   __throw_exception_with_stack_trace__deps: ['$getCppExceptionTag', '$getExceptionMessage'],

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:

   emscripten/src/runtime_exceptions.js

   Line 20 in 6ad2f5e

   const excInfo = getExceptionMessage(excPtr);

   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_ns, and it depends on findMatchingCatch, which depends on __cxa_end_catch:

   emscripten/src/lib/libexceptions.js

   Lines 371 to 405 in 7840305

   	#if !WASM_EXCEPTIONS
   	// In LLVM, exceptions generate a set of functions of form
   	// __cxa_find_matching_catch_2(), __cxa_find_matching_catch_3(), etc. where the
   	// number specifies the number of arguments. In Emscripten, route all these to
   	// a single function '__cxa_find_matching_catch' that variadically processes all
   	// of these functions using JS 'arguments' object.
   	addCxaCatch = (n) => {
   	const args = [];
   	// Confusingly, the actual number of argument is n - 2. According to the llvm
   	// code in WebAssemblyLowerEmscriptenEHSjLj.cpp:
   	// This is because a landingpad instruction contains two more arguments, a
   	// personality function and a cleanup bit, and __cxa_find_matching_catch_N
   	// functions are named after the number of arguments in the original landingpad
   	// instruction.
   	let sig = 'p';
   	for (let i = 0; i < n - 2; i++) {
   	args.push(`arg${i}`);
   	sig += 'p';
   	}
   	const argString = args.join(',');
   	LibraryManager.library[`__cxa_find_matching_catch_${n}__sig`] = sig;
   	LibraryManager.library[`__cxa_find_matching_catch_${n}__deps`] = ['$findMatchingCatch'];
   	LibraryManager.library[`__cxa_find_matching_catch_${n}`] = eval(`(${args}) => findMatchingCatch([${argString}])`);
   	};
   	// Add the first 2-5 catch handlers preemptively. Others get added on demand in
   	// jsifier. This is done here primarily so that these symbols end up with the
   	// correct deps in the stub library that we pass to wasm-ld.
   	// Note: __cxa_find_matching_catch_N function uses N = NumClauses + 2 so
   	// __cxa_find_matching_catch_2 is the first such function with zero clauses.
   	// See WebAssemblyLowerEmscriptenEHSjLj.cpp.
   	for (let i = 2; i < 5; i++) {
   	addCxaCatch(i)
   	}
   	#endif

   emscripten/src/lib/libexceptions.js

   Line 217 in 7840305

   $findMatchingCatch__deps: ['$exceptionLast', '$ExceptionInfo', '__cxa_can_catch', '$setTempRet0'],

   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 Move parts of emscripten exception handling to native code. NFC #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]

Nice!

[sbc100]

Perhaps this dependency warrants a comment since its not directly called below in this function?

[aheejin]

Done: 21a2cc9

[sbc100]

Should it even be legal to link C programs with -fexceptions / -fwasm-exceptions?

Currently we do default C++ linking but I'm hoping to disable that soon by turning off LINK_AS_CXX by default.

[aheejin]

clang -fexceptions main.c

compiles without a problem, so I guess it should..?

By the way what does LINK_AS_CXX do? It is already false by default:

emscripten/src/settings_internal.js

Lines 213 to 214 in c624110

	// Set to true if we are linking as C++ and including C++ stdlibs
	var LINK_AS_CXX = false;

[sbc100]

I know it compiles without a problem today, but maybe it shouldn't?

Actually it looks like clang allows -fexceptions and -fno-exceptions for plain-old-c code so I guess this fine.

LINK_AS_CXX is set based on DEFAULT_TO_CXX which I'm hoping to disable by default soon. It means you will need to use em++ to link C++ programs just like you need to use g++ or clang++ today.

[aheejin]

OK, changed main.c to main.cpp in the test: 121100c

[sbc100]

Since this is now just one element maybe move the comment outside and make this into single line of code

[aheejin]

Done: 27068e9

[sbc100]

Should it even be legal to link C programs with -fexceptions / -fwasm-exceptions?

Currently we do default C++ linking but I'm hoping to disable that soon by turning off LINK_AS_CXX by default.

I'm not saying we shouldn't land this as-is, but just wondering if its desirable to to make -fexceptions at leat not valid for C programs. In which case maybe this test could just use main.cpp instead?]

Maybe it should be called test_unused_eh_dce ?

[aheejin]

Should it even be legal to link C programs with -fexceptions / -fwasm-exceptions?

Currently we do default C++ linking but I'm hoping to disable that soon by turning off LINK_AS_CXX by default.
I'm not saying we shouldn't land this as-is, but just wondering if its desirable to to make -fexceptions at leat not valid for C programs. In which case maybe this test could just use main.cpp instead?

This looks a duplication of #26493 (comment). I replied there.

Maybe it should be called test_unused_eh_dce ?

Done: 9e9b4a2

[sbc100]

Do we need to update any corresponding docs for this?

[aheejin]

Hmm, how about just removing EXPORT_EXCEPTION_HANDLING_HELPERS? Given that using throw (either in Emscripten or Wasm EH) exports getExceptionMessage anyway (this behavior was there; it hasn't changed in this PR), the only thing a user may additionally want to use from the JS side is decrementExceptionRefcount to avoid memory leaks.

emscripten/test/test_core.py

Lines 1534 to 1548 in c624110

	int main() {
	EM_ASM({
	for (let i = 1; i < 6; i++){
	try {
	_throw_exc(i);
	} catch(p) {
	// Because we are catching and handling the exception in JS, the normal
	// exception catching C++ code doesn't kick in, so we need to make sure we free
	// the exception, if necessary. By decrementing the refcount we trigger the
	// free'ing of the exception.
	out(getExceptionMessage(p).toString());
	decrementExceptionRefcount(p);
	}
	}
	});

And currently EXPORT_EXCEPTION_HANDLING_HELPERS exports three functions: getExceptionMessage, incrementExceptionRefcount, and decrementExceptionRefcount. Actually I can't think of any reason why a user would want to call incrementExceptionRefcount. It was there just because decrementExceptionRefcount was there and they looked like a pair.

How about adding those in/decrementExceptionRecount to __cxa_throw's deps when EXCEPTION_STACK_TRACES is on, like we did for getExceptionMessage here, and just removing EXPORT_EXCEPTION_HANDLING_HELPERS? I think one less EH option is better. The current docs say you need to turn EXPORT_EXCEPTION_HANDLING_HELPERS on to use getExceptionMessage (which is not true) but gives the test_core.py code example above (which contains decrementExceptionRefcount, which does require EXPORT_EXCEPTION_HANDLING_HELPERS). If we just remove this option, users can use all those functions whenever EXCEPTION_STACK_TRACES is on, without setting another option.

If you agree, I'll do it as a follow-up.

[sbc100]

I'm always a fan of removing settings. Followup makes sense yes.

[aheejin]

#26499

[sbc100]

typo in storeEcxeption

[aheejin]

Fixed