Skip to content

Commit

Permalink
Fix for zero count in glUniform family of functions
Browse files Browse the repository at this point in the history 
The previous fix for this was in emscripten-core#16837, but I looks like that only
covered the new "garbage-free" webgl2 path.  When old webgl1 path was
still using the zero count value.  For example the following line was
unguarded:

```
var view = miniTempWebGLFloatBuffers[4 * count - 1];
```

This recently resurfaced because I introduced
`WEBGL_USE_GARBAGE_FREE_APIS` which is currently disabled for memories
larger 2gb.  This meant that users with large memories were forces onto
the old path where the bug still existed.

Rather than adding yet more `count &&` prefixes, this patch simply adds
a single early return at the top of each function.

Fixes: emscripten-core#21567
  • Loading branch information
@sbc100
sbc100 committed Mar 20, 2024
1 parent be8f1d1 commit 386965b
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 38 deletions.
55 changes: 33 additions & 22 deletions src/library_webgl.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2421,17 +2421,18 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
GL.validateGLObjectID(GLctx.currentProgram.uniformLocsById, location, 'glUniform1iv', 'location');
assert((value & 3) == 0, 'Pointer to integer data passed to glUniform1iv must be aligned to four bytes!');
#endif
if (!count) return;
#if MIN_WEBGL_VERSION >= 2 && WEBGL_USE_GARBAGE_FREE_APIS
#if GL_ASSERTIONS
assert(GL.currentContext.version >= 2);
#endif
count && GLctx.uniform1iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count);
GLctx.uniform1iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count);
#else
#if WEBGL_USE_GARBAGE_FREE_APIS
if ({{{ isCurrentContextWebGL2() }}}) {
count && GLctx.uniform1iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count);
GLctx.uniform1iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count);
return;
}
#endif
Expand Down Expand Up @@ -2462,17 +2463,18 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
GL.validateGLObjectID(GLctx.currentProgram.uniformLocsById, location, 'glUniform2iv', 'location');
assert((value & 3) == 0, 'Pointer to integer data passed to glUniform2iv must be aligned to four bytes!');
#endif
if (!count) return;
#if MIN_WEBGL_VERSION >= 2 && WEBGL_USE_GARBAGE_FREE_APIS
#if GL_ASSERTIONS
assert(GL.currentContext.version >= 2);
#endif
count && GLctx.uniform2iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count*2);
GLctx.uniform2iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count*2);
#else
#if WEBGL_USE_GARBAGE_FREE_APIS
if ({{{ isCurrentContextWebGL2() }}}) {
count && GLctx.uniform2iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count*2);
GLctx.uniform2iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count*2);
return;
}
#endif
Expand Down Expand Up @@ -2504,17 +2506,18 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
GL.validateGLObjectID(GLctx.currentProgram.uniformLocsById, location, 'glUniform3iv', 'location');
assert((value & 3) == 0, 'Pointer to integer data passed to glUniform3iv must be aligned to four bytes!');
#endif
if (!count) return;
#if MIN_WEBGL_VERSION >= 2 && WEBGL_USE_GARBAGE_FREE_APIS
#if GL_ASSERTIONS
assert(GL.currentContext.version >= 2);
#endif
count && GLctx.uniform3iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count*3);
GLctx.uniform3iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count*3);
#else
#if WEBGL_USE_GARBAGE_FREE_APIS
if ({{{ isCurrentContextWebGL2() }}}) {
count && GLctx.uniform3iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count*3);
GLctx.uniform3iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count*3);
return;
}
#endif
Expand Down Expand Up @@ -2547,17 +2550,18 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
GL.validateGLObjectID(GLctx.currentProgram.uniformLocsById, location, 'glUniform4iv', 'location');
assert((value & 3) == 0, 'Pointer to integer data passed to glUniform4iv must be aligned to four bytes!');
#endif
if (!count) return;
#if MIN_WEBGL_VERSION >= 2 && WEBGL_USE_GARBAGE_FREE_APIS
#if GL_ASSERTIONS
assert(GL.currentContext.version >= 2);
#endif
count && GLctx.uniform4iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count*4);
GLctx.uniform4iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count*4);
#else
#if WEBGL_USE_GARBAGE_FREE_APIS
if ({{{ isCurrentContextWebGL2() }}}) {
count && GLctx.uniform4iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count*4);
GLctx.uniform4iv(webglGetUniformLocation(location), HEAP32, {{{ getHeapOffset('value', 'i32') }}}, count*4);
return;
}
#endif
Expand Down Expand Up @@ -2591,17 +2595,18 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
GL.validateGLObjectID(GLctx.currentProgram.uniformLocsById, location, 'glUniform1fv', 'location');
assert((value & 3) == 0, 'Pointer to float data passed to glUniform1fv must be aligned to four bytes!');
#endif
if (!count) return;
#if MIN_WEBGL_VERSION >= 2 && WEBGL_USE_GARBAGE_FREE_APIS
#if GL_ASSERTIONS
assert(GL.currentContext.version >= 2);
#endif
count && GLctx.uniform1fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count);
GLctx.uniform1fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count);
#else
#if WEBGL_USE_GARBAGE_FREE_APIS
if ({{{ isCurrentContextWebGL2() }}}) {
count && GLctx.uniform1fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count);
GLctx.uniform1fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count);
return;
}
#endif
Expand Down Expand Up @@ -2632,17 +2637,18 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
GL.validateGLObjectID(GLctx.currentProgram.uniformLocsById, location, 'glUniform2fv', 'location');
assert((value & 3) == 0, 'Pointer to float data passed to glUniform2fv must be aligned to four bytes!');
#endif
if (!count) return;
#if MIN_WEBGL_VERSION >= 2 && WEBGL_USE_GARBAGE_FREE_APIS
#if GL_ASSERTIONS
assert(GL.currentContext.version >= 2);
#endif
count && GLctx.uniform2fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*2);
GLctx.uniform2fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*2);
#else
#if WEBGL_USE_GARBAGE_FREE_APIS
if ({{{ isCurrentContextWebGL2() }}}) {
count && GLctx.uniform2fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*2);
GLctx.uniform2fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*2);
return;
}
#endif
Expand Down Expand Up @@ -2674,17 +2680,18 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
GL.validateGLObjectID(GLctx.currentProgram.uniformLocsById, location, 'glUniform3fv', 'location');
assert((value % 4) == 0, 'Pointer to float data passed to glUniform3fv must be aligned to four bytes!' + value);
#endif
if (!count) return;
#if MIN_WEBGL_VERSION >= 2 && WEBGL_USE_GARBAGE_FREE_APIS
#if GL_ASSERTIONS
assert(GL.currentContext.version >= 2);
#endif
count && GLctx.uniform3fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*3);
GLctx.uniform3fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*3);
#else
#if WEBGL_USE_GARBAGE_FREE_APIS
if ({{{ isCurrentContextWebGL2() }}}) {
count && GLctx.uniform3fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*3);
GLctx.uniform3fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*3);
return;
}
#endif
Expand Down Expand Up @@ -2717,17 +2724,18 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
GL.validateGLObjectID(GLctx.currentProgram.uniformLocsById, location, 'glUniform4fv', 'location');
assert((value & 3) == 0, 'Pointer to float data passed to glUniform4fv must be aligned to four bytes!');
#endif
if (!count) return;
#if MIN_WEBGL_VERSION >= 2 && WEBGL_USE_GARBAGE_FREE_APIS
#if GL_ASSERTIONS
assert(GL.currentContext.version >= 2);
#endif
count && GLctx.uniform4fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*4);
GLctx.uniform4fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*4);
#else
#if WEBGL_USE_GARBAGE_FREE_APIS
if ({{{ isCurrentContextWebGL2() }}}) {
count && GLctx.uniform4fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*4);
GLctx.uniform4fv(webglGetUniformLocation(location), HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*4);
return;
}
#endif
Expand Down Expand Up @@ -2765,17 +2773,18 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
GL.validateGLObjectID(GLctx.currentProgram.uniformLocsById, location, 'glUniformMatrix2fv', 'location');
assert((value & 3) == 0, 'Pointer to float data passed to glUniformMatrix2fv must be aligned to four bytes!');
#endif
if (!count) return;
#if MIN_WEBGL_VERSION >= 2 && WEBGL_USE_GARBAGE_FREE_APIS
#if GL_ASSERTIONS
assert(GL.currentContext.version >= 2);
#endif
count && GLctx.uniformMatrix2fv(webglGetUniformLocation(location), !!transpose, HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*4);
GLctx.uniformMatrix2fv(webglGetUniformLocation(location), !!transpose, HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*4);
#else
#if WEBGL_USE_GARBAGE_FREE_APIS
if ({{{ isCurrentContextWebGL2() }}}) {
count && GLctx.uniformMatrix2fv(webglGetUniformLocation(location), !!transpose, HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*4);
GLctx.uniformMatrix2fv(webglGetUniformLocation(location), !!transpose, HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*4);
return;
}
#endif
Expand Down Expand Up @@ -2809,17 +2818,18 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
GL.validateGLObjectID(GLctx.currentProgram.uniformLocsById, location, 'glUniformMatrix3fv', 'location');
assert((value & 3) == 0, 'Pointer to float data passed to glUniformMatrix3fv must be aligned to four bytes!');
#endif
if (!count) return;
#if MIN_WEBGL_VERSION >= 2 && WEBGL_USE_GARBAGE_FREE_APIS
#if GL_ASSERTIONS
assert(GL.currentContext.version >= 2);
#endif
count && GLctx.uniformMatrix3fv(webglGetUniformLocation(location), !!transpose, HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*9);
GLctx.uniformMatrix3fv(webglGetUniformLocation(location), !!transpose, HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*9);
#else
#if WEBGL_USE_GARBAGE_FREE_APIS
if ({{{ isCurrentContextWebGL2() }}}) {
count && GLctx.uniformMatrix3fv(webglGetUniformLocation(location), !!transpose, HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*9);
GLctx.uniformMatrix3fv(webglGetUniformLocation(location), !!transpose, HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*9);
return;
}
#endif
Expand Down Expand Up @@ -2858,17 +2868,18 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
GL.validateGLObjectID(GLctx.currentProgram.uniformLocsById, location, 'glUniformMatrix4fv', 'location');
assert((value & 3) == 0, 'Pointer to float data passed to glUniformMatrix4fv must be aligned to four bytes!');
#endif
if (!count) return;
#if MIN_WEBGL_VERSION >= 2 && WEBGL_USE_GARBAGE_FREE_APIS
#if GL_ASSERTIONS
assert(GL.currentContext.version >= 2);
#endif
count && GLctx.uniformMatrix4fv(webglGetUniformLocation(location), !!transpose, HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*16);
GLctx.uniformMatrix4fv(webglGetUniformLocation(location), !!transpose, HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*16);
#else
#if WEBGL_USE_GARBAGE_FREE_APIS
if ({{{ isCurrentContextWebGL2() }}}) {
count && GLctx.uniformMatrix4fv(webglGetUniformLocation(location), !!transpose, HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*16);
GLctx.uniformMatrix4fv(webglGetUniformLocation(location), !!transpose, HEAPF32, {{{ getHeapOffset('value', 'float') }}}, count*16);
return;
}
#endif
Expand Down
8 changes: 4 additions & 4 deletions test/code_size/hello_webgl2_wasm.json
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.html": 569,
"a.html.gz": 379,
"a.js": 4539,
"a.js.gz": 2315,
"a.js": 4541,
"a.js.gz": 2317,
"a.wasm": 10454,
"a.wasm.gz": 6727,
"total": 15562,
"total_gz": 9421
"total": 15564,
"total_gz": 9423
}
8 changes: 4 additions & 4 deletions test/code_size/hello_webgl2_wasm2js.json
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"a.html": 354,
"a.html.gz": 266,
"a.js": 22282,
"a.js.gz": 11608,
"total": 22636,
"total_gz": 11874
"a.js": 22284,
"a.js.gz": 11611,
"total": 22638,
"total_gz": 11877
}
8 changes: 4 additions & 4 deletions test/code_size/hello_webgl_wasm.json
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.html": 569,
"a.html.gz": 379,
"a.js": 4056,
"a.js.gz": 2152,
"a.js": 4061,
"a.js.gz": 2154,
"a.wasm": 10454,
"a.wasm.gz": 6727,
"total": 15079,
"total_gz": 9258
"total": 15084,
"total_gz": 9260
}
8 changes: 4 additions & 4 deletions test/code_size/hello_webgl_wasm2js.json
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"a.html": 354,
"a.html.gz": 266,
"a.js": 21784,
"a.js.gz": 11435,
"total": 22138,
"total_gz": 11701
"a.js": 21789,
"a.js.gz": 11437,
"total": 22143,
"total_gz": 11703
}

0 comments on commit 386965b

Please sign in to comment.