Diff
Modified: trunk/LayoutTests/ChangeLog (214085 => 214086)
--- trunk/LayoutTests/ChangeLog 2017-03-17 00:51:01 UTC (rev 214085)
+++ trunk/LayoutTests/ChangeLog 2017-03-17 00:51:02 UTC (rev 214086)
@@ -1,3 +1,13 @@
+2017-03-16 Dean Jackson <[email protected]>
+
+ WebGL: Improve index validation when using uint index values
+ https://bugs.webkit.org/show_bug.cgi?id=169798
+
+ Reviewed by Simon Fraser.
+
+ * fast/canvas/webgl/draw-elements-out-of-bounds-uint-index-expected.txt: Added.
+ * fast/canvas/webgl/draw-elements-out-of-bounds-uint-index.html: Added.
+
2017-03-16 Youenn Fablet <[email protected]>
Wrap legacy WebRTC API in runtime flag
Added: trunk/LayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index-expected.txt (0 => 214086)
--- trunk/LayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index-expected.txt 2017-03-17 00:51:02 UTC (rev 214086)
@@ -0,0 +1,10 @@
+Test of drawElements with out-of-bounds parameters using OES_element_index_uint
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS context.drawElements(context.LINES, 1, context.UNSIGNED_INT, 0) generated expected GL error: INVALID_OPERATION.
+PASS context.drawElements(context.LINES, 1, context.UNSIGNED_INT, 0) generated expected GL error: INVALID_OPERATION.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Property changes on: trunk/LayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index-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/draw-elements-out-of-bounds-uint-index.html (0 => 214086)
--- trunk/LayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index.html (rev 0)
+++ trunk/LayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index.html 2017-03-17 00:51:02 UTC (rev 214086)
@@ -0,0 +1,51 @@
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<div id="description"></div>
+<div id="console"></div>
+
+<script>
+description("Test of drawElements with out-of-bounds parameters using OES_element_index_uint");
+
+if (window.internals)
+ window.internals.settings.setWebGLErrorsToConsoleEnabled(false);
+
+var context = create3DContext();
+var program = loadStandardProgram(context);
+
+context.useProgram(program);
+context.getExtension("OES_element_index_uint");
+
+var buffer = context.createBuffer();
+context.bindBuffer(context.ARRAY_BUFFER, buffer);
+
+var data = "" Uint8Array(0x100);
+context.bufferData(context.ARRAY_BUFFER, data, context.STATIC_DRAW);
+
+buffer = context.createBuffer();
+context.bindBuffer(context.ELEMENT_ARRAY_BUFFER, buffer);
+
+data = "" Uint32Array(new ArrayBuffer(0x10));
+data[0] = 0xffffffff;
+for (let i = 1; i < data.length; i++){
+ data[i] = 1;
+}
+context.bufferData(context.ELEMENT_ARRAY_BUFFER, data, context.STATIC_DRAW);
+
+context.enableVertexAttribArray(0);
+context.enableVertexAttribArray(1);
+context.vertexAttribPointer(0, 1, context.UNSIGNED_BYTE, false, 0, 0);
+context.vertexAttribPointer(1, 1, context.UNSIGNED_BYTE, false, 0, 0);
+
+shouldGenerateGLError(context, context.INVALID_OPERATION, "context.drawElements(context.LINES, 1, context.UNSIGNED_INT, 0)");
+
+data[0] = 0xfffffffd;
+context.bufferData(context.ELEMENT_ARRAY_BUFFER, data, context.STATIC_DRAW);
+
+shouldGenerateGLError(context, context.INVALID_OPERATION, "context.drawElements(context.LINES, 1, context.UNSIGNED_INT, 0)");
+</script>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index.html
___________________________________________________________________
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/html
\ No newline at end of property
Modified: trunk/Source/WebCore/ChangeLog (214085 => 214086)
--- trunk/Source/WebCore/ChangeLog 2017-03-17 00:51:01 UTC (rev 214085)
+++ trunk/Source/WebCore/ChangeLog 2017-03-17 00:51:02 UTC (rev 214086)
@@ -1,3 +1,30 @@
+2017-03-16 Dean Jackson <[email protected]>
+
+ WebGL: Improve index validation when using uint index values
+ https://bugs.webkit.org/show_bug.cgi?id=169798
+
+ Reviewed by Simon Fraser.
+
+ Make sure that we test index validation with the correct type.
+ Also stop using -1 in WebGLBuffer to indicate non-existant values.
+
+ Test: fast/canvas/webgl/draw-elements-out-of-bounds-uint-index.html
+
+ * html/canvas/WebGL2RenderingContext.cpp:
+ (WebCore::WebGL2RenderingContext::validateIndexArrayConservative): Use optional<> and
+ unsigned values.
+ * html/canvas/WebGLBuffer.cpp: Use unsigned for maxIndex (they can't be negative)
+ and optional<> to indicate unknown value.
+ (WebCore::WebGLBuffer::getCachedMaxIndex):
+ (WebCore::WebGLBuffer::setCachedMaxIndex):
+ * html/canvas/WebGLBuffer.h:
+ * html/canvas/WebGLRenderingContext.cpp:
+ (WebCore::WebGLRenderingContext::validateIndexArrayConservative): Use optional<> and
+ unsigned values.
+ * html/canvas/WebGLRenderingContextBase.cpp:
+ (WebCore::WebGLRenderingContextBase::validateVertexAttributes): No need to check if
+ an unsigned value is less than zero.
+
2017-03-16 Simon Fraser <[email protected]>
Improve the system tracing points
Modified: trunk/Source/WebCore/html/canvas/WebGL2RenderingContext.cpp (214085 => 214086)
--- trunk/Source/WebCore/html/canvas/WebGL2RenderingContext.cpp 2017-03-17 00:51:01 UTC (rev 214085)
+++ trunk/Source/WebCore/html/canvas/WebGL2RenderingContext.cpp 2017-03-17 00:51:02 UTC (rev 214086)
@@ -1748,14 +1748,14 @@
auto* buffer = elementArrayBuffer->elementArrayBuffer();
ASSERT(buffer);
- int maxIndex = elementArrayBuffer->getCachedMaxIndex(type);
- if (maxIndex < 0) {
+ std::optional<unsigned> maxIndex = elementArrayBuffer->getCachedMaxIndex(type);
+ if (!maxIndex) {
// Compute the maximum index in the entire buffer for the given type of index.
switch (type) {
case GraphicsContext3D::UNSIGNED_BYTE: {
const GC3Dubyte* p = static_cast<const GC3Dubyte*>(buffer->data());
for (GC3Dsizeiptr i = 0; i < numElements; i++)
- maxIndex = std::max(maxIndex, static_cast<int>(p[i]));
+ maxIndex = maxIndex ? std::max(maxIndex.value(), static_cast<unsigned>(p[i])) : static_cast<unsigned>(p[i]);
break;
}
case GraphicsContext3D::UNSIGNED_SHORT: {
@@ -1762,7 +1762,7 @@
numElements /= sizeof(GC3Dushort);
const GC3Dushort* p = static_cast<const GC3Dushort*>(buffer->data());
for (GC3Dsizeiptr i = 0; i < numElements; i++)
- maxIndex = std::max(maxIndex, static_cast<int>(p[i]));
+ maxIndex = maxIndex ? std::max(maxIndex.value(), static_cast<unsigned>(p[i])) : static_cast<unsigned>(p[i]);
break;
}
case GraphicsContext3D::UNSIGNED_INT: {
@@ -1769,23 +1769,25 @@
numElements /= sizeof(GC3Duint);
const GC3Duint* p = static_cast<const GC3Duint*>(buffer->data());
for (GC3Dsizeiptr i = 0; i < numElements; i++)
- maxIndex = std::max(maxIndex, static_cast<int>(p[i]));
+ maxIndex = maxIndex ? std::max(maxIndex.value(), static_cast<unsigned>(p[i])) : static_cast<unsigned>(p[i]);
break;
}
default:
return false;
}
- elementArrayBuffer->setCachedMaxIndex(type, maxIndex);
+ if (maxIndex)
+ elementArrayBuffer->setCachedMaxIndex(type, maxIndex.value());
}
- if (maxIndex >= 0) {
- // The number of required elements is one more than the maximum
- // index that will be accessed.
- numElementsRequired = maxIndex + 1;
- return true;
- }
-
- return false;
+ if (!maxIndex)
+ return false;
+
+ // 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;
}
bool WebGL2RenderingContext::validateBlendEquation(const char* functionName, GC3Denum mode)
Modified: trunk/Source/WebCore/html/canvas/WebGLBuffer.cpp (214085 => 214086)
--- trunk/Source/WebCore/html/canvas/WebGLBuffer.cpp 2017-03-17 00:51:01 UTC (rev 214085)
+++ trunk/Source/WebCore/html/canvas/WebGLBuffer.cpp 2017-03-17 00:51:02 UTC (rev 214086)
@@ -228,16 +228,16 @@
return m_byteLength;
}
-int WebGLBuffer::getCachedMaxIndex(GC3Denum type)
+std::optional<unsigned> WebGLBuffer::getCachedMaxIndex(GC3Denum type)
{
for (auto& cache : m_maxIndexCache) {
if (cache.type == type)
return cache.maxIndex;
}
- return -1;
+ return std::nullopt;
}
-void WebGLBuffer::setCachedMaxIndex(GC3Denum type, int value)
+void WebGLBuffer::setCachedMaxIndex(GC3Denum type, unsigned value)
{
for (auto& cache : m_maxIndexCache) {
if (cache.type == type) {
Modified: trunk/Source/WebCore/html/canvas/WebGLBuffer.h (214085 => 214086)
--- trunk/Source/WebCore/html/canvas/WebGLBuffer.h 2017-03-17 00:51:01 UTC (rev 214085)
+++ trunk/Source/WebCore/html/canvas/WebGLBuffer.h 2017-03-17 00:51:02 UTC (rev 214086)
@@ -52,11 +52,10 @@
GC3Dsizeiptr byteLength() const;
const JSC::ArrayBuffer* elementArrayBuffer() const { return m_elementArrayBuffer.get(); }
- // Gets the cached max index for the given type. Returns -1 if
- // none has been set.
- int getCachedMaxIndex(GC3Denum type);
+ // Gets the cached max index for the given type if one has been set.
+ std::optional<unsigned> getCachedMaxIndex(GC3Denum type);
// Sets the cached max index for the given type.
- void setCachedMaxIndex(GC3Denum type, int value);
+ void setCachedMaxIndex(GC3Denum type, unsigned value);
GC3Denum getTarget() const { return m_target; }
void setTarget(GC3Denum, bool forWebGL2);
@@ -83,11 +82,10 @@
// that size.
struct MaxIndexCacheEntry {
GC3Denum type;
- int maxIndex;
+ unsigned maxIndex;
};
// OpenGL ES 2.0 only has two valid index types (UNSIGNED_BYTE
- // and UNSIGNED_SHORT), but might as well leave open the
- // possibility of adding others.
+ // and UNSIGNED_SHORT) plus one extension (UNSIGNED_INT).
MaxIndexCacheEntry m_maxIndexCache[4];
unsigned m_nextAvailableCacheEntry { 0 };
Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp (214085 => 214086)
--- trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp 2017-03-17 00:51:01 UTC (rev 214085)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp 2017-03-17 00:51:02 UTC (rev 214086)
@@ -754,14 +754,14 @@
const ArrayBuffer* buffer = elementArrayBuffer->elementArrayBuffer();
ASSERT(buffer);
- int maxIndex = elementArrayBuffer->getCachedMaxIndex(type);
- if (maxIndex < 0) {
+ std::optional<unsigned> maxIndex = elementArrayBuffer->getCachedMaxIndex(type);
+ if (!maxIndex) {
// Compute the maximum index in the entire buffer for the given type of index.
switch (type) {
case GraphicsContext3D::UNSIGNED_BYTE: {
const GC3Dubyte* p = static_cast<const GC3Dubyte*>(buffer->data());
for (GC3Dsizeiptr i = 0; i < numElements; i++)
- maxIndex = std::max(maxIndex, static_cast<int>(p[i]));
+ maxIndex = maxIndex ? std::max(maxIndex.value(), static_cast<unsigned>(p[i])) : static_cast<unsigned>(p[i]);
break;
}
case GraphicsContext3D::UNSIGNED_SHORT: {
@@ -768,7 +768,7 @@
numElements /= sizeof(GC3Dushort);
const GC3Dushort* p = static_cast<const GC3Dushort*>(buffer->data());
for (GC3Dsizeiptr i = 0; i < numElements; i++)
- maxIndex = std::max(maxIndex, static_cast<int>(p[i]));
+ maxIndex = maxIndex ? std::max(maxIndex.value(), static_cast<unsigned>(p[i])) : static_cast<unsigned>(p[i]);
break;
}
case GraphicsContext3D::UNSIGNED_INT: {
@@ -777,23 +777,25 @@
numElements /= sizeof(GC3Duint);
const GC3Duint* p = static_cast<const GC3Duint*>(buffer->data());
for (GC3Dsizeiptr i = 0; i < numElements; i++)
- maxIndex = std::max(maxIndex, static_cast<int>(p[i]));
+ maxIndex = maxIndex ? std::max(maxIndex.value(), static_cast<unsigned>(p[i])) : static_cast<unsigned>(p[i]);
break;
}
default:
return false;
}
- elementArrayBuffer->setCachedMaxIndex(type, maxIndex);
+ if (maxIndex)
+ elementArrayBuffer->setCachedMaxIndex(type, maxIndex.value());
}
- if (maxIndex >= 0) {
- // The number of required elements is one more than the maximum
- // index that will be accessed.
- numElementsRequired = maxIndex + 1;
- return true;
- }
-
- return false;
+ if (!maxIndex)
+ return false;
+
+ // 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;
}
bool WebGLRenderingContext::validateBlendEquation(const char* functionName, GC3Denum mode)
Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp (214085 => 214086)
--- trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp 2017-03-17 00:51:01 UTC (rev 214085)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp 2017-03-17 00:51:02 UTC (rev 214086)
@@ -1781,7 +1781,7 @@
return false;
}
- if (elementCount <= 0)
+ if (!elementCount)
return true;
// Look in each consumed vertex attrib (by the current program).
