Merge r177165 - InstancedArray crashes attempting to draw out of bounds

https://bugs.webkit.org/show_bug.cgi?id=139521 Source/WebCore: Reviewed by Simon Fraser. We were not doing index validation correctly for instancing. Test: fast/canvas/webgl/angle-instanced-arrays-out-of-bounds.html * html/canvas/WebGLRenderingContext.cpp: (WebCore::WebGLRenderingContext::validateVertexAttributes): We need to check the number of instances drawn against the amount of instance data that has been provided, taking into account the number of repeats (the divisor). (WebCore::WebGLRenderingContext::drawArrays): Added some whitespace to make it more clear. (WebCore::WebGLRenderingContext::validateDrawElements): This needs to take a primcount parameter so that it can correctly validate the call (when used from drawElementsInstanced). (WebCore::WebGLRenderingContext::drawElements): New signature to validate. (WebCore::WebGLRenderingContext::drawArraysInstanced): Rearrange this a bit. The primcount validation is already being done by the validateDrawArrays call. Also, there was a bogus UNUSED_PARAM hanging around. (WebCore::WebGLRenderingContext::drawElementsInstanced): Similar rearrangement. Use the primcount parameter. * html/canvas/WebGLRenderingContext.h: LayoutTests: <rdar://problem/17540398> Reviewed by Simon Fraser. This is a copy of the official webgl/1.0.3 test. * platform/mac-mountainlion/fast/canvas/webgl/angle-instanced-arrays-out-of-bounds-expected.txt: Added. This extension is not available on Mountain Lion. * fast/canvas/webgl/angle-instanced-arrays-out-of-bounds-expected.txt: Added. * fast/canvas/webgl/angle-instanced-arrays-out-of-bounds.html: Added. * fast/canvas/webgl/resources/out-of-bounds-test.js: Added. (OutOfBoundsTest): Canonical link: https://commits.webkit.org/154760.281@webkitgtk/2.6 git-svn-id: https://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.6@178351 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jan 13, 2015 · e4b5df0 · e4b5df0
1 parent 28da89d
commit e4b5df0
Show file tree

Hide file tree

Showing 8 changed files with 892 additions and 30 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,19 @@
+2014-12-10  Dean Jackson  <dino@apple.com>
+
+        InstancedArray crashes attempting to draw out of bounds
+        https://bugs.webkit.org/show_bug.cgi?id=139521
+        <rdar://problem/17540398>
+
+        Reviewed by Simon Fraser.
+
+        This is a copy of the official webgl/1.0.3 test.
+
+        * platform/mac-mountainlion/fast/canvas/webgl/angle-instanced-arrays-out-of-bounds-expected.txt: Added. This extension is not available on Mountain Lion.
+        * fast/canvas/webgl/angle-instanced-arrays-out-of-bounds-expected.txt: Added.
+        * fast/canvas/webgl/angle-instanced-arrays-out-of-bounds.html: Added.
+        * fast/canvas/webgl/resources/out-of-bounds-test.js: Added.
+        (OutOfBoundsTest):
+
 2014-12-10  Chris Dumez  <cdumez@apple.com>
 
         http://omfgdogs.info/ only animates when you resize the window

diff --git a/LayoutTests/fast/canvas/webgl/angle-instanced-arrays-out-of-bounds-expected.txt b/LayoutTests/fast/canvas/webgl/angle-instanced-arrays-out-of-bounds-expected.txt
diff --git a/LayoutTests/fast/canvas/webgl/angle-instanced-arrays-out-of-bounds.html b/LayoutTests/fast/canvas/webgl/angle-instanced-arrays-out-of-bounds.html
@@ -0,0 +1,80 @@
+<!--
+
+/*
+** Copyright (c) 2014 The Khronos Group Inc.
+**
+** Permission is hereby granted, free of charge, to any person obtaining a
+** copy of this software and/or associated documentation files (the
+** "Materials"), to deal in the Materials without restriction, including
+** without limitation the rights to use, copy, modify, merge, publish,
+** distribute, sublicense, and/or sell copies of the Materials, and to
+** permit persons to whom the Materials are furnished to do so, subject to
+** the following conditions:
+**
+** The above copyright notice and this permission notice shall be included
+** in all copies or substantial portions of the Materials.
+**
+** THE MATERIALS ARE PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+** EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+** MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+** IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+** CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+** TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+** MATERIALS OR THE USE OR OTHER DEALINGS IN THE MATERIALS.
+*/
+
+-->
+
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<link rel="stylesheet" href="../../../resources/js-test-style.css"/>
+<script src="resources/desktop-gl-constants.js" type="text/javascript"></script>
+<script src="../../../resources/js-test-pre.js"></script>
+<script src="resources/webgl-test.js"> </script>
+<script src="resources/webgl-test-utils.js"></script>
+<script src="resources/out-of-bounds-test.js"></script>
+</head>
+<body>
+<div id="description"></div>
+<canvas id="canvas" style="width: 50px; height: 50px;"></canvas>
+<div id="console"></div>
+
+<script>
+"use strict";
+description("Test of drawArraysInstancedANGLE and drawElementsInstancedANGLE with out-of-bounds parameters");
+
+var wtu = WebGLTestUtils;
+var canvas = document.getElementById("canvas");
+var gl = wtu.create3DContext(canvas);
+var ext = wtu.getExtensionWithKnownPrefixes(gl, "ANGLE_instanced_arrays");
+if (!ext) {
+    testPassed("No ANGLE_instanced_arrays support -- this is legal");
+} else {
+    testPassed("Successfully enabled ANGLE_instanced_arrays extension");
+    debug("");
+    debug("Test with 1 instance without instanced attributes");
+    debug("");
+    OutOfBoundsTest.runDrawArraysTest("ext.drawArraysInstancedANGLE(gl.TRIANGLES, $(offset), $(count), 1)", gl, wtu, ext);
+    debug("");
+    OutOfBoundsTest.runDrawElementsTest("ext.drawElementsInstancedANGLE(gl.TRIANGLES, $(count), $(type), $(offset), 1)", gl, wtu, ext);
+    debug("");
+    debug("Test with 2 instances without instanced attributes");
+    debug("");
+    OutOfBoundsTest.runDrawArraysTest("ext.drawArraysInstancedANGLE(gl.TRIANGLES, $(offset), $(count), 2)", gl, wtu, ext);
+    debug("");
+    OutOfBoundsTest.runDrawElementsTest("ext.drawElementsInstancedANGLE(gl.TRIANGLES, $(count), $(type), $(offset), 2)", gl, wtu, ext);
+    debug("");
+    OutOfBoundsTest.runDrawArraysInstancedTest("ext.drawArraysInstancedANGLE(gl.TRIANGLES, $(offset), $(count), $(primcount))", gl, wtu, ext);
+    debug("");
+    OutOfBoundsTest.runDrawElementsInstancedTest("ext.drawElementsInstancedANGLE(gl.TRIANGLES, $(count), $(type), $(offset), $(primcount))", gl, wtu, ext);
+    debug("");
+}
+
+var successfullyParsed = true;
+</script>
+
+<script src="../../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/canvas/webgl/resources/out-of-bounds-test.js b/LayoutTests/fast/canvas/webgl/resources/out-of-bounds-test.js
diff --git a/...form/mac-mountainlion/fast/canvas/webgl/angle-instanced-arrays-out-of-bounds-expected.txt b/...form/mac-mountainlion/fast/canvas/webgl/angle-instanced-arrays-out-of-bounds-expected.txt
@@ -0,0 +1,9 @@
+Test of drawArraysInstancedANGLE and drawElementsInstancedANGLE with out-of-bounds parameters
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+ PASS No ANGLE_instanced_arrays support -- this is legal
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,29 @@
+2014-12-10  Dean Jackson  <dino@apple.com>
+
+        InstancedArray crashes attempting to draw out of bounds
+        https://bugs.webkit.org/show_bug.cgi?id=139521
+
+        Reviewed by Simon Fraser.
+
+        We were not doing index validation correctly for instancing.
+
+        Test: fast/canvas/webgl/angle-instanced-arrays-out-of-bounds.html
+
+        * html/canvas/WebGLRenderingContext.cpp:
+        (WebCore::WebGLRenderingContext::validateVertexAttributes): We need to check
+        the number of instances drawn against the amount of instance data that has
+        been provided, taking into account the number of repeats (the divisor).
+        (WebCore::WebGLRenderingContext::drawArrays): Added some whitespace to make it more clear.
+        (WebCore::WebGLRenderingContext::validateDrawElements): This needs to take a primcount
+        parameter so that it can correctly validate the call (when used from drawElementsInstanced).
+        (WebCore::WebGLRenderingContext::drawElements): New signature to validate.
+        (WebCore::WebGLRenderingContext::drawArraysInstanced): Rearrange this a bit. The
+        primcount validation is already being done by the validateDrawArrays call. Also, there
+        was a bogus UNUSED_PARAM hanging around.
+        (WebCore::WebGLRenderingContext::drawElementsInstanced): Similar rearrangement. Use
+        the primcount parameter.
+        * html/canvas/WebGLRenderingContext.h:
+
 2014-12-11  Alexey Proskuryakov  <ap@apple.com>
 
         REGRESSION (Async Text Input): Text input method state is not reset when reloading a page

diff --git a/Source/WebCore/html/canvas/WebGLRenderingContext.cpp b/Source/WebCore/html/canvas/WebGLRenderingContext.cpp
@@ -1950,10 +1950,16 @@ bool WebGLRenderingContext::validateVertexAttributes(unsigned elementCount, unsi
                 ASSERT(state.stride > 0);
                 if (bytesRemaining >= state.bytesPerElement)
                     numElements = 1 + (bytesRemaining - state.bytesPerElement) / state.stride;
-                if (!state.divisor)
+                unsigned instancesRequired = 0;
+                if (state.divisor) {
+                    instancesRequired = ceil(static_cast<float>(primitiveCount) / state.divisor);
+                    if (instancesRequired > numElements)
+                        return false;
+                } else {
                     sawNonInstancedAttrib = true;
-                if ((!state.divisor && elementCount > numElements) || (state.divisor && primitiveCount > numElements))
-                    return false;
+                    if (elementCount > numElements)
+                        return false;
+                }
             }
         }
     }
@@ -1977,7 +1983,7 @@ bool WebGLRenderingContext::validateWebGLObject(const char* functionName, WebGLO
     return true;
 }
 
-bool WebGLRenderingContext::validateDrawArrays(const char* functionName, GC3Denum mode, GC3Dint first, GC3Dsizei count, GC3Dsizei primcount)
+bool WebGLRenderingContext::validateDrawArrays(const char* functionName, GC3Denum mode, GC3Dint first, GC3Dsizei count, GC3Dsizei primitiveCount)
 {
     if (isContextLostOrPending() || !validateDrawMode(functionName, mode))
         return false;
@@ -1995,7 +2001,7 @@ bool WebGLRenderingContext::validateDrawArrays(const char* functionName, GC3Denu
         return false;
     }
 
-    if (primcount < 0) {
+    if (primitiveCount < 0) {
         synthesizeGLError(GraphicsContext3D::INVALID_VALUE, functionName, "primcount < 0");
         return false;
     }
@@ -2005,8 +2011,8 @@ bool WebGLRenderingContext::validateDrawArrays(const char* functionName, GC3Denu
         Checked<GC3Dint, RecordOverflow> checkedFirst(first);
         Checked<GC3Dint, RecordOverflow> checkedCount(count);
         Checked<GC3Dint, RecordOverflow> checkedSum = checkedFirst + checkedCount;
-        Checked<GC3Dint, RecordOverflow> checkedPrimCount(primcount);
-        if (checkedSum.hasOverflowed() || checkedPrimCount.hasOverflowed() || !validateVertexAttributes(checkedSum.unsafeGet(), checkedPrimCount.unsafeGet())) {
+        Checked<GC3Dint, RecordOverflow> checkedPrimitiveCount(primitiveCount);
+        if (checkedSum.hasOverflowed() || checkedPrimitiveCount.hasOverflowed() || !validateVertexAttributes(checkedSum.unsafeGet(), checkedPrimitiveCount.unsafeGet())) {
             synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, functionName, "attempt to access out of bounds arrays");
             return false;
         }
@@ -2040,15 +2046,17 @@ void WebGLRenderingContext::drawArrays(GC3Denum mode, GC3Dint first, GC3Dsizei c
         vertexAttrib0Simulated = simulateVertexAttrib0(first + count - 1);
     if (!isGLES2NPOTStrict())
         checkTextureCompleteness("drawArrays", true);
+
     m_context->drawArrays(mode, first, count);
+
     if (!isGLES2Compliant() && vertexAttrib0Simulated)
         restoreStatesAfterVertexAttrib0Simulation();
     if (!isGLES2NPOTStrict())
         checkTextureCompleteness("drawArrays", false);
     markContextChanged();
 }
 
-bool WebGLRenderingContext::validateDrawElements(const char* functionName, GC3Denum mode, GC3Dsizei count, GC3Denum type, long long offset, unsigned& numElements)
+bool WebGLRenderingContext::validateDrawElements(const char* functionName, GC3Denum mode, GC3Dsizei count, GC3Denum type, long long offset, unsigned& numElements, GC3Dsizei primitiveCount)
 {
     if (isContextLostOrPending() || !validateDrawMode(functionName, mode))
         return false;
@@ -2080,6 +2088,11 @@ bool WebGLRenderingContext::validateDrawElements(const char* functionName, GC3De
         return false;
     }
 
+    if (primitiveCount < 0) {
+        synthesizeGLError(GraphicsContext3D::INVALID_VALUE, functionName, "primcount < 0");
+        return false;
+    }
+
     if (!m_boundVertexArrayObject->getElementArrayBuffer()) {
         synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, functionName, "no ELEMENT_ARRAY_BUFFER bound");
         return false;
@@ -2093,8 +2106,16 @@ bool WebGLRenderingContext::validateDrawElements(const char* functionName, GC3De
         }
         if (!count)
             return false;
-        if (!validateIndexArrayConservative(type, numElements) || !validateVertexAttributes(numElements)) {
-            if (!validateIndexArrayPrecise(count, type, static_cast<GC3Dintptr>(offset), numElements) || !validateVertexAttributes(numElements)) {
+
+        Checked<GC3Dint, RecordOverflow> checkedCount(count);
+        Checked<GC3Dint, RecordOverflow> checkedPrimitiveCount(primitiveCount);
+        if (checkedCount.hasOverflowed() || checkedPrimitiveCount.hasOverflowed()) {
+            synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, functionName, "attempt to access out of bounds arrays");
+            return false;
+        }
+
+        if (!validateIndexArrayConservative(type, numElements) || !validateVertexAttributes(numElements, checkedPrimitiveCount.unsafeGet())) {
+            if (!validateIndexArrayPrecise(checkedCount.unsafeGet(), type, static_cast<GC3Dintptr>(offset), numElements) || !validateVertexAttributes(numElements, checkedPrimitiveCount.unsafeGet())) {
                 synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, functionName, "attempt to access out of bounds arrays");
                 return false;
             }
@@ -2120,7 +2141,7 @@ void WebGLRenderingContext::drawElements(GC3Denum mode, GC3Dsizei count, GC3Denu
     UNUSED_PARAM(ec);
 
     unsigned numElements = 0;
-    if (!validateDrawElements("drawElements", mode, count, type, offset, numElements))
+    if (!validateDrawElements("drawElements", mode, count, type, offset, numElements, 0))
         return;
 
     clearIfComposited();
@@ -2133,7 +2154,9 @@ void WebGLRenderingContext::drawElements(GC3Denum mode, GC3Dsizei count, GC3Denu
     }
     if (!isGLES2NPOTStrict())
         checkTextureCompleteness("drawElements", true);
+
     m_context->drawElements(mode, count, type, static_cast<GC3Dintptr>(offset));
+
     if (!isGLES2Compliant() && vertexAttrib0Simulated)
         restoreStatesAfterVertexAttrib0Simulation();
     if (!isGLES2NPOTStrict())
@@ -6176,28 +6199,24 @@ bool WebGLRenderingContext::supportsDrawBuffers()
 
 void WebGLRenderingContext::drawArraysInstanced(GC3Denum mode, GC3Dint first, GC3Dsizei count, GC3Dsizei primcount)
 {
-    if (!validateDrawArrays("drawArraysInstanced", mode, first, count, primcount))
-        return;
-
-    if (primcount < 0) {
-        synthesizeGLError(GraphicsContext3D::INVALID_VALUE, "drawArraysInstanced", "primcount < 0");
-        return;
-    }
-
     if (!primcount) {
         markContextChanged();
         return;
     }
 
+    if (!validateDrawArrays("drawArraysInstanced", mode, first, count, primcount))
+        return;
+
     clearIfComposited();
 
     bool vertexAttrib0Simulated = false;
     if (!isGLES2Compliant())
         vertexAttrib0Simulated = simulateVertexAttrib0(first + count - 1);
     if (!isGLES2NPOTStrict())
         checkTextureCompleteness("drawArraysInstanced", true);
-    UNUSED_PARAM(primcount);
+
     m_context->drawArraysInstanced(mode, first, count, primcount);
+
     if (!isGLES2Compliant() && vertexAttrib0Simulated)
         restoreStatesAfterVertexAttrib0Simulation();
     if (!isGLES2NPOTStrict())
@@ -6207,20 +6226,15 @@ void WebGLRenderingContext::drawArraysInstanced(GC3Denum mode, GC3Dint first, GC
 
 void WebGLRenderingContext::drawElementsInstanced(GC3Denum mode, GC3Dsizei count, GC3Denum type, long long offset, GC3Dsizei primcount)
 {
-    unsigned numElements = 0;
-    if (!validateDrawElements("drawElementsInstanced", mode, count, type, offset, numElements))
-        return;
-
-    if (primcount < 0) {
-        synthesizeGLError(GraphicsContext3D::INVALID_VALUE, "drawElementsInstanced", "primcount < 0");
-        return;
-    }
-
     if (!primcount) {
         markContextChanged();
         return;
     }
 
+    unsigned numElements = 0;
+    if (!validateDrawElements("drawElementsInstanced", mode, count, type, offset, numElements, primcount))
+        return;
+
     clearIfComposited();
 
     bool vertexAttrib0Simulated = false;
@@ -6231,7 +6245,9 @@ void WebGLRenderingContext::drawElementsInstanced(GC3Denum mode, GC3Dsizei count
     }
     if (!isGLES2NPOTStrict())
         checkTextureCompleteness("drawElementsInstanced", true);
+
     m_context->drawElementsInstanced(mode, count, type, static_cast<GC3Dintptr>(offset), primcount);
+
     if (!isGLES2Compliant() && vertexAttrib0Simulated)
         restoreStatesAfterVertexAttrib0Simulation();
     if (!isGLES2NPOTStrict())

diff --git a/Source/WebCore/html/canvas/WebGLRenderingContext.h b/Source/WebCore/html/canvas/WebGLRenderingContext.h
@@ -388,7 +388,7 @@ class WebGLRenderingContext : public CanvasRenderingContext, public ActiveDOMObj
     bool validateWebGLObject(const char*, WebGLObject*);
 
     bool validateDrawArrays(const char* functionName, GC3Denum mode, GC3Dint first, GC3Dsizei count, GC3Dsizei primcount);
-    bool validateDrawElements(const char* functionName, GC3Denum mode, GC3Dsizei count, GC3Denum type, long long offset, unsigned& numElements);
+    bool validateDrawElements(const char* functionName, GC3Denum mode, GC3Dsizei count, GC3Denum type, long long offset, unsigned& numElements, GC3Dsizei primcount);
 
     // Adds a compressed texture format.
     void addCompressedTextureFormat(GC3Denum);