[3.x] Build javascript+GDNative export template with -s WASM_BIGINT

[derivator]

This tells emscripten to pass i64 values from WebAssembly to JavaScript as BigInt instead of as two i32s, which could lead to crashes when GDNative modules are used.

@hoodmane wrote about it here. The gist of it (as I understand it) is that without this patch, emscripten generates custom wrapper functions per function signature to do the (i32, i32) -> i64 conversion, and if a GDNative module has to call a function with a signature not present in the Godot main module, an error is thrown. For my godot-rust project, this patch fixes an immediate crash on launch.

Note that this does reduce browser compatibility a bit for GDNative exports (to browsers supporting BigInt64Array, although emscripten-core/emscripten#17103 if merged would relax that requirement to just BigInt).

[Faless]

if a GDNative module has to call a function with a signature not present in the Godot main module, an error is thrown

Sorry, I don't fully understand this part. Where is the function defined then?

Note that this does reduce browser compatibility a bit for GDNative exports

I feel that's kind of a problem for now, at least in the 3.x branch which tries to have wide browser support.

[derivator]

if a GDNative module has to call a function with a signature not present in the Godot main module, an error is thrown

Sorry, I don't fully understand this part. Where is the function defined then?

Sorry, it's explained better in the blogpost I linked:

If a function called inside of a C++ try block has a different signature from all functions called in try blocks in the main executable, we see the error above.

The function is defined in (and called by) the GDNative module itself, but for exception-handling-purposes is wrapped in a JavaScript try-block, so the call makes a roundtrip from wasm to js. That's where the wrapper function comes in.

Note that this does reduce browser compatibility a bit for GDNative exports

I feel that's kind of a problem for now, at least in the 3.x branch which tries to have wide browser support.

Well, that applies only to exports with GDNative enabled, which without this patch are broken anyway it seems.

[hoodmane]

Minimum browser versions for webassembly are:

• Firefox 52
• Chrome 57
• Safari 11

Minimum browser versions for -sWASM_BIGINT are:

• Firefox 68 (Firefox 52 -- 67 which is 0.08% of browsers)
• Chrome 67 (Chrome 57 -- 66 which is 0.16% of browsers)
• Safari 15.0 or Safari 14.0 (Safari 11 -- 13 which is 1.34% of browsers, Safari 14 is 2.65% of browsers)

So you see the issue is mostly Safari. Current tip of tree Emscripten only supports Safari 15 with -sWASM_BIGINT, whereas with emscripten-core/emscripten#17103 it would support Safari 14.

[hoodmane]

It would be possible to fix Emscripten dynamic linking without -sWASM_BIGINT. I could potentially contribute it even. The Emscripten developers don't seem to be interested in working on it though.

[hoodmane]

To enable the Safari 14 compatibility when it comes along you should also pass -sPOLYFILL.

[Faless]

Well, that applies only to exports with GDNative enabled, which without this patch are broken anyway it seems.

That's not the case, given I do test GDNative side modules, and they do work, maybe it's broken for your case, but not "generally broken".

If a function called inside of a C++ try block has a different signature from all functions called in try blocks in the main executable, we see the error above.

From the post, this seem an issue specific to C++ exception, which are disabled for the main module to reduce size so I'm actually surprised would work in the side modules at all.

[hoodmane]

It's a problem with panics. If you disable exception support, ideally it should abort at runtime when an exception is thrown. Currently there are linker errors. It might be worth trying to compile with panic=abort, I suspect that might fix the problem.

[Faless]

If you disable exception support, ideally it should abort at runtime when an exception is thrown.

But then I don't see how BIGINT support fixes the OP issue if it's caused by try/catch blocks and exceptions are disabled anyway so it should abort?

[hoodmane]

The link error is caused by a three-way interaction between stack unwinding, 64 bit integer handling, and dynamic linking.

[hoodmane]

Okay I am pretty sure that setting panic=abort when building with Emscripten in all the godot-rust crates would fix the problem.

You can test whether a library is going to cause a problem with:

wasm-objdump mylib.wasm -j Import -x | grep 'legalimport$invoke'

if you see any output, it is probably broken.

[akien-mga]

Sorry for the delay following up on this.

It's not clear to me what's a reproducer for this issue so we could confirm the need for WASM_BIGINT in 3.x.

I do see however that we added WASM_BIGINT in 4.x, as required for Emscripten 3.1.41 and later.

CC @adamscott

[akien-mga]

I found that this was superseded by #88594, which should be cherry-picked for 3.x.
Thanks for the contribution!