Validate that attrib offsets don't overflow buffers.
During draw calls, we wouldn't add the current attrib offset to the
required draw call size when checking attributes. This could lead to
us producing warnings in the D3D11 runtime, and miss returning some
errors.
BUG=angleproject:1339
Change-Id: I03555be396df46f83d96dfb34fbcb145169625e8
Reviewed-on: https://chromium-review.googlesource.com/331807
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp
index 12c7612..c06eb7a 100644
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -68,13 +68,16 @@
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 @@
return false;
}
- if (!ValidateDrawAttribs(context, primcount, static_cast<GLsizei>(indexRangeOut->end)))
+ if (!ValidateDrawAttribs(context, primcount, static_cast<GLint>(indexRangeOut->vertexCount())))
{
return false;
}
diff --git a/src/tests/gl_tests/VertexAttributeTest.cpp b/src/tests/gl_tests/VertexAttributeTest.cpp
index 1f732e7..c9f293b 100644
--- a/src/tests/gl_tests/VertexAttributeTest.cpp
+++ b/src/tests/gl_tests/VertexAttributeTest.cpp
@@ -67,16 +67,61 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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:
diff --git a/src/tests/test_utils/ANGLETest.cpp b/src/tests/test_utils/ANGLETest.cpp
index ee14e6c..b308e11 100644
--- a/src/tests/test_utils/ANGLETest.cpp
+++ b/src/tests/test_utils/ANGLETest.cpp
@@ -65,7 +65,11 @@
} // 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()
{
+ if (mQuadVertexBuffer)
+ {
+ glDeleteBuffers(1, &mQuadVertexBuffer);
+ }
SafeDelete(mEGLWindow);
}
@@ -183,6 +191,58 @@
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);
diff --git a/src/tests/test_utils/ANGLETest.h b/src/tests/test_utils/ANGLETest.h
index 3e73712..b58ee30 100644
--- a/src/tests/test_utils/ANGLETest.h
+++ b/src/tests/test_utils/ANGLETest.h
@@ -114,6 +114,14 @@
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 @@
bool mIgnoreD3D11SDKLayersWarnings;
+ // Used for indexed quad rendering
+ GLuint mQuadVertexBuffer;
+
static OSWindow *mOSWindow;
};