Exception handling support to MINIMAL_RUNTIME

[juj]

This refactors exception handling support to its own file, breaks up the $EXCEPTIONS object, and adds exceptions support to MINIMAL_RUNTIME.

Also adds tracking that the new library_exceptions.js file will only be linked in if -fno-exceptions is not used. However -fno-exceptions is traditionally a compiler flag and not a linker flag, so at link time one should also issue -fno-exceptions to enforce no exceptions getting in, or add the link time flag -s SUPPORT_CPP_THROW_EXCEPTION=0 explicitly. This new flag is convenient when one is linking in multiple libraries and doesn't know if one of them used exceptions or not - with -s SUPPORT_CPP_THROW_EXCEPTION=0 the whole library_exceptions.js file will be omitted, giving immediate linker errors for any exceptions related symbols.

[kripken]

Flag-wise, we already have a link-time flag called DISABLE_EXCEPTION_CATCHING (on by default). So users that want exceptions have been turning it off already.

[kripken]

Looks mostly good, but I think we should use the existing DISABLE_EXCEPTION_CATCHING flag as mentioned, and another minor comment or too.

If we do want another link-time flag, I'm not opposed to that, but tying it to this PR seems risky.

[juj]

Looks mostly good, but I think we should use the existing DISABLE_EXCEPTION_CATCHING flag as mentioned, and another minor comment or too.

The use of DISABLE_EXCEPTION_CATCHING and SUPPORT_CPP_THROW_EXCEPTION are different, and both can have their uses. If I use DISABLE_EXCEPTION_CATCHING=1, it means that throws are supported and compile ok, but they'll fall through to top level scope at runtime, and you won't know at compile/link time if any code actually did use exceptions, whereas SUPPORT_CPP_THROW_EXCEPTION=0 means that we issue an error at link time if any of the C++ exception machinery is used anywhere in the linked program. SUPPORT_CPP_THROW_EXCEPTION=0 could be renamed to a new mode DISABLE_EXCEPTION_CATCHING=3, but that might be a bit more confusing, since it already uses the =1 or =2 setting for "all/select functions only" options.

If I have
a.cpp

int main()
{
	throw 4;
}

and I do

$ em++ a.cpp -o a.bc
$ em++ a.bc -o a.html -fno-exceptions

the above code will compile and run, and the throw will be there, printing

exception thrown: 5247224 - Exception catching is disabled, this exception cannot be caught. Compile with -s DISABLE_EXCEPTION_CATCHING=0 or DISABLE_EXCEPTION_CATCHING=2 to catch.

Imagine the two lines may be coming from different stages of the build system, or a.bc might be code that was checked in to a precompiled code repository. After this PR, -fno-exceptions gets also turned into a link flag, and the above code prints

$ em++ a.cpp -o a.bc
$ em++ a.bc -o a.html -fno-exceptions
error: undefined symbol: __cxa_allocate_exception
warning: To disable errors for undefined symbols use `-s ERROR_ON_UNDEFINED_SYMBOLS=0`
error: undefined symbol: __cxa_throw
Error: Aborting compilation due to previous errors

I overloaded this setting onto -fno-exceptions for convenience and to be "idiomatic". The same feature can/could be accessed just with

$ em++ a.bc -o a.html -s SUPPORT_CPP_THROW_EXCEPTION=0
error: undefined symbol: __cxa_allocate_exception
warning: To disable errors for undefined symbols use `-s ERROR_ON_UNDEFINED_SYMBOLS=0`
error: undefined symbol: __cxa_throw
Error: Aborting compilation due to previous errors

to be explicit.

[kripken]

I see the difference between the flags now. But I don't see the benefit. Can the old flag not reduce the throw into an abort(), and then they are basically the same?

[juj]

The old -s DISABLE_EXCEPTION_CATCHING=1 flag still allows one to write a global JS window.onerror() handler where one decodes std::exception::what() to a human readable string for debugging or similar. One example here: https://stackoverflow.com/a/33743704 . So being able to throw C++ exceptions can still be useful. If we turned -s DISABLE_EXCEPTION_CATCHING=1 into a throw 'abort';, they'll lose that benefit.

[kripken]

I see, thanks.

It seems more natural to have DISABLE_EXCEPTION_THROWING to parallel the _CATCHING flag, then. But the argument that there's an existing compiler flag also has merit. I don't have a strong intuition here, maybe best to ask people with more experience in the matter - @sbc100 @dschuff, thoughts?

[juj]

The PR does latch this onto -fno-exceptions, but the reason why there's also a dedicated option is that I don't know if some build systems might not let one specify -fno-exceptions at link time, in which case a dedicated option would be useful. (also that -s setting is internally used to route the information from frontend to linker, so making it publicly visible was trivial, nicer than marking it internal)

[sbc100]

I like splitting the exceptions library stuff into its own file and disabling it completely when exceptions are not requested.

Is it worth being more consistent about forcing using to opt into exception handling though? IIUC, at the moment we disable exceptions by default at compile time but allow them by default at link time?

[sbc100]

Looks like two functions moved from JS exported functions to native functions here.. At least for wasm backend. Does this make sense? Do you know which functions moved?

[juj]

I don't know the details on what changed here, this was done blind against the CI. Updated the code a little, not sure what CI says on this now. The major change is the unpacking of the $EXCEPTIONS object, so more functions are now tracked as JS functions; and the deps info mechanism was revised, not sure which function it is pulling in to these (if I had to guess, it'd be free()) I don't have upstream wasm LLVM installed at the moment to verify though.

[kripken]

I think this is good: what happened is we have _ZSt18uncaught_exceptionv in library.js, as a hack. With the more proper deps here, we get it from the system library. So an import is replaced by an export.

We should remove _ZSt18uncaught_exceptionv from the new src/library_exceptions.js file, but can leave that to a followup.

[sbc100]

Where these deps simply wrong?

[juj]

They were taking a shortcut, i.e. __gxx_personality_v0 depended on something that depended on something that eventually depended on these. I don't remember any more what the exact chain here was to start with, but adjusted to be more precise.

[juj]

Oh wait, now I remember. This is due to limitation of deps tracking, a JS function can't depend on a C++ function, then depend back on JS function, and again to C++ function. This seems to be due to JS->C++ function dependencies being resolved once first, and then all JS->JS dependencies second. This moved over to deps_info.json to help around that limitation. (it can safely be added to both, put it back here to be clearer)

[sbc100]

Just to be clear, if the symbol dependency system works correctly the exception handling library function will only be included if they are used by the program?

And this new link time flag lets you know if any of the code you are linking references any of these symbol, because it generated a link error in such cases.

And we are not changing the emscipten default here which is to allow exception thrown at link time by default but disallow throwing at compile time default. Seems reasonable, if a little confusing.

[juj]

Just to be clear, if the symbol dependency system works correctly the exception handling library function will only be included if they are used by the program?

Correct, the deps system does track through when to emit exceptions related code. Although I am not sure if the current deps chain is 100% accurate. One example is that we have something like this:

  __gxx_personality_v0__deps: ['_ZSt18uncaught_exceptionv', '__cxa_find_matching_catch'],
  __gxx_personality_v0: function() {
  },

that is, a no-op function that is marked to depend on exceptions. I am not versed on the details of gxx_personality_v0's semantics, but I notice if I remove those deps, then test suite did not pass, so that is necessary.

And this new link time flag lets you know if any of the code you are linking references any of these symbol, because it generated a link error in such cases.

That's correct.

And we are not changing the emscipten default here which is to allow exception thrown at link time by default but disallow throwing at compile time default. Seems reasonable, if a little confusing.

That is right. It is important to allow throwing by default, since that will make exceptions raise all the way up, which lets one do a convenient global error handler to track aborts from C++ throws.

[juj]

It seems more natural to have DISABLE_EXCEPTION_THROWING to parallel the _CATCHING flag, then. But the argument that there's an existing compiler flag also has merit. I don't have a strong intuition here, maybe best to ask people with more experience in the matter - @sbc100 @dschuff, thoughts?

Renamed SUPPORT_CPP_THROW_EXCEPTION to DISABLE_EXCEPTION_THROWING, that seems like a more symmetric name.

[juj]

Rebased this PR to latest, not sure if this will pass everything, but let's see how things look on the CI.

[kripken]

almost lgtm, but I do have questions about all those _1,2,3,.. etc. functions, that worry me.

[kripken]

why are all these 1,2,3 etc. versions needed? and why just 10?

[juj]

The change to explicitly list out __cxa_find_matching_catch_<number> is a change to migrate these functions to be defined as JS library functions (in src/library_exceptions.js ), instead of the symbols being defined ad hoc in JS linker (used to be in src/jsifier.js at

emscripten/src/jsifier.js

Line 103 in ee231b0

// special logic

). See #8046 that discusses this aspect in more detail.

[kripken]

I see, sounds good - if we have a nice error if we need more than 10. What would happen in that case?

[juj]

if there are more than 10, there will likely be an undefined symbol error, since the linker cannot find a function that LLVM emitted a dependence to

[kripken]

I worry that's not super-clear, but if it comes up we can look into improving it I guess.

[kripken]

why are all these added?

[juj]

Hmm, I don't really know how the JS exceptions machinery works in dynamic linking across main module and side module - before this PR, what would happen if a side module throws an exception with say 3 arguments, but main module did not have a need to generate ___cxa_find_matching_catch_3 in code?

That is, superficially I get an impression that main module would need to emit all these variants to prepare for all possibilities in the side module?

[kripken]

Honestly, I think exceptions probably did not work in all cases in dynamic linking, we only have very basic testing for that.

[kripken]

why does this go up to 31, if the limit is 10?

[juj]

Forgot to update the test - earlier I had it go up to 31, but that was excessive. Updated.

[kripken]

lgtm with those last comments addressed.