Validate that attrib offsets don't overflow buffers.

src/libANGLE/validationES.cpp

@@ -68,13 +68,16 @@ bool ValidateDrawAttribs(ValidationContext *context, GLint primcount, GLint maxV
                     GLint64 attribSize =
                         static_cast<GLint64>(ComputeVertexAttributeTypeSize(attrib));
                     GLint64 attribDataSize = (maxVertexElement - 1) * attribStride + attribSize;
+                    GLint64 attribOffset   = static_cast<GLint64>(attrib.offset);
 
                     // [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 (attribDataSize > buffer->getSize())
+                    if (attribDataSize + attribOffset > buffer->getSize())
                     {
-                        context->recordError(Error(GL_INVALID_OPERATION));
+                        context->recordError(
+                            Error(GL_INVALID_OPERATION,
+                                  "Vertex buffer is not big enough for the draw call"));
                         return false;
                     }
                 }
@@ -2017,7 +2020,7 @@ bool ValidateDrawElements(ValidationContext *context,
         return false;
     }
 
-    if (!ValidateDrawAttribs(context, primcount, static_cast<GLsizei>(indexRangeOut->end)))
+    if (!ValidateDrawAttribs(context, primcount, static_cast<GLint>(indexRangeOut->vertexCount())))
     {
         return false;
     }

src/tests/gl_tests/VertexAttributeTest.cpp

@@ -67,16 +67,61 @@ class VertexAttributeTest : public ANGLETest
         IMMEDIATE,
     };
 
-    struct TestData
-    {
+    struct TestData final : angle::NonCopyable
+    {
+        TestData(GLenum typeIn,
+                 GLboolean normalizedIn,
+                 Source sourceIn,
+                 const void *inputDataIn,
+                 const GLfloat *expectedDataIn)
+            : type(typeIn),
+              normalized(normalizedIn),
+              bufferOffset(0),
+              source(Source::BUFFER),
+              inputData(inputDataIn),
+              expectedData(expectedDataIn)
+        {
+        }
+
         GLenum type;
         GLboolean normalized;
+        size_t bufferOffset;
         Source source;
 
         const void *inputData;
         const GLfloat *expectedData;
     };
 
+    void setupTest(const TestData &test, GLint typeSize)
+    {
+        if (mProgram == 0)
+        {
+            initBasicProgram();
+        }
+
+        if (test.source == Source::BUFFER)
+        {
+            GLsizei dataSize = mVertexCount * TypeStride(test.type) * typeSize;
+            glBindBuffer(GL_ARRAY_BUFFER, mBuffer);
+            glBufferData(GL_ARRAY_BUFFER, dataSize, test.inputData, GL_STATIC_DRAW);
+            glVertexAttribPointer(mTestAttrib, typeSize, test.type, test.normalized, 0,
+                                  reinterpret_cast<GLvoid *>(test.bufferOffset));
+            glBindBuffer(GL_ARRAY_BUFFER, 0);
+        }
+        else
+        {
+            ASSERT_EQ(Source::IMMEDIATE, test.source);
+            glBindBuffer(GL_ARRAY_BUFFER, 0);
+            glVertexAttribPointer(mTestAttrib, typeSize, test.type, test.normalized, 0,
+                                  test.inputData);
+        }
+
+        glVertexAttribPointer(mExpectedAttrib, typeSize, GL_FLOAT, GL_FALSE, 0, test.expectedData);
+
+        glEnableVertexAttribArray(mTestAttrib);
+        glEnableVertexAttribArray(mExpectedAttrib);
+    }
+
     void runTest(const TestData &test)
     {
         // TODO(geofflang): Figure out why this is broken on AMD OpenGL
@@ -100,29 +145,7 @@ class VertexAttributeTest : public ANGLETest
         for (GLint i = 0; i < 4; i++)
         {
             GLint typeSize = i + 1;
-
-            if (test.source == Source::BUFFER)
-            {
-                GLsizei dataSize = mVertexCount * TypeStride(test.type) * typeSize;
-                glBindBuffer(GL_ARRAY_BUFFER, mBuffer);
-                glBufferData(GL_ARRAY_BUFFER, dataSize, test.inputData, GL_STATIC_DRAW);
-                glVertexAttribPointer(mTestAttrib, typeSize, test.type, test.normalized, 0,
-                                      nullptr);
-                glBindBuffer(GL_ARRAY_BUFFER, 0);
-            }
-            else
-            {
-                ASSERT_EQ(Source::IMMEDIATE, test.source);
-                glBindBuffer(GL_ARRAY_BUFFER, 0);
-                glVertexAttribPointer(mTestAttrib, typeSize, test.type, test.normalized, 0,
-                                      test.inputData);
-            }
-
-            glVertexAttribPointer(mExpectedAttrib, typeSize, GL_FLOAT, GL_FALSE, 0,
-                                  test.expectedData);
-
-            glEnableVertexAttribArray(mTestAttrib);
-            glEnableVertexAttribArray(mExpectedAttrib);
+            setupTest(test, typeSize);
 
             drawQuad(mProgram, "position", 0.5f);
 
@@ -250,7 +273,7 @@ TEST_P(VertexAttributeTest, UnsignedByteUnnormalized)
         expectedData[i] = inputData[i];
     }
 
-    TestData data = {GL_UNSIGNED_BYTE, GL_FALSE, Source::IMMEDIATE, inputData, expectedData};
+    TestData data(GL_UNSIGNED_BYTE, GL_FALSE, Source::IMMEDIATE, inputData, expectedData);
     runTest(data);
 }
 
@@ -263,7 +286,7 @@ TEST_P(VertexAttributeTest, UnsignedByteNormalized)
         expectedData[i] = Normalize(inputData[i]);
     }
 
-    TestData data = {GL_UNSIGNED_BYTE, GL_TRUE, Source::IMMEDIATE, inputData, expectedData};
+    TestData data(GL_UNSIGNED_BYTE, GL_TRUE, Source::IMMEDIATE, inputData, expectedData);
     runTest(data);
 }
 
@@ -276,7 +299,7 @@ TEST_P(VertexAttributeTest, ByteUnnormalized)
         expectedData[i] = inputData[i];
     }
 
-    TestData data = {GL_BYTE, GL_FALSE, Source::IMMEDIATE, inputData, expectedData};
+    TestData data(GL_BYTE, GL_FALSE, Source::IMMEDIATE, inputData, expectedData);
     runTest(data);
 }
 
@@ -289,7 +312,7 @@ TEST_P(VertexAttributeTest, ByteNormalized)
         expectedData[i] = Normalize(inputData[i]);
     }
 
-    TestData data = {GL_BYTE, GL_TRUE, Source::IMMEDIATE, inputData, expectedData};
+    TestData data(GL_BYTE, GL_TRUE, Source::IMMEDIATE, inputData, expectedData);
     runTest(data);
 }
 
@@ -302,7 +325,7 @@ TEST_P(VertexAttributeTest, UnsignedShortUnnormalized)
         expectedData[i] = inputData[i];
     }
 
-    TestData data = {GL_UNSIGNED_SHORT, GL_FALSE, Source::IMMEDIATE, inputData, expectedData};
+    TestData data(GL_UNSIGNED_SHORT, GL_FALSE, Source::IMMEDIATE, inputData, expectedData);
     runTest(data);
 }
 
@@ -315,7 +338,7 @@ TEST_P(VertexAttributeTest, UnsignedShortNormalized)
         expectedData[i] = Normalize(inputData[i]);
     }
 
-    TestData data = {GL_UNSIGNED_SHORT, GL_TRUE, Source::IMMEDIATE, inputData, expectedData};
+    TestData data(GL_UNSIGNED_SHORT, GL_TRUE, Source::IMMEDIATE, inputData, expectedData);
     runTest(data);
 }
 
@@ -328,7 +351,7 @@ TEST_P(VertexAttributeTest, ShortUnnormalized)
         expectedData[i] = inputData[i];
     }
 
-    TestData data = {GL_SHORT, GL_FALSE, Source::IMMEDIATE, inputData, expectedData};
+    TestData data(GL_SHORT, GL_FALSE, Source::IMMEDIATE, inputData, expectedData);
     runTest(data);
 }
 
@@ -341,7 +364,7 @@ TEST_P(VertexAttributeTest, ShortNormalized)
         expectedData[i] = Normalize(inputData[i]);
     }
 
-    TestData data = {GL_SHORT, GL_TRUE, Source::IMMEDIATE, inputData, expectedData};
+    TestData data(GL_SHORT, GL_TRUE, Source::IMMEDIATE, inputData, expectedData);
     runTest(data);
 }
 
@@ -362,7 +385,7 @@ TEST_P(VertexAttributeTestES3, IntUnnormalized)
         expectedData[i] = static_cast<GLfloat>(inputData[i]);
     }
 
-    TestData data = {GL_INT, GL_FALSE, Source::BUFFER, inputData, expectedData};
+    TestData data(GL_INT, GL_FALSE, Source::BUFFER, inputData, expectedData);
     runTest(data);
 }
 
@@ -377,7 +400,7 @@ TEST_P(VertexAttributeTestES3, IntNormalized)
         expectedData[i] = Normalize(inputData[i]);
     }
 
-    TestData data = {GL_INT, GL_TRUE, Source::BUFFER, inputData, expectedData};
+    TestData data(GL_INT, GL_TRUE, Source::BUFFER, inputData, expectedData);
     runTest(data);
 }
 
@@ -393,7 +416,7 @@ TEST_P(VertexAttributeTestES3, UnsignedIntUnnormalized)
         expectedData[i] = static_cast<GLfloat>(inputData[i]);
     }
 
-    TestData data = {GL_UNSIGNED_INT, GL_FALSE, Source::BUFFER, inputData, expectedData};
+    TestData data(GL_UNSIGNED_INT, GL_FALSE, Source::BUFFER, inputData, expectedData);
     runTest(data);
 }
 
@@ -409,7 +432,7 @@ TEST_P(VertexAttributeTestES3, UnsignedIntNormalized)
         expectedData[i] = Normalize(inputData[i]);
     }
 
-    TestData data = {GL_UNSIGNED_INT, GL_TRUE, Source::BUFFER, inputData, expectedData};
+    TestData data(GL_UNSIGNED_INT, GL_TRUE, Source::BUFFER, inputData, expectedData);
     runTest(data);
 }
 
@@ -484,6 +507,44 @@ TEST_P(VertexAttributeTest, SimpleBindAttribLocation)
     EXPECT_PIXEL_NEAR(0, 0, 128, 0, 0, 255, 1);
 }
 
+// Verify that drawing with a large out-of-range offset generates INVALID_OPERATION.
+TEST_P(VertexAttributeTest, DrawArraysBufferTooSmall)
+{
+    GLfloat inputData[mVertexCount];
+    GLfloat expectedData[mVertexCount];
+    for (size_t count = 0; count < mVertexCount; ++count)
+    {
+        inputData[count]    = static_cast<GLfloat>(count);
+        expectedData[count] = inputData[count];
+    }
+
+    TestData data(GL_FLOAT, GL_FALSE, Source::BUFFER, inputData, expectedData);
+    data.bufferOffset = mVertexCount * TypeStride(GL_FLOAT);
+
+    setupTest(data, 1);
+    drawQuad(mProgram, "position", 0.5f);
+    EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+}
+
+// Verify that index draw with an out-of-range offset generates INVALID_OPERATION.
+TEST_P(VertexAttributeTest, DrawElementsBufferTooSmall)
+{
+    GLfloat inputData[mVertexCount];
+    GLfloat expectedData[mVertexCount];
+    for (size_t count = 0; count < mVertexCount; ++count)
+    {
+        inputData[count]    = static_cast<GLfloat>(count);
+        expectedData[count] = inputData[count];
+    }
+
+    TestData data(GL_FLOAT, GL_FALSE, Source::BUFFER, inputData, expectedData);
+    data.bufferOffset = (mVertexCount - 3) * TypeStride(GL_FLOAT);
+
+    setupTest(data, 1);
+    drawIndexedQuad(mProgram, "position", 0.5f);
+    EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+}
+
 class VertexAttributeCachingTest : public VertexAttributeTest
 {
   protected:

src/tests/test_utils/ANGLETest.cpp

@@ -65,7 +65,11 @@ std::ostream &operator<<(std::ostream &ostream, const GLColor &color)
 }  // namespace angle
 
 ANGLETest::ANGLETest()
-    : mEGLWindow(nullptr), mWidth(16), mHeight(16), mIgnoreD3D11SDKLayersWarnings(false)
+    : mEGLWindow(nullptr),
+      mWidth(16),
+      mHeight(16),
+      mIgnoreD3D11SDKLayersWarnings(false),
+      mQuadVertexBuffer(0)
 {
     mEGLWindow =
         new EGLWindow(GetParam().majorVersion, GetParam().minorVersion, GetParam().eglParameters);
@@ -73,6 +77,10 @@ ANGLETest::ANGLETest()
 
 ANGLETest::~ANGLETest()
 {
+    if (mQuadVertexBuffer)
+    {
+        glDeleteBuffers(1, &mQuadVertexBuffer);
+    }
     SafeDelete(mEGLWindow);
 }
 
@@ -183,6 +191,58 @@ void ANGLETest::drawQuad(GLuint program,
     glUseProgram(0);
 }
 
+void ANGLETest::drawIndexedQuad(GLuint program,
+                                const std::string &positionAttribName,
+                                GLfloat positionAttribZ)
+{
+    drawIndexedQuad(program, positionAttribName, positionAttribZ, 1.0f);
+}
+
+void ANGLETest::drawIndexedQuad(GLuint program,
+                                const std::string &positionAttribName,
+                                GLfloat positionAttribZ,
+                                GLfloat positionAttribXYScale)
+{
+    GLint positionLocation = glGetAttribLocation(program, positionAttribName.c_str());
+
+    glUseProgram(program);
+
+    GLuint prevBinding = 0;
+    glGetIntegerv(GL_ARRAY_BUFFER_BINDING, reinterpret_cast<GLint *>(&prevBinding));
+
+    if (mQuadVertexBuffer == 0)
+    {
+        glGenBuffers(1, &mQuadVertexBuffer);
+        const GLfloat vertices[] = {
+            -1.0f * positionAttribXYScale, 1.0f * positionAttribXYScale,  positionAttribZ,
+            -1.0f * positionAttribXYScale, -1.0f * positionAttribXYScale, positionAttribZ,
+            1.0f * positionAttribXYScale,  -1.0f * positionAttribXYScale, positionAttribZ,
+            1.0f * positionAttribXYScale,  1.0f * positionAttribXYScale,  positionAttribZ,
+        };
+        glBindBuffer(GL_ARRAY_BUFFER, mQuadVertexBuffer);
+        glBufferData(GL_ARRAY_BUFFER, sizeof(GLfloat) * 3 * 4, vertices, GL_STATIC_DRAW);
+    }
+    else
+    {
+        glBindBuffer(GL_ARRAY_BUFFER, mQuadVertexBuffer);
+    }
+
+    glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, nullptr);
+    glEnableVertexAttribArray(positionLocation);
+    glBindBuffer(GL_ARRAY_BUFFER, prevBinding);
+
+    const GLushort indices[] = {
+        0, 1, 2, 0, 2, 3,
+    };
+
+    glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_SHORT, indices);
+
+    glDisableVertexAttribArray(positionLocation);
+    glVertexAttribPointer(positionLocation, 4, GL_FLOAT, GL_FALSE, 0, NULL);
+
+    glUseProgram(0);
+}
+
 GLuint ANGLETest::compileShader(GLenum type, const std::string &source)
 {
     GLuint shader = glCreateShader(type);

src/tests/test_utils/ANGLETest.h

@@ -114,6 +114,14 @@ class ANGLETest : public ::testing::TestWithParam<angle::PlatformParameters>
                          const std::string &positionAttribName,
                          GLfloat positionAttribZ,
                          GLfloat positionAttribXYScale);
+    void drawIndexedQuad(GLuint program,
+                         const std::string &positionAttribName,
+                         GLfloat positionAttribZ);
+    void drawIndexedQuad(GLuint program,
+                         const std::string &positionAttribName,
+                         GLfloat positionAttribZ,
+                         GLfloat positionAttribXYScale);
+
     static GLuint compileShader(GLenum type, const std::string &source);
     static bool extensionEnabled(const std::string &extName);
     static bool eglClientExtensionEnabled(const std::string &extName);
@@ -154,6 +162,9 @@ class ANGLETest : public ::testing::TestWithParam<angle::PlatformParameters>
 
     bool mIgnoreD3D11SDKLayersWarnings;
 
+    // Used for indexed quad rendering
+    GLuint mQuadVertexBuffer;
+
     static OSWindow *mOSWindow;
 };