[WebGL] Simulated vertexAttrib0 can sometimes cause OUT_OF_MEMORY errors

https://bugs.webkit.org/show_bug.cgi?id=181558
<rdar://problem/36189833>

Reviewed by Eric Carlson.

Source/WebCore:

Very large element indices in the ELEMENT_ARRAY_BUFFER meant that
our simulated vertexAttrib0 buffer might be too large. We need
to check for out-of-memory, but we can also detect some of the issues
earlier in our validation code. Additionally, make sure that we don't
accidentally cast an unsigned to a signed.

Test: fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html

* html/canvas/WebGL2RenderingContext.cpp:
(WebCore::WebGL2RenderingContext::validateIndexArrayConservative): Update validation
code to look for overflow, rather than relying on looking for sign changes.
* html/canvas/WebGLRenderingContext.cpp:
(WebCore::WebGLRenderingContext::validateIndexArrayConservative): Ditto.
* html/canvas/WebGLRenderingContextBase.cpp:
(WebCore::WebGLRenderingContextBase::validateIndexArrayPrecise):
(WebCore::WebGLRenderingContextBase::drawArrays): Check that we were able to simulate.
(WebCore::WebGLRenderingContextBase::drawElements):
(WebCore::WebGLRenderingContextBase::validateSimulatedVertexAttrib0): Update validation code, and
use GC3Duint, since that's what the indicies are.
(WebCore::WebGLRenderingContextBase::simulateVertexAttrib0): Ditto.
(WebCore::WebGLRenderingContextBase::drawArraysInstanced): Check that we were able to simulate.
(WebCore::WebGLRenderingContextBase::drawElementsInstanced):
* html/canvas/WebGLRenderingContextBase.h:

LayoutTests:

* fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies-expected.txt: Added.
* fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html: Added.
* platform/mac/TestExpectations: Test crashes on Sierra and earlier.

Canonical link: https://commits.webkit.org/197474@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@226916 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,15 @@
+2018-01-11  Dean Jackson  <dino@apple.com>
+
+        [WebGL] Simulated vertexAttrib0 can sometimes cause OUT_OF_MEMORY errors
+        https://bugs.webkit.org/show_bug.cgi?id=181558
+        <rdar://problem/36189833>
+
+        Reviewed by Eric Carlson.
+
+        * fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies-expected.txt: Added.
+        * fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html: Added.
+        * platform/mac/TestExpectations: Test crashes on Sierra and earlier.
+
 2018-01-12  Dean Jackson  <dino@apple.com>
 
         drawElements should be invalid if vertexAttrib0 doesn't have data

LayoutTests/fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies-expected.txt

@@ -0,0 +1,5 @@
+CONSOLE MESSAGE: line 49: WebGL: INVALID_OPERATION: drawElements: attempt to access out of bounds arrays
+CONSOLE MESSAGE: line 59: WebGL: INVALID_OPERATION: drawElements: unable to simulate vertexAttrib0 array
+PASS: MAX_UINT index was unable to be simulated
+PASS: Huge index was unable to be simulated
+

LayoutTests/fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html

@@ -0,0 +1,67 @@
+<canvas id="canvas" width="10" height="10"></canvas>
+<div></div>
+<script id='vertex-shader' type='x-shader/x-vertex'>
+attribute vec3 position;
+void main(void) {
+  gl_Position =  vec4(position, 1.0);
+}
+</script>
+<script id='fragment-shader' type='x-shader/x-fragment'>
+precision mediump float;
+void main(void) {
+    gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0);
+}
+</script>
+<script>
+function output(msg) {
+    document.querySelector("div").innerHTML += msg + "<br>";
+}
+
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+let canvas = document.getElementById("canvas");
+let gl = canvas.getContext("webgl");
+
+gl.getExtension("OES_element_index_uint");
+
+let vShader = gl.createShader(gl.VERTEX_SHADER);
+gl.shaderSource(vShader, document.getElementById("vertex-shader").text);
+gl.compileShader(vShader);
+
+let fShader = gl.createShader(gl.FRAGMENT_SHADER);
+gl.shaderSource(fShader, document.getElementById("fragment-shader").text);
+gl.compileShader(fShader);
+program = gl.createProgram();
+gl.attachShader(program, vShader);
+gl.attachShader(program, fShader);
+gl.linkProgram(program);
+gl.useProgram(program);
+
+let buffer = gl.createBuffer();
+gl.bindBuffer(gl.ELEMENT_ARRAY_BUFFER, buffer);
+
+let data;
+
+// Maximum uint.
+data = new Uint8Array([255, 255, 255, 255]);
+gl.bufferData(gl.ELEMENT_ARRAY_BUFFER, data, gl.DYNAMIC_DRAW);
+gl.drawElements(gl.TRIANGLE_STRIP, 1, gl.UNSIGNED_INT,0);
+
+if (gl.getError() == gl.INVALID_OPERATION)
+    output("PASS: MAX_UINT index was unable to be simulated");
+else
+    output("FAIL: MAX_UINT index did not fail validation");
+
+// Two large numbers, one of which is smaller than 0.25 * max uint.
+data = new Uint32Array([380000000, 4294967295]);
+gl.bufferData(gl.ELEMENT_ARRAY_BUFFER, data, gl.DYNAMIC_DRAW);
+gl.drawElements(gl.TRIANGLE_STRIP, 1, gl.UNSIGNED_INT,0);
+
+if (gl.getError() == gl.INVALID_OPERATION)
+    output("PASS: Huge index was unable to be simulated");
+else
+    output("FAIL: Huge index did not fail validation");
+
+</script>
+

LayoutTests/platform/mac/TestExpectations

@@ -1767,3 +1767,7 @@ webkit.org/b/181502 swipe/pushstate-with-manual-scrollrestoration.html [ Failure
 webkit.org/b/181479 http/tests/misc/slow-loading-animated-image.html [ Pass ImageOnlyFailure ]
 
 webkit.org/b/181494 accessibility/mac/aria-multiple-liveregions-notification.html [ Pass Failure ]
+
+# A lot of GPU hardware simply crashes with this test, since it allocates a lot of memory.
+# It is enabled on systems that instead return GL_OUT_OF_MEMORY.
+[ ElCapitan Sierra ] fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html [ Skip ]

Source/WebCore/ChangeLog

@@ -1,3 +1,35 @@
+2018-01-11  Dean Jackson  <dino@apple.com>
+
+        [WebGL] Simulated vertexAttrib0 can sometimes cause OUT_OF_MEMORY errors
+        https://bugs.webkit.org/show_bug.cgi?id=181558
+        <rdar://problem/36189833>
+
+        Reviewed by Eric Carlson.
+
+        Very large element indices in the ELEMENT_ARRAY_BUFFER meant that
+        our simulated vertexAttrib0 buffer might be too large. We need
+        to check for out-of-memory, but we can also detect some of the issues
+        earlier in our validation code. Additionally, make sure that we don't
+        accidentally cast an unsigned to a signed.
+
+        Test: fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html
+
+        * html/canvas/WebGL2RenderingContext.cpp:
+        (WebCore::WebGL2RenderingContext::validateIndexArrayConservative): Update validation
+        code to look for overflow, rather than relying on looking for sign changes.
+        * html/canvas/WebGLRenderingContext.cpp:
+        (WebCore::WebGLRenderingContext::validateIndexArrayConservative): Ditto.
+        * html/canvas/WebGLRenderingContextBase.cpp:
+        (WebCore::WebGLRenderingContextBase::validateIndexArrayPrecise):
+        (WebCore::WebGLRenderingContextBase::drawArrays): Check that we were able to simulate.
+        (WebCore::WebGLRenderingContextBase::drawElements):
+        (WebCore::WebGLRenderingContextBase::validateSimulatedVertexAttrib0): Update validation code, and
+        use GC3Duint, since that's what the indicies are.
+        (WebCore::WebGLRenderingContextBase::simulateVertexAttrib0): Ditto.
+        (WebCore::WebGLRenderingContextBase::drawArraysInstanced): Check that we were able to simulate.
+        (WebCore::WebGLRenderingContextBase::drawElementsInstanced):
+        * html/canvas/WebGLRenderingContextBase.h:
+
 2018-01-12  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Large in-place attachment elements cause the document width to expand when inserted

Source/WebCore/html/canvas/WebGL2RenderingContext.cpp

@@ -1822,10 +1822,12 @@ bool WebGL2RenderingContext::validateIndexArrayConservative(GC3Denum type, unsig
 
     // The number of required elements is one more than the maximum
     // index that will be accessed.
-    numElementsRequired = maxIndex.value() + 1;
-
-    // Check for overflow.
-    return numElementsRequired > 0;
+    Checked<unsigned, RecordOverflow> checkedNumElementsRequired = Checked<unsigned>(maxIndex.value());
+    checkedNumElementsRequired += 1;
+    if (checkedNumElementsRequired.hasOverflowed())
+        return false;
+    numElementsRequired = checkedNumElementsRequired.unsafeGet();
+    return true;
 }
 
 bool WebGL2RenderingContext::validateBlendEquation(const char* functionName, GC3Denum mode)

Source/WebCore/html/canvas/WebGLRenderingContext.cpp

@@ -720,10 +720,12 @@ bool WebGLRenderingContext::validateIndexArrayConservative(GC3Denum type, unsign
 
     // The number of required elements is one more than the maximum
     // index that will be accessed.
-    numElementsRequired = maxIndex.value() + 1;
-
-    // Check for overflow.
-    return numElementsRequired > 0;
+    Checked<unsigned, RecordOverflow> checkedNumElementsRequired = Checked<unsigned>(maxIndex.value());
+    checkedNumElementsRequired += 1;
+    if (checkedNumElementsRequired.hasOverflowed())
+        return false;
+    numElementsRequired = checkedNumElementsRequired.unsafeGet();
+    return true;
 }
 
 bool WebGLRenderingContext::validateBlendEquation(const char* functionName, GC3Denum mode)

Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp

@@ -1892,8 +1892,12 @@ bool WebGLRenderingContextBase::validateIndexArrayPrecise(GC3Dsizei count, GC3De
     }
 
     // Then set the last index in the index array and make sure it is valid.
-    numElementsRequired = lastIndex + 1;
-    return numElementsRequired > 0;
+    Checked<unsigned, RecordOverflow> checkedNumElementsRequired = Checked<unsigned>(lastIndex);
+    checkedNumElementsRequired += 1;
+    if (checkedNumElementsRequired.hasOverflowed())
+        return false;
+    numElementsRequired = checkedNumElementsRequired.unsafeGet();
+    return true;
 }
 
 bool WebGLRenderingContextBase::validateVertexAttributes(unsigned elementCount, unsigned primitiveCount)
@@ -2117,8 +2121,15 @@ void WebGLRenderingContextBase::drawArrays(GC3Denum mode, GC3Dint first, GC3Dsiz
     clearIfComposited();
 
     bool vertexAttrib0Simulated = false;
-    if (!isGLES2Compliant())
-        vertexAttrib0Simulated = simulateVertexAttrib0(first + count - 1);
+    if (!isGLES2Compliant()) {
+        auto simulateVertexAttrib0Status = simulateVertexAttrib0(first + count - 1);
+        if (!simulateVertexAttrib0Status) {
+            // We were unable to simulate the attribute buffer.
+            synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, "drawArrays", "unable to simulate vertexAttrib0 array");
+            return;
+        }
+        vertexAttrib0Simulated = simulateVertexAttrib0Status.value();
+    }
     bool usesFallbackTexture = false;
     if (!isGLES2NPOTStrict())
         usesFallbackTexture = checkTextureCompleteness("drawArrays", true);
@@ -2147,7 +2158,13 @@ void WebGLRenderingContextBase::drawElements(GC3Denum mode, GC3Dsizei count, GC3
     if (!isGLES2Compliant()) {
         if (!numElements)
             validateIndexArrayPrecise(count, type, static_cast<GC3Dintptr>(offset), numElements);
-        vertexAttrib0Simulated = simulateVertexAttrib0(numElements);
+        auto simulateVertexAttrib0Status = simulateVertexAttrib0(numElements);
+        if (!simulateVertexAttrib0Status) {
+            // We were unable to simulate the attribute buffer.
+            synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, "drawElements", "unable to simulate vertexAttrib0 array");
+            return;
+        }
+        vertexAttrib0Simulated = simulateVertexAttrib0Status.value();
     }
 
     bool usesFallbackTexture = false;
@@ -5684,11 +5701,8 @@ void WebGLRenderingContextBase::initVertexAttrib0()
     m_vertexAttrib0UsedBefore = false;
 }
 
-bool WebGLRenderingContextBase::validateSimulatedVertexAttrib0(GC3Dsizei numVertex)
+bool WebGLRenderingContextBase::validateSimulatedVertexAttrib0(GC3Duint numVertex)
 {
-    if (numVertex < 0)
-        return false;
-
     if (!m_currentProgram)
         return true;
 
@@ -5700,15 +5714,17 @@ bool WebGLRenderingContextBase::validateSimulatedVertexAttrib0(GC3Dsizei numVert
     if (state.enabled)
         return true;
 
-    Checked<GC3Dsizei, RecordOverflow> bufferSize(numVertex);
+    Checked<GC3Duint, RecordOverflow> bufferSize(numVertex);
     bufferSize += 1;
-    bufferSize *= Checked<GC3Dsizei>(4);
+    bufferSize *= Checked<GC3Duint>(4);
+    if (bufferSize.hasOverflowed())
+        return false;
     Checked<GC3Dsizeiptr, RecordOverflow> bufferDataSize(bufferSize);
     bufferDataSize *= Checked<GC3Dsizeiptr>(sizeof(GC3Dfloat));
-    return !bufferDataSize.hasOverflowed();
+    return !bufferDataSize.hasOverflowed() && bufferDataSize.unsafeGet() > 0;
 }
 
-bool WebGLRenderingContextBase::simulateVertexAttrib0(GC3Dsizei numVertex)
+std::optional<bool> WebGLRenderingContextBase::simulateVertexAttrib0(GC3Duint numVertex)
 {
     if (!m_currentProgram)
         return false;
@@ -5724,15 +5740,21 @@ bool WebGLRenderingContextBase::simulateVertexAttrib0(GC3Dsizei numVertex)
     m_vertexAttrib0UsedBefore = true;
     m_context->bindBuffer(GraphicsContext3D::ARRAY_BUFFER, m_vertexAttrib0Buffer->object());
 
-    Checked<GC3Dsizei> bufferSize(numVertex);
+    Checked<GC3Duint> bufferSize(numVertex);
     bufferSize += 1;
-    bufferSize *= Checked<GC3Dsizei>(4);
+    bufferSize *= Checked<GC3Duint>(4);
 
     Checked<GC3Dsizeiptr> bufferDataSize(bufferSize);
     bufferDataSize *= Checked<GC3Dsizeiptr>(sizeof(GC3Dfloat));
 
     if (bufferDataSize.unsafeGet() > m_vertexAttrib0BufferSize) {
+        m_context->moveErrorsToSyntheticErrorList();
         m_context->bufferData(GraphicsContext3D::ARRAY_BUFFER, bufferDataSize.unsafeGet(), 0, GraphicsContext3D::DYNAMIC_DRAW);
+        if (m_context->getError() != GraphicsContext3D::NO_ERROR) {
+            // We were unable to create a buffer.
+            m_vertexAttrib0UsedBefore = false;
+            return std::nullopt;
+        }
         m_vertexAttrib0BufferSize = bufferDataSize.unsafeGet();
         m_forceAttrib0BufferRefill = true;
     }
@@ -5747,7 +5769,7 @@ bool WebGLRenderingContextBase::simulateVertexAttrib0(GC3Dsizei numVertex)
             || attribValue.value[3] != m_vertexAttrib0BufferValue[3])) {
 
         auto bufferData = std::make_unique<GC3Dfloat[]>(bufferSize.unsafeGet());
-        for (GC3Dsizei ii = 0; ii < numVertex + 1; ++ii) {
+        for (GC3Duint ii = 0; ii < numVertex + 1; ++ii) {
             bufferData[ii * 4] = attribValue.value[0];
             bufferData[ii * 4 + 1] = attribValue.value[1];
             bufferData[ii * 4 + 2] = attribValue.value[2];
@@ -6050,8 +6072,15 @@ void WebGLRenderingContextBase::drawArraysInstanced(GC3Denum mode, GC3Dint first
     clearIfComposited();
 
     bool vertexAttrib0Simulated = false;
-    if (!isGLES2Compliant())
-        vertexAttrib0Simulated = simulateVertexAttrib0(first + count - 1);
+    if (!isGLES2Compliant()) {
+        auto simulateVertexAttrib0Status = simulateVertexAttrib0(first + count - 1);
+        if (!simulateVertexAttrib0Status) {
+            // We were unable to simulate the attribute buffer.
+            synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, "drawArraysInstanced", "unable to simulate vertexAttrib0 array");
+            return;
+        }
+        vertexAttrib0Simulated = simulateVertexAttrib0Status.value();
+    }
     if (!isGLES2NPOTStrict())
         checkTextureCompleteness("drawArraysInstanced", true);
 
@@ -6081,7 +6110,13 @@ void WebGLRenderingContextBase::drawElementsInstanced(GC3Denum mode, GC3Dsizei c
     if (!isGLES2Compliant()) {
         if (!numElements)
             validateIndexArrayPrecise(count, type, static_cast<GC3Dintptr>(offset), numElements);
-        vertexAttrib0Simulated = simulateVertexAttrib0(numElements);
+        auto simulateVertexAttrib0Status = simulateVertexAttrib0(numElements);
+        if (!simulateVertexAttrib0Status) {
+            // We were unable to simulate the attribute buffer.
+            synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, "drawArraysInstanced", "unable to simulate vertexAttrib0 array");
+            return;
+        }
+        vertexAttrib0Simulated = simulateVertexAttrib0Status.value();
     }
     if (!isGLES2NPOTStrict())
         checkTextureCompleteness("drawElementsInstanced", true);

Source/WebCore/html/canvas/WebGLRenderingContextBase.h

@@ -792,8 +792,8 @@ class WebGLRenderingContextBase : public GPUBasedCanvasRenderingContext, private
 
     // Helpers for simulating vertexAttrib0.
     void initVertexAttrib0();
-    bool simulateVertexAttrib0(GC3Dsizei numVertex);
-    bool validateSimulatedVertexAttrib0(GC3Dsizei numVertex);
+    std::optional<bool> simulateVertexAttrib0(GC3Duint numVertex);
+    bool validateSimulatedVertexAttrib0(GC3Duint numVertex);
     void restoreStatesAfterVertexAttrib0Simulation();
 
     void dispatchContextLostEvent();