glClientWaitSync and glWaitSync broken with WASM_BIGINT=1

[JosiahWhite]

I am encountering runtime errors when trying to run an application built with emscripten, based on what I have found, the bindings for emscripten_glClientWaitSync and emscripten_glWaitSync are always defined to have split parameters for the timeout parameter in the header files. I am also getting an error in the console from wasm complaining that the function call ABI doesn't match what is expected.

From what I grasp, this relates to how on platforms not supporting BIGINT the timeout parameter needs split into a hi/lo pair. I can manually update the bindings to pass a single GLuint64 in system/lib/gl/webgl2.c and system/include/webgl/webgl2.h. After rebuilding the bindings via embuilder and rebuilding my application, everything appears to work fine but this doesn't seem like the proper way the issue should be solved.

I don't have a problem fixing this bug if I could get some insight into how the header files and bindings should be updated to properly work with both WASM_BIGINT=0 and WASM_BIGINT=1.

[sbc100]

It looks like the emscripten-specific emscripten_glClientWaitSync and emscripten_glWaitSync versions use the split params, because we need to support proxying GL calls between threads and the proxying system does not support i64.

See

emscripten/system/lib/gl/webgl2.c

Lines 98 to 111 in ffb1e90

	GLenum glClientWaitSync(GLsync p0, GLbitfield p1, GLuint64 p2) {
	GL_FUNCTION_TRACE();
	if (pthread_getspecific(currentThreadOwnsItsWebGLContext))
	return emscripten_glClientWaitSync(p0, p1, p2 & 0xFFFFFFFF, (p2 >> 32) & 0xFFFFFFFF);
	else
	return (GLenum)emscripten_sync_run_in_main_runtime_thread(EM_FUNC_SIG_IIIII, &emscripten_glClientWaitSync, p0, p1, p2 & 0xFFFFFFFF, (p2 >> 32) & 0xFFFFFFFF);
	}
	void glWaitSync(GLsync p0, GLbitfield p1, GLuint64 p2) {
	GL_FUNCTION_TRACE();
	if (pthread_getspecific(currentThreadOwnsItsWebGLContext))
	emscripten_glWaitSync(p0, p1, p2 & 0xFFFFFFFF, (p2 >> 32) & 0xFFFFFFFF);
	else
	emscripten_sync_run_in_main_runtime_thread(EM_FUNC_SIG_VIIII, &emscripten_glWaitSync, p0, p1, p2 & 0xFFFFFFFF, (p2 >> 32) & 0xFFFFFFFF);
	}

The normal non-emscripten versions glClientWaitSync and glWaitSync use i64 as expected.

Is there some issue with the emscripten_ versions having a different signature? I would hope this is a mostly internal issue, but maybe there is some reason why its causing issues?

[sbc100]

Looking at the proxying system it does look like J (int64) is supported so maybe this workaround is no longer needed?

[sbc100]

In fact the proxying system had PARAM_I64 since it was added in cdf7bc5.. so I'm confused about why this was done this way at all. @juj, do you happen to remember?

[JosiahWhite]

The underlying emscripten bindings for glClientWaitSync use an i64 when WASM_BIGINT is enabled here:

emscripten/src/lib/libwebgl2.js

Lines 680 to 690 in ffb1e90

	#if !WASM_BIGINT
	glClientWaitSync__deps: ['$convertI32PairToI53'],
	#endif
	glClientWaitSync: (sync, flags, {{{ defineI64Param('timeout') }}}) => {
	// WebGL2 vs GLES3 differences: in GLES3, the timeout parameter is a uint64, where 0xFFFFFFFFFFFFFFFFULL means GL_TIMEOUT_IGNORED.
	// In JS, there's no 64-bit value types, so instead timeout is taken to be signed, and GL_TIMEOUT_IGNORED is given value -1.
	// Inherently the value accepted in the timeout is lossy, and can't take in arbitrary u64 bit pattern (but most likely doesn't matter)
	// See https://www.khronos.org/registry/webgl/specs/latest/2.0/#5.15
	{{{ receiveI64ParamAsI53Unchecked('timeout'); }}}
	return GLctx.clientWaitSync(GL.syncs[sync], flags, timeout);
	},

Which seems to be why the function is changing to accept an i64, but the bindings are expecting it to always use a pair of i32s? Same story for glWaitSync a few lines below. Should this branch be removed?

[sbc100]

I took a stab at fixing it in #25933.

Sadly I don't think we have any tests that cover this API.. at least not as far as I can tell.

[JosiahWhite]

I'll test the PR in roughly 12 hours and let you know how it goes

[juj]

LGTM with the PR. I can't quite remember what the issue with that code could have been.

Would be fantastic to get manual testing from @JosiahWhite if you have a moment - that would ensure that there are no unexpected subtle issues with the PR.