fix dynamic linking int64 exceptions

[joemarshall]

Currently if you dynamic link to something, and throw an exception in a function which returns int64, a javascript exception is thrown.

This is the same bug that was fixed in #15974 but in this case the dynamic linker is creating the invoke functions in javascript, rather than it being created in advance.

Fixes #21758

[joemarshall]

@sbc100 If you look at the code for this, I added a one line check on exceptions thrown, as opposed to adding two function generators, one for bigint return and one for normal, as I think code size is a priority here right? I don't think there's a massive overhead given it is only in the exception thrown case anyway.

Other than that, I think this is pretty uncontroversial; it would be good to get it in soon if possible; I'll submit a patch to it in pyodide until it's in.

[sbc100]

It would be great to add a test for this. Ideally we would figure out which test already covers this case and extend it.

[joemarshall]

@sbc100 I added a test for this - it was quite fiddly to force the invoke call to happen, but the test fails before the fix and works after now..

[sbc100]

Thanks for putting in the effort to make that test case. It looks like it it is indeed quite hard to reproduce!

I was wondering if there are any existing test that test this feature explictly.. I'll see if I can find one.

[sbc100]

It looks like we do have some existing tests for this part of the code:

coer0.test_dylink_i64_invoke
coer0.test_dylink_i64_invoke_bigint
coer0.test_dylink_raii_exceptions
coer0.test_pthread_dylink_exceptions

other.test_main_module_without_exceptions_message
other.test_metadce_hello_dylink

At least these are the tests that fail when I disable the createInvokeFunction stuff.

I wonder if we can extend/update these tests to cover this corner case? (I think test_dylink_i64_invoke looks like the one)

[joemarshall]

Ha ha, you were right. Now I know how to trigger it, it took 3 line changes to that existing test.

[joemarshall]

Incidentally, it appears empty throw statements don't always work in emscripten. I don't think that is necessarily a bad thing, as who does that... but it may be an inconsistency from standard C++ behaviour and thus another bug?

[joemarshall]

@sbc100 I updated this to main again, can we merge it now?