- Revision
- 226814
- Author
- d...@apple.com
- Date
- 2018-01-11 15:57:52 -0800 (Thu, 11 Jan 2018)
Log Message
[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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (226813 => 226814)
--- trunk/LayoutTests/ChangeLog 2018-01-11 23:41:31 UTC (rev 226813)
+++ trunk/LayoutTests/ChangeLog 2018-01-11 23:57:52 UTC (rev 226814)
@@ -1,3 +1,14 @@
+2018-01-11 Dean Jackson <d...@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.
+
2018-01-11 Chris Dumez <cdu...@apple.com>
ASSERTION FAILED: registration in WebCore::SWServerWorker::skipWaiting()
Added: trunk/LayoutTests/fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies-expected.txt (0 => 226814)
--- trunk/LayoutTests/fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies-expected.txt 2018-01-11 23:57:52 UTC (rev 226814)
@@ -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
+
Property changes on: trunk/LayoutTests/fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies-expected.txt
___________________________________________________________________
Added: svn:eol-style
+native
\ No newline at end of property
Added: svn:keywords
+Date Revision
\ No newline at end of property
Added: svn:mime-type
+text/plain
\ No newline at end of property
Added: trunk/LayoutTests/fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html (0 => 226814)
--- trunk/LayoutTests/fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html (rev 0)
+++ trunk/LayoutTests/fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html 2018-01-11 23:57:52 UTC (rev 226814)
@@ -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 = "" 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 = "" 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>
+
Property changes on: trunk/LayoutTests/fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html
___________________________________________________________________
Added: svn:eol-style
+native
\ No newline at end of property
Added: svn:executable
+*
\ No newline at end of property
Added: svn:keywords
+Date Revision
\ No newline at end of property
Added: svn:mime-type
+text/html
\ No newline at end of property
Modified: trunk/Source/WebCore/ChangeLog (226813 => 226814)
--- trunk/Source/WebCore/ChangeLog 2018-01-11 23:41:31 UTC (rev 226813)
+++ trunk/Source/WebCore/ChangeLog 2018-01-11 23:57:52 UTC (rev 226814)
@@ -1,3 +1,35 @@
+2018-01-11 Dean Jackson <d...@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-11 Chris Dumez <cdu...@apple.com>
ASSERTION FAILED: registration in WebCore::SWServerWorker::skipWaiting()
Modified: trunk/Source/WebCore/html/canvas/WebGL2RenderingContext.cpp (226813 => 226814)
--- trunk/Source/WebCore/html/canvas/WebGL2RenderingContext.cpp 2018-01-11 23:41:31 UTC (rev 226813)
+++ trunk/Source/WebCore/html/canvas/WebGL2RenderingContext.cpp 2018-01-11 23:57:52 UTC (rev 226814)
@@ -1822,10 +1822,12 @@
// 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)
Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp (226813 => 226814)
--- trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp 2018-01-11 23:41:31 UTC (rev 226813)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp 2018-01-11 23:57:52 UTC (rev 226814)
@@ -720,10 +720,12 @@
// 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)
Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp (226813 => 226814)
--- trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp 2018-01-11 23:41:31 UTC (rev 226813)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp 2018-01-11 23:57:52 UTC (rev 226814)
@@ -1892,8 +1892,12 @@
}
// 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)
@@ -2115,8 +2119,15 @@
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);
@@ -2145,7 +2156,13 @@
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;
@@ -5682,11 +5699,8 @@
m_vertexAttrib0UsedBefore = false;
}
-bool WebGLRenderingContextBase::validateSimulatedVertexAttrib0(GC3Dsizei numVertex)
+bool WebGLRenderingContextBase::validateSimulatedVertexAttrib0(GC3Duint numVertex)
{
- if (numVertex < 0)
- return false;
-
if (!m_currentProgram)
return true;
@@ -5698,15 +5712,17 @@
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;
@@ -5722,15 +5738,21 @@
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;
}
@@ -5745,7 +5767,7 @@
|| 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];
@@ -6048,8 +6070,15 @@
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);
@@ -6079,7 +6108,13 @@
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);
Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h (226813 => 226814)
--- trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h 2018-01-11 23:41:31 UTC (rev 226813)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h 2018-01-11 23:57:52 UTC (rev 226814)
@@ -792,8 +792,8 @@
// 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();
