Add parameter names to GL Emulation functions to fix 4GB mode link error #19962
Conversation
test/test_browser.py
Outdated
| self.btest('glgettexenv.c', args=['-sLEGACY_GL_EMULATION', '-lGL', '-lSDL'], expected='1') | ||
| # Test with 4GB maximum memory to test that GL emulation does not error | ||
| # during link. | ||
| self.btest('glgettexenv.c', args=['-sLEGACY_GL_EMULATION', '-lGL', '-lSDL', '-sMAXIMUM_MEMORY=4GB', '-sALLOW_MEMORY_GROWTH'], expected='1') |
There was a problem hiding this comment.
You would also get the same error by referencing these symbols in a wasm64 build.
There was a problem hiding this comment.
Would that be better? It seems less intrusive to increase the max memory to me. Unless you mean we should also have wasm64 coverage?
There was a problem hiding this comment.
No, this approach seems fine.
test/glgettexenv.c
Outdated
| if (argc == 1234) { | ||
| GLint i; | ||
| glGetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_WIDTH, &i); | ||
| } |
There was a problem hiding this comment.
Just taking the address of this function would also be enough to trigger the error. (i.e. printf("%p\n", glGetTexLevelParameteriv) ) although I'm not sure that is more readable.
There was a problem hiding this comment.
Oh wait, I remember now how I went about testing and fixing this the last time it came up. We can use -sINCLUDE_FULL_LIBRARY for force inclusion of all JS library funcs.
See #19902
Instead of this test I would add a wasm64 or 4gb mode to other.test_full_js_library. That should catch this as well as any others.
There was a problem hiding this comment.
Sounds good, done. I also confirmed the new test fails before the fix here.
Fixes #19944
Without this, we hit