Modified: trunk/Source/WebCore/ChangeLog (226952 => 226953)
--- trunk/Source/WebCore/ChangeLog 2018-01-15 19:16:03 UTC (rev 226952)
+++ trunk/Source/WebCore/ChangeLog 2018-01-15 20:43:39 UTC (rev 226953)
@@ -1,5 +1,28 @@
2018-01-15 Dean Jackson <d...@apple.com>
+ Use a helper function for checked arithmetic in WebGL validation
+ https://bugs.webkit.org/show_bug.cgi?id=181620
+ <rdar://problem/36485879>
+
+ Reviewed by Eric Carlson.
+
+ Eric recommended using a templated helper function to do
+ a common arithmetic check in WebGL validation.
+
+ * html/canvas/WebGL2RenderingContext.cpp:
+ (WebCore::WebGL2RenderingContext::validateIndexArrayConservative):
+ * html/canvas/WebGLRenderingContext.cpp:
+ (WebCore::WebGLRenderingContext::validateIndexArrayConservative):
+ * html/canvas/WebGLRenderingContextBase.cpp:
+ (WebCore::WebGLRenderingContextBase::validateIndexArrayPrecise):
+ (WebCore::WebGLRenderingContextBase::validateDrawArrays):
+ (WebCore::WebGLRenderingContextBase::validateSimulatedVertexAttrib0):
+ (WebCore::WebGLRenderingContextBase::simulateVertexAttrib0):
+ * html/canvas/WebGLRenderingContextBase.h:
+ (WebCore::WebGLRenderingContextBase::checkedAddAndMultiply): New helper.
+
+2018-01-15 Dean Jackson <d...@apple.com>
+
Use traits for animation timing functions
https://bugs.webkit.org/show_bug.cgi?id=181651
<rdar://problem/36525328>
Modified: trunk/Source/WebCore/html/canvas/WebGL2RenderingContext.cpp (226952 => 226953)
--- trunk/Source/WebCore/html/canvas/WebGL2RenderingContext.cpp 2018-01-15 19:16:03 UTC (rev 226952)
+++ trunk/Source/WebCore/html/canvas/WebGL2RenderingContext.cpp 2018-01-15 20:43:39 UTC (rev 226953)
@@ -1822,11 +1822,11 @@
// The number of required elements is one more than the maximum
// index that will be accessed.
- Checked<unsigned, RecordOverflow> checkedNumElementsRequired = Checked<unsigned>(maxIndex.value());
- checkedNumElementsRequired += 1;
- if (checkedNumElementsRequired.hasOverflowed())
+ auto checkedNumElementsRequired = checkedAddAndMultiply<unsigned>(maxIndex.value(), 1, 1);
+ if (!checkedNumElementsRequired)
return false;
- numElementsRequired = checkedNumElementsRequired.unsafeGet();
+ numElementsRequired = checkedNumElementsRequired.value();
+
return true;
}
Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp (226952 => 226953)
--- trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp 2018-01-15 19:16:03 UTC (rev 226952)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp 2018-01-15 20:43:39 UTC (rev 226953)
@@ -718,13 +718,12 @@
if (!maxIndex)
return false;
- // The number of required elements is one more than the maximum
- // index that will be accessed.
- Checked<unsigned, RecordOverflow> checkedNumElementsRequired = Checked<unsigned>(maxIndex.value());
- checkedNumElementsRequired += 1;
- if (checkedNumElementsRequired.hasOverflowed())
+ // The number of required elements is one more than the maximum index that will be accessed.
+ auto checkedNumElementsRequired = checkedAddAndMultiply<unsigned>(maxIndex.value(), 1, 1);
+ if (!checkedNumElementsRequired)
return false;
- numElementsRequired = checkedNumElementsRequired.unsafeGet();
+ numElementsRequired = checkedNumElementsRequired.value();
+
return true;
}
Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp (226952 => 226953)
--- trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp 2018-01-15 19:16:03 UTC (rev 226952)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp 2018-01-15 20:43:39 UTC (rev 226953)
@@ -1892,11 +1892,10 @@
}
// Then set the last index in the index array and make sure it is valid.
- Checked<unsigned, RecordOverflow> checkedNumElementsRequired = Checked<unsigned>(lastIndex);
- checkedNumElementsRequired += 1;
- if (checkedNumElementsRequired.hasOverflowed())
+ auto checkedNumElementsRequired = checkedAddAndMultiply<unsigned>(lastIndex, 1, 1);
+ if (!checkedNumElementsRequired)
return false;
- numElementsRequired = checkedNumElementsRequired.unsafeGet();
+ numElementsRequired = checkedNumElementsRequired.value();
return true;
}
@@ -2010,9 +2009,7 @@
}
// Ensure we have a valid rendering state.
- Checked<GC3Dint, RecordOverflow> checkedFirst(first);
- Checked<GC3Dint, RecordOverflow> checkedCount(count);
- Checked<GC3Dint, RecordOverflow> checkedSum = checkedFirst + checkedCount;
+ Checked<GC3Dint, RecordOverflow> checkedSum = Checked<GC3Dint, RecordOverflow>(first) + Checked<GC3Dint, RecordOverflow>(count);
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");
@@ -5714,12 +5711,11 @@
if (state.enabled)
return true;
- Checked<GC3Duint, RecordOverflow> bufferSize(numVertex);
- bufferSize += 1;
- bufferSize *= Checked<GC3Duint>(4);
- if (bufferSize.hasOverflowed())
+ auto bufferSize = checkedAddAndMultiply<GC3Duint>(numVertex, 1, 4);
+ if (!bufferSize)
return false;
- Checked<GC3Dsizeiptr, RecordOverflow> bufferDataSize(bufferSize);
+
+ Checked<GC3Dsizeiptr, RecordOverflow> bufferDataSize(bufferSize.value());
bufferDataSize *= Checked<GC3Dsizeiptr>(sizeof(GC3Dfloat));
return !bufferDataSize.hasOverflowed() && bufferDataSize.unsafeGet() > 0;
}
@@ -5740,22 +5736,19 @@
m_vertexAttrib0UsedBefore = true;
m_context->bindBuffer(GraphicsContext3D::ARRAY_BUFFER, m_vertexAttrib0Buffer->object());
- Checked<GC3Duint> bufferSize(numVertex);
- bufferSize += 1;
- bufferSize *= Checked<GC3Duint>(4);
+ // We know bufferSize and bufferDataSize won't overflow or go negative, thanks to validateSimulatedVertexAttrib0
+ GC3Duint bufferSize = (numVertex + 1) * 4;
+ GC3Dsizeiptr bufferDataSize = bufferSize * sizeof(GC3Dfloat);
- Checked<GC3Dsizeiptr> bufferDataSize(bufferSize);
- bufferDataSize *= Checked<GC3Dsizeiptr>(sizeof(GC3Dfloat));
-
- if (bufferDataSize.unsafeGet() > m_vertexAttrib0BufferSize) {
+ if (bufferDataSize > m_vertexAttrib0BufferSize) {
m_context->moveErrorsToSyntheticErrorList();
- m_context->bufferData(GraphicsContext3D::ARRAY_BUFFER, bufferDataSize.unsafeGet(), 0, GraphicsContext3D::DYNAMIC_DRAW);
+ m_context->bufferData(GraphicsContext3D::ARRAY_BUFFER, bufferDataSize, 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_vertexAttrib0BufferSize = bufferDataSize;
m_forceAttrib0BufferRefill = true;
}
@@ -5768,7 +5761,7 @@
|| attribValue.value[2] != m_vertexAttrib0BufferValue[2]
|| attribValue.value[3] != m_vertexAttrib0BufferValue[3])) {
- auto bufferData = std::make_unique<GC3Dfloat[]>(bufferSize.unsafeGet());
+ auto bufferData = std::make_unique<GC3Dfloat[]>(bufferSize);
for (GC3Duint ii = 0; ii < numVertex + 1; ++ii) {
bufferData[ii * 4] = attribValue.value[0];
bufferData[ii * 4 + 1] = attribValue.value[1];
@@ -5780,7 +5773,7 @@
m_vertexAttrib0BufferValue[2] = attribValue.value[2];
m_vertexAttrib0BufferValue[3] = attribValue.value[3];
m_forceAttrib0BufferRefill = false;
- m_context->bufferSubData(GraphicsContext3D::ARRAY_BUFFER, 0, bufferDataSize.unsafeGet(), bufferData.get());
+ m_context->bufferSubData(GraphicsContext3D::ARRAY_BUFFER, 0, bufferDataSize, bufferData.get());
}
m_context->vertexAttribPointer(0, 4, GraphicsContext3D::FLOAT, 0, 0, 0);
return true;
Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h (226952 => 226953)
--- trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h 2018-01-15 19:16:03 UTC (rev 226952)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h 2018-01-15 20:43:39 UTC (rev 226953)
@@ -829,6 +829,8 @@
HTMLCanvasElement* htmlCanvas();
OffscreenCanvas* offscreenCanvas();
+ template <typename T> inline std::optional<T> checkedAddAndMultiply(T value, T add, T multiply);
+
private:
bool validateArrayBufferType(const char* functionName, GC3Denum type, std::optional<JSC::TypedArrayType>);
void registerWithWebGLStateTracker();
@@ -840,6 +842,18 @@
Timer m_checkForContextLossHandlingTimer;
};
+template <typename T>
+inline std::optional<T> WebGLRenderingContextBase::checkedAddAndMultiply(T value, T add, T multiply)
+{
+ Checked<T, RecordOverflow> checkedResult = Checked<T>(value);
+ checkedResult += Checked<T>(add);
+ checkedResult *= Checked<T>(multiply);
+ if (checkedResult.hasOverflowed())
+ return std::nullopt;
+
+ return checkedResult.unsafeGet();
+}
+
} // namespace WebCore
SPECIALIZE_TYPE_TRAITS_CANVASRENDERINGCONTEXT(WebCore::WebGLRenderingContextBase, isWebGL())
