Add tests for the OOB checks for vertex buffers
BUG=angleproject:1523
Change-Id: I9ec9fefc635d0338285b430152586fdd39f227c5
Reviewed-on: https://chromium-review.googlesource.com/422964
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp
index dbddbfb..c9b559f 100644
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -50,87 +50,92 @@
for (size_t attributeIndex = 0; attributeIndex < maxEnabledAttrib; ++attributeIndex)
{
const VertexAttribute &attrib = vertexAttribs[attributeIndex];
- if (program->isAttribLocationActive(attributeIndex) && attrib.enabled)
+ if (!program->isAttribLocationActive(attributeIndex) || !attrib.enabled)
{
- gl::Buffer *buffer = attrib.buffer.get();
+ continue;
+ }
- if (buffer)
- {
- GLint maxVertexElement = 0;
- bool readsData = false;
- if (attrib.divisor == 0)
- {
- readsData = vertexCount > 0;
- maxVertexElement = maxVertex;
- }
- else if (primcount > 0)
- {
- readsData = true;
- maxVertexElement = (primcount - 1) / attrib.divisor;
- }
-
- // If we're drawing zero vertices, we have enough data.
- if (readsData)
- {
- // We do manual overflow checks here instead of using safe_math.h because it was
- // a bottleneck. Thanks to some properties of GL we know inequalities that can
- // help us make the overflow checks faster.
-
- // The max possible attribSize is 16 for a vector of 4 32 bit values.
- constexpr uint64_t kMaxAttribSize = 16;
- constexpr uint64_t kIntMax = std::numeric_limits<int>::max();
- constexpr uint64_t kUint64Max = std::numeric_limits<uint64_t>::max();
-
- // We know attribStride is given as a GLsizei which is typedefed to int.
- // We also know an upper bound for attribSize.
- static_assert(std::is_same<int, GLsizei>::value, "");
- uint64_t attribStride = ComputeVertexAttributeStride(attrib);
- uint64_t attribSize = ComputeVertexAttributeTypeSize(attrib);
- ASSERT(attribStride <= kIntMax && attribSize <= kMaxAttribSize);
-
- // Computing the max offset using uint64_t without attrib.offset is overflow
- // safe. Note: Last vertex element does not take the full stride!
- static_assert(kIntMax * kIntMax < kUint64Max - kMaxAttribSize, "");
- uint64_t attribDataSizeNoOffset = maxVertexElement * attribStride + attribSize;
-
- // An overflow can happen when adding the offset, check for it.
- uint64_t attribOffset = attrib.offset;
- if (attribDataSizeNoOffset > kUint64Max - attrib.offset)
- {
- context->handleError(Error(GL_INVALID_OPERATION, "Integer overflow."));
- return false;
- }
- uint64_t attribDataSizeWithOffset = attribDataSizeNoOffset + attribOffset;
-
- // [OpenGL ES 3.0.2] section 2.9.4 page 40:
- // We can return INVALID_OPERATION if our vertex attribute does not have
- // enough backing data.
- if (attribDataSizeWithOffset > static_cast<uint64_t>(buffer->getSize()))
- {
- context->handleError(
- Error(GL_INVALID_OPERATION,
- "Vertex buffer is not big enough for the draw call"));
- return false;
- }
- }
- }
- else if (webglCompatibility)
+ // If we have no buffer, then we either get an error, or there are no more checks to be done.
+ gl::Buffer *buffer = attrib.buffer.get();
+ if (!buffer)
+ {
+ if (webglCompatibility)
{
// [WebGL 1.0] Section 6.5 Enabled Vertex Attributes and Range Checking
- // If a vertex attribute is enabled as an array via enableVertexAttribArray but no
- // buffer is bound to that attribute via bindBuffer and vertexAttribPointer, then
- // calls to drawArrays or drawElements will generate an INVALID_OPERATION error.
+ // If a vertex attribute is enabled as an array via enableVertexAttribArray but
+ // no buffer is bound to that attribute via bindBuffer and vertexAttribPointer,
+ // then calls to drawArrays or drawElements will generate an INVALID_OPERATION
+ // error.
context->handleError(
Error(GL_INVALID_OPERATION, "An enabled vertex array has no buffer."));
+ return false;
}
- else if (attrib.pointer == NULL)
+ else if (attrib.pointer == nullptr)
{
// This is an application error that would normally result in a crash,
// but we catch it and return an error
- context->handleError(Error(
- GL_INVALID_OPERATION, "An enabled vertex array has no buffer and no pointer."));
+ context->handleError(
+ Error(GL_INVALID_OPERATION,
+ "An enabled vertex array has no buffer and no pointer."));
return false;
}
+ continue;
+ }
+
+ // If we're drawing zero vertices, we have enough data.
+ if (vertexCount <= 0 || primcount <= 0)
+ {
+ continue;
+ }
+
+ GLint maxVertexElement = 0;
+ if (attrib.divisor == 0)
+ {
+ maxVertexElement = maxVertex;
+ }
+ else
+ {
+ maxVertexElement = (primcount - 1) / attrib.divisor;
+ }
+
+ // We do manual overflow checks here instead of using safe_math.h because it was
+ // a bottleneck. Thanks to some properties of GL we know inequalities that can
+ // help us make the overflow checks faster.
+
+ // The max possible attribSize is 16 for a vector of 4 32 bit values.
+ constexpr uint64_t kMaxAttribSize = 16;
+ constexpr uint64_t kIntMax = std::numeric_limits<int>::max();
+ constexpr uint64_t kUint64Max = std::numeric_limits<uint64_t>::max();
+
+ // We know attribStride is given as a GLsizei which is typedefed to int.
+ // We also know an upper bound for attribSize.
+ static_assert(std::is_same<int, GLsizei>::value, "");
+ uint64_t attribStride = ComputeVertexAttributeStride(attrib);
+ uint64_t attribSize = ComputeVertexAttributeTypeSize(attrib);
+ ASSERT(attribStride <= kIntMax && attribSize <= kMaxAttribSize);
+
+ // Computing the max offset using uint64_t without attrib.offset is overflow
+ // safe. Note: Last vertex element does not take the full stride!
+ static_assert(kIntMax * kIntMax < kUint64Max - kMaxAttribSize, "");
+ uint64_t attribDataSizeNoOffset = maxVertexElement * attribStride + attribSize;
+
+ // An overflow can happen when adding the offset, check for it.
+ uint64_t attribOffset = attrib.offset;
+ if (attribDataSizeNoOffset > kUint64Max - attrib.offset)
+ {
+ context->handleError(Error(GL_INVALID_OPERATION, "Integer overflow."));
+ return false;
+ }
+ uint64_t attribDataSizeWithOffset = attribDataSizeNoOffset + attribOffset;
+
+ // [OpenGL ES 3.0.2] section 2.9.4 page 40:
+ // We can return INVALID_OPERATION if our vertex attribute does not have
+ // enough backing data.
+ if (attribDataSizeWithOffset > static_cast<uint64_t>(buffer->getSize()))
+ {
+ context->handleError(Error(GL_INVALID_OPERATION,
+ "Vertex buffer is not big enough for the draw call"));
+ return false;
}
}
diff --git a/src/libGLESv2/entry_points_gles_2_0.cpp b/src/libGLESv2/entry_points_gles_2_0.cpp
index bd43f36..536d276 100644
--- a/src/libGLESv2/entry_points_gles_2_0.cpp
+++ b/src/libGLESv2/entry_points_gles_2_0.cpp
@@ -758,7 +758,7 @@
Context *context = GetValidGlobalContext();
if (context)
{
- if (!ValidateDrawArrays(context, mode, first, count, 0))
+ if (!ValidateDrawArrays(context, mode, first, count, 1))
{
return;
}
@@ -776,7 +776,7 @@
if (context)
{
IndexRange indexRange;
- if (!ValidateDrawElements(context, mode, count, type, indices, 0, &indexRange))
+ if (!ValidateDrawElements(context, mode, count, type, indices, 1, &indexRange))
{
return;
}
diff --git a/src/tests/gl_tests/WebGLCompatibilityTest.cpp b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
index 3e6aed6..c95d791 100644
--- a/src/tests/gl_tests/WebGLCompatibilityTest.cpp
+++ b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
@@ -39,6 +39,10 @@
PFNGLREQUESTEXTENSIONANGLEPROC glRequestExtensionANGLE = nullptr;
};
+class WebGL2CompatibilityTest : public WebGLCompatibilityTest
+{
+};
+
// Context creation would fail if EGL_ANGLE_create_context_webgl_compatibility was not available so
// the GL extension should always be present
TEST_P(WebGLCompatibilityTest, ExtensionStringExposed)
@@ -164,7 +168,7 @@
glUseProgram(program.get());
const auto &vertices = GetQuadVertices();
- glVertexAttribPointer(posLocation, 3, GL_FLOAT, GL_FALSE, 0, vertices.data());
+ glVertexAttribPointer(posLocation, 3, GL_FLOAT, GL_FALSE, 4, vertices.data());
glEnableVertexAttribArray(posLocation);
ASSERT_GL_NO_ERROR();
@@ -300,6 +304,121 @@
EXPECT_GL_ERROR(GL_INVALID_VALUE);
}
+// Test the checks for OOB reads in the vertex buffers, non-instanced version
+TEST_P(WebGLCompatibilityTest, DrawArraysBufferOutOfBoundsNonInstanced)
+{
+ const std::string &vert =
+ "attribute float a_pos;\n"
+ "void main()\n"
+ "{\n"
+ " gl_Position = vec4(a_pos, a_pos, a_pos, 1.0);\n"
+ "}\n";
+
+ const std::string &frag =
+ "precision highp float;\n"
+ "void main()\n"
+ "{\n"
+ " gl_FragColor = vec4(1.0);\n"
+ "}\n";
+
+ ANGLE_GL_PROGRAM(program, vert, frag);
+
+ GLint posLocation = glGetAttribLocation(program.get(), "a_pos");
+ ASSERT_NE(-1, posLocation);
+ glUseProgram(program.get());
+
+ GLBuffer buffer;
+ glBindBuffer(GL_ARRAY_BUFFER, buffer.get());
+ glBufferData(GL_ARRAY_BUFFER, 16, nullptr, GL_STATIC_DRAW);
+
+ glEnableVertexAttribArray(posLocation);
+
+ const uint8_t* zeroOffset = nullptr;
+
+ // Test touching the last element is valid.
+ glVertexAttribPointer(0, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 12);
+ glDrawArrays(GL_POINTS, 0, 4);
+ ASSERT_GL_NO_ERROR();
+
+ // Test touching the last element + 1 is invalid.
+ glVertexAttribPointer(0, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 13);
+ glDrawArrays(GL_POINTS, 0, 4);
+ EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+
+ // Test touching the last element is valid, using a stride.
+ glVertexAttribPointer(0, 1, GL_UNSIGNED_BYTE, GL_FALSE, 2, zeroOffset + 9);
+ glDrawArrays(GL_POINTS, 0, 4);
+ ASSERT_GL_NO_ERROR();
+
+ // Test touching the last element + 1 is invalid, using a stride.
+ glVertexAttribPointer(0, 1, GL_UNSIGNED_BYTE, GL_FALSE, 2, zeroOffset + 10);
+ glDrawArrays(GL_POINTS, 0, 4);
+ EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+
+ // Test any offset is valid if no vertices are drawn.
+ glVertexAttribPointer(0, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 32);
+ glDrawArrays(GL_POINTS, 0, 0);
+ ASSERT_GL_NO_ERROR();
+}
+
+// Test the checks for OOB reads in the vertex buffers, instanced version
+TEST_P(WebGL2CompatibilityTest, DrawArraysBufferOutOfBoundsInstanced)
+{
+ const std::string &vert =
+ "attribute float a_pos;\n"
+ "void main()\n"
+ "{\n"
+ " gl_Position = vec4(a_pos, a_pos, a_pos, 1.0);\n"
+ "}\n";
+
+ const std::string &frag =
+ "precision highp float;\n"
+ "void main()\n"
+ "{\n"
+ " gl_FragColor = vec4(1.0);\n"
+ "}\n";
+
+ ANGLE_GL_PROGRAM(program, vert, frag);
+
+ GLint posLocation = glGetAttribLocation(program.get(), "a_pos");
+ ASSERT_NE(-1, posLocation);
+ glUseProgram(program.get());
+
+ GLBuffer buffer;
+ glBindBuffer(GL_ARRAY_BUFFER, buffer.get());
+ glBufferData(GL_ARRAY_BUFFER, 16, nullptr, GL_STATIC_DRAW);
+
+ glEnableVertexAttribArray(posLocation);
+ glVertexAttribDivisor(posLocation, 1);
+
+ const uint8_t* zeroOffset = nullptr;
+
+ // Test touching the last element is valid.
+ glVertexAttribPointer(0, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 12);
+ glDrawArraysInstanced(GL_POINTS, 0, 1, 4);
+ ASSERT_GL_NO_ERROR();
+
+ // Test touching the last element + 1 is invalid.
+ glVertexAttribPointer(0, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 13);
+ glDrawArraysInstanced(GL_POINTS, 0, 1, 4);
+ EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+
+ // Test touching the last element is valid, using a stride.
+ glVertexAttribPointer(0, 1, GL_UNSIGNED_BYTE, GL_FALSE, 2, zeroOffset + 9);
+ glDrawArraysInstanced(GL_POINTS, 0, 1, 4);
+ ASSERT_GL_NO_ERROR();
+
+ // Test touching the last element + 1 is invalid, using a stride.
+ glVertexAttribPointer(0, 1, GL_UNSIGNED_BYTE, GL_FALSE, 2, zeroOffset + 10);
+ glDrawArraysInstanced(GL_POINTS, 0, 1, 4);
+ EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+
+ // Test any offset is valid if no vertices are drawn.
+ glVertexAttribPointer(0, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 32);
+ glDrawArraysInstanced(GL_POINTS, 0, 1, 0);
+ ASSERT_GL_NO_ERROR();
+}
+
// Use this to select which configurations (e.g. which renderer, which GLES major version) these
// tests should be run against.
ANGLE_INSTANTIATE_TEST(WebGLCompatibilityTest,
@@ -312,4 +431,9 @@
ES2_OPENGLES(),
ES3_OPENGLES());
+ANGLE_INSTANTIATE_TEST(WebGL2CompatibilityTest,
+ ES3_D3D11(),
+ ES3_OPENGL(),
+ ES3_OPENGLES());
+
} // namespace