Add tests for the OOB checks for vertex buffers

src/libANGLE/validationES.cpp

@@ -50,87 +50,92 @@ bool ValidateDrawAttribs(ValidationContext *context,
     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();
-
-            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;
+            continue;
+        }
 
-                    // [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;
         }
     }
 

src/libGLESv2/entry_points_gles_2_0.cpp

@@ -758,7 +758,7 @@ void GL_APIENTRY DrawArrays(GLenum mode, GLint first, GLsizei count)
     Context *context = GetValidGlobalContext();
     if (context)
     {
-        if (!ValidateDrawArrays(context, mode, first, count, 0))
+        if (!ValidateDrawArrays(context, mode, first, count, 1))
         {
             return;
         }
@@ -776,7 +776,7 @@ void GL_APIENTRY DrawElements(GLenum mode, GLsizei count, GLenum type, const GLv
     if (context)
     {
         IndexRange indexRange;
-        if (!ValidateDrawElements(context, mode, count, type, indices, 0, &indexRange))
+        if (!ValidateDrawElements(context, mode, count, type, indices, 1, &indexRange))
         {
             return;
         }

src/tests/gl_tests/WebGLCompatibilityTest.cpp

@@ -39,6 +39,10 @@ class WebGLCompatibilityTest : public ANGLETest
     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 @@ TEST_P(WebGLCompatibilityTest, ForbidsClientSideArrayBuffer)
     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 @@ TEST_P(WebGLCompatibilityTest, MaxStride)
     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 @@ ANGLE_INSTANTIATE_TEST(WebGLCompatibilityTest,
                        ES2_OPENGLES(),
                        ES3_OPENGLES());
 
+ANGLE_INSTANTIATE_TEST(WebGL2CompatibilityTest,
+                       ES3_D3D11(),
+                       ES3_OPENGL(),
+                       ES3_OPENGLES());
+
 }  // namespace