Check that transform feedback will not overflow its buffers.
Also fix the check for uniform buffer size to use the actual buffer
size instead of the size of the bound range.
Bug: 820639
Change-Id: Iaa2a617ee7ce5ce7cfabbf64bd1d6f8c82c46b65
Reviewed-on: https://chromium-review.googlesource.com/965627
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/libANGLE/Context.cpp b/src/libANGLE/Context.cpp
index 570a961..9140870 100644
--- a/src/libANGLE/Context.cpp
+++ b/src/libANGLE/Context.cpp
@@ -129,20 +129,13 @@
}
}
-void MarkTransformFeedbackBufferUsage(gl::TransformFeedback *transformFeedback)
+void MarkTransformFeedbackBufferUsage(gl::TransformFeedback *transformFeedback,
+ GLsizei count,
+ GLsizei instanceCount)
{
if (transformFeedback && transformFeedback->isActive() && !transformFeedback->isPaused())
{
- for (size_t tfBufferIndex = 0; tfBufferIndex < transformFeedback->getIndexedBufferCount();
- tfBufferIndex++)
- {
- const gl::OffsetBindingPointer<gl::Buffer> &buffer =
- transformFeedback->getIndexedBuffer(tfBufferIndex);
- if (buffer.get() != nullptr)
- {
- buffer->onTransformFeedback();
- }
- }
+ transformFeedback->onVerticesDrawn(count, instanceCount);
}
}
@@ -1811,7 +1804,7 @@
ANGLE_CONTEXT_TRY(prepareForDraw());
ANGLE_CONTEXT_TRY(mImplementation->drawArrays(this, mode, first, count));
- MarkTransformFeedbackBufferUsage(mGLState.getCurrentTransformFeedback());
+ MarkTransformFeedbackBufferUsage(mGLState.getCurrentTransformFeedback(), count, 1);
}
void Context::drawArraysInstanced(GLenum mode, GLint first, GLsizei count, GLsizei instanceCount)
@@ -1825,7 +1818,7 @@
ANGLE_CONTEXT_TRY(prepareForDraw());
ANGLE_CONTEXT_TRY(
mImplementation->drawArraysInstanced(this, mode, first, count, instanceCount));
- MarkTransformFeedbackBufferUsage(mGLState.getCurrentTransformFeedback());
+ MarkTransformFeedbackBufferUsage(mGLState.getCurrentTransformFeedback(), count, instanceCount);
}
void Context::drawElements(GLenum mode, GLsizei count, GLenum type, const void *indices)
diff --git a/src/libANGLE/ErrorStrings.h b/src/libANGLE/ErrorStrings.h
index 3e62b67..1c6e4e4 100644
--- a/src/libANGLE/ErrorStrings.h
+++ b/src/libANGLE/ErrorStrings.h
@@ -189,6 +189,7 @@
ERRMSG(TransformFeedbackBufferDoubleBound,
"A transform feedback buffer that would be written to is also bound to a "
"non-transform-feedback target, which would cause undefined behavior.");
+ERRMSG(TransformFeedbackBufferTooSmall, "Not enough space in bound transform feedback buffers.");
ERRMSG(TransformFeedbackDoesNotExist, "Transform feedback object that does not exist.");
ERRMSG(TypeMismatch,
"Passed in texture target and format must match the one originally used to define the "
diff --git a/src/libANGLE/MemoryProgramCache.cpp b/src/libANGLE/MemoryProgramCache.cpp
index e27d426..83d7348 100644
--- a/src/libANGLE/MemoryProgramCache.cpp
+++ b/src/libANGLE/MemoryProgramCache.cpp
@@ -428,6 +428,8 @@
"Too many shader types");
state->mLinkedShaderStages = stream.readInt<unsigned long>();
+ state->updateTransformFeedbackStrides();
+
return program->getImplementation()->load(context, infoLog, &stream);
}
diff --git a/src/libANGLE/Program.cpp b/src/libANGLE/Program.cpp
index d3f237d..ee934e0 100644
--- a/src/libANGLE/Program.cpp
+++ b/src/libANGLE/Program.cpp
@@ -1229,6 +1229,30 @@
}
}
+void ProgramState::updateTransformFeedbackStrides()
+{
+ if (mTransformFeedbackBufferMode == GL_INTERLEAVED_ATTRIBS)
+ {
+ mTransformFeedbackStrides.resize(1);
+ size_t totalSize = 0;
+ for (auto &varying : mLinkedTransformFeedbackVaryings)
+ {
+ totalSize += varying.size() * VariableExternalSize(varying.type);
+ }
+ mTransformFeedbackStrides[0] = static_cast<GLsizei>(totalSize);
+ }
+ else
+ {
+ mTransformFeedbackStrides.resize(mLinkedTransformFeedbackVaryings.size());
+ for (size_t i = 0; i < mLinkedTransformFeedbackVaryings.size(); i++)
+ {
+ auto &varying = mLinkedTransformFeedbackVaryings[i];
+ mTransformFeedbackStrides[i] =
+ static_cast<GLsizei>(varying.size() * VariableExternalSize(varying.type));
+ }
+ }
+}
+
// Returns the program object to an unlinked state, before re-linking, or at destruction
void Program::unlink()
{
@@ -3229,6 +3253,7 @@
}
}
}
+ mState.updateTransformFeedbackStrides();
}
ProgramMergedVaryings Program::getMergedVaryings(const Context *context) const
diff --git a/src/libANGLE/Program.h b/src/libANGLE/Program.h
index 97c6dee..b7cba58 100644
--- a/src/libANGLE/Program.h
+++ b/src/libANGLE/Program.h
@@ -358,6 +358,8 @@
friend class MemoryProgramCache;
friend class Program;
+ void updateTransformFeedbackStrides();
+
std::string mLabel;
sh::WorkGroupSize mComputeShaderLocalSize;
@@ -431,6 +433,9 @@
GLenum mGeometryShaderOutputPrimitiveType;
int mGeometryShaderInvocations;
int mGeometryShaderMaxVertices;
+
+ // The size of the data written to each transform feedback buffer per vertex.
+ std::vector<GLsizei> mTransformFeedbackStrides;
};
class ProgramBindings final : angle::NonCopyable
@@ -690,6 +695,11 @@
ComponentTypeMask getAttributesTypeMask() const { return mState.mAttributesTypeMask; }
AttributesMask getAttributesMask() const { return mState.mAttributesMask; }
+ const std::vector<GLsizei> &getTransformFeedbackStrides() const
+ {
+ return mState.mTransformFeedbackStrides;
+ }
+
private:
~Program() override;
diff --git a/src/libANGLE/TransformFeedback.cpp b/src/libANGLE/TransformFeedback.cpp
index 73bd71f..856714f 100644
--- a/src/libANGLE/TransformFeedback.cpp
+++ b/src/libANGLE/TransformFeedback.cpp
@@ -6,6 +6,7 @@
#include "libANGLE/TransformFeedback.h"
+#include "common/mathutil.h"
#include "libANGLE/Buffer.h"
#include "libANGLE/Caps.h"
#include "libANGLE/Context.h"
@@ -14,14 +15,44 @@
#include "libANGLE/renderer/GLImplFactory.h"
#include "libANGLE/renderer/TransformFeedbackImpl.h"
+#include <limits>
+
namespace gl
{
+angle::CheckedNumeric<GLsizeiptr> GetVerticesNeededForDraw(GLenum primitiveMode,
+ GLsizei count,
+ GLsizei primcount)
+{
+ if (count < 0 || primcount < 0)
+ {
+ return 0;
+ }
+ // Transform feedback only outputs complete primitives, so we need to round down to the nearest
+ // complete primitive before multiplying by the number of instances.
+ angle::CheckedNumeric<GLsizeiptr> checkedCount = count;
+ angle::CheckedNumeric<GLsizeiptr> checkedPrimcount = primcount;
+ switch (primitiveMode)
+ {
+ case GL_TRIANGLES:
+ return checkedPrimcount * (checkedCount - checkedCount % 3);
+ case GL_LINES:
+ return checkedPrimcount * (checkedCount - checkedCount % 2);
+ case GL_POINTS:
+ return checkedPrimcount * checkedCount;
+ default:
+ NOTREACHED();
+ return checkedPrimcount * checkedCount;
+ }
+}
+
TransformFeedbackState::TransformFeedbackState(size_t maxIndexedBuffers)
: mLabel(),
mActive(false),
mPrimitiveMode(GL_NONE),
mPaused(false),
+ mVerticesDrawn(0),
+ mVertexCapacity(0),
mProgram(nullptr),
mIndexedBuffers(maxIndexedBuffers)
{
@@ -87,15 +118,37 @@
mState.mActive = true;
mState.mPrimitiveMode = primitiveMode;
mState.mPaused = false;
+ mState.mVerticesDrawn = 0;
mImplementation->begin(primitiveMode);
bindProgram(context, program);
+
+ if (program)
+ {
+ // Compute the number of vertices we can draw before overflowing the bound buffers.
+ auto strides = program->getTransformFeedbackStrides();
+ ASSERT(strides.size() <= mState.mIndexedBuffers.size() && !strides.empty());
+ GLsizeiptr minCapacity = std::numeric_limits<GLsizeiptr>::max();
+ for (size_t index = 0; index < strides.size(); index++)
+ {
+ GLsizeiptr capacity =
+ GetBoundBufferAvailableSize(mState.mIndexedBuffers[index]) / strides[index];
+ minCapacity = std::min(minCapacity, capacity);
+ }
+ mState.mVertexCapacity = minCapacity;
+ }
+ else
+ {
+ mState.mVertexCapacity = 0;
+ }
}
void TransformFeedback::end(const Context *context)
{
- mState.mActive = false;
- mState.mPrimitiveMode = GL_NONE;
- mState.mPaused = false;
+ mState.mActive = false;
+ mState.mPrimitiveMode = GL_NONE;
+ mState.mPaused = false;
+ mState.mVerticesDrawn = 0;
+ mState.mVertexCapacity = 0;
mImplementation->end();
if (mState.mProgram)
{
@@ -131,6 +184,30 @@
return mState.mPrimitiveMode;
}
+bool TransformFeedback::checkBufferSpaceForDraw(GLsizei count, GLsizei primcount) const
+{
+ auto vertices =
+ mState.mVerticesDrawn + GetVerticesNeededForDraw(mState.mPrimitiveMode, count, primcount);
+ return vertices.IsValid() && vertices.ValueOrDie() <= mState.mVertexCapacity;
+}
+
+void TransformFeedback::onVerticesDrawn(GLsizei count, GLsizei primcount)
+{
+ ASSERT(mState.mActive && !mState.mPaused);
+ // All draws should be validated with checkBufferSpaceForDraw so ValueOrDie should never fail.
+ mState.mVerticesDrawn =
+ (mState.mVerticesDrawn + GetVerticesNeededForDraw(mState.mPrimitiveMode, count, primcount))
+ .ValueOrDie();
+
+ for (auto &buffer : mState.mIndexedBuffers)
+ {
+ if (buffer.get() != nullptr)
+ {
+ buffer->onTransformFeedback();
+ }
+ }
+}
+
void TransformFeedback::bindProgram(const Context *context, Program *program)
{
if (mState.mProgram != program)
diff --git a/src/libANGLE/TransformFeedback.h b/src/libANGLE/TransformFeedback.h
index 311a560..5e650f5 100644
--- a/src/libANGLE/TransformFeedback.h
+++ b/src/libANGLE/TransformFeedback.h
@@ -44,6 +44,8 @@
bool mActive;
GLenum mPrimitiveMode;
bool mPaused;
+ GLsizeiptr mVerticesDrawn;
+ GLsizeiptr mVertexCapacity;
Program *mProgram;
@@ -68,6 +70,14 @@
bool isActive() const;
bool isPaused() const;
GLenum getPrimitiveMode() const;
+ // Validates that the vertices produced by a draw call will fit in the bound transform feedback
+ // buffers.
+ bool checkBufferSpaceForDraw(GLsizei count, GLsizei primcount) const;
+ // This must be called after each draw call when transform feedback is enabled to keep track of
+ // how many vertices have been written to the buffers. This information is needed by
+ // checkBufferSpaceForDraw because each draw call appends vertices to the buffers starting just
+ // after the last vertex of the previous draw call.
+ void onVerticesDrawn(GLsizei count, GLsizei primcount);
bool hasBoundProgram(GLuint program) const;
diff --git a/src/libANGLE/angletypes.cpp b/src/libANGLE/angletypes.cpp
index dfecf75..e46f1f8 100644
--- a/src/libANGLE/angletypes.cpp
+++ b/src/libANGLE/angletypes.cpp
@@ -342,4 +342,33 @@
return (outputTypes & inputMask) == ((inputTypes & outputMask) & inputMask);
}
+GLsizeiptr GetBoundBufferAvailableSize(const OffsetBindingPointer<Buffer> &binding)
+{
+ Buffer *buffer = binding.get();
+ if (buffer)
+ {
+ if (binding.getSize() == 0)
+ return static_cast<GLsizeiptr>(buffer->getSize());
+ angle::CheckedNumeric<GLintptr> offset = binding.getOffset();
+ angle::CheckedNumeric<GLsizeiptr> size = binding.getSize();
+ angle::CheckedNumeric<GLsizeiptr> bufferSize = buffer->getSize();
+ auto end = offset + size;
+ auto clampedSize = size;
+ auto difference = end - bufferSize;
+ if (!difference.IsValid())
+ {
+ return 0;
+ }
+ if (difference.ValueOrDie() > 0)
+ {
+ clampedSize = size - difference;
+ }
+ return clampedSize.ValueOrDefault(0);
+ }
+ else
+ {
+ return 0;
+ }
+}
+
} // namespace gl
diff --git a/src/libANGLE/angletypes.h b/src/libANGLE/angletypes.h
index 229d0ef..405ad4f 100644
--- a/src/libANGLE/angletypes.h
+++ b/src/libANGLE/angletypes.h
@@ -331,6 +331,11 @@
template <typename T>
using AttribArray = std::array<T, MAX_VERTEX_ATTRIBS>;
+// OffsetBindingPointer.getSize() returns the size specified by the user, which may be larger than
+// the size of the bound buffer. This function reduces the returned size to fit the bound buffer if
+// necessary. Returns 0 if no buffer is bound or if integer overflow occurs.
+GLsizeiptr GetBoundBufferAvailableSize(const OffsetBindingPointer<Buffer> &binding);
+
} // namespace gl
namespace rx
diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp
index a8d2fe3..ada0959 100644
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -19,6 +19,7 @@
#include "libANGLE/Texture.h"
#include "libANGLE/TransformFeedback.h"
#include "libANGLE/VertexArray.h"
+#include "libANGLE/angletypes.h"
#include "libANGLE/formatutils.h"
#include "libANGLE/queryconversions.h"
#include "libANGLE/validationES2.h"
@@ -2765,13 +2766,7 @@
return false;
}
- size_t uniformBufferSize = uniformBuffer.getSize();
- if (uniformBufferSize == 0)
- {
- // Bind the whole buffer.
- uniformBufferSize = static_cast<size_t>(uniformBuffer->getSize());
- }
-
+ size_t uniformBufferSize = GetBoundBufferAvailableSize(uniformBuffer);
if (uniformBufferSize < uniformBlock.dataSize)
{
// undefined behaviour
@@ -2838,14 +2833,23 @@
const State &state = context->getGLState();
gl::TransformFeedback *curTransformFeedback = state.getCurrentTransformFeedback();
if (curTransformFeedback && curTransformFeedback->isActive() &&
- !curTransformFeedback->isPaused() && curTransformFeedback->getPrimitiveMode() != mode)
+ !curTransformFeedback->isPaused())
{
- // It is an invalid operation to call DrawArrays or DrawArraysInstanced with a draw mode
- // that does not match the current transform feedback object's draw mode (if transform
- // feedback
- // is active), (3.0.2, section 2.14, pg 86)
- ANGLE_VALIDATION_ERR(context, InvalidOperation(), InvalidDrawModeTransformFeedback);
- return false;
+ if (curTransformFeedback->getPrimitiveMode() != mode)
+ {
+ // It is an invalid operation to call DrawArrays or DrawArraysInstanced with a draw mode
+ // that does not match the current transform feedback object's draw mode (if transform
+ // feedback
+ // is active), (3.0.2, section 2.14, pg 86)
+ ANGLE_VALIDATION_ERR(context, InvalidOperation(), InvalidDrawModeTransformFeedback);
+ return false;
+ }
+
+ if (!curTransformFeedback->checkBufferSpaceForDraw(count, primcount))
+ {
+ ANGLE_VALIDATION_ERR(context, InvalidOperation(), TransformFeedbackBufferTooSmall);
+ return false;
+ }
}
if (!ValidateDrawBase(context, mode, count))
diff --git a/src/tests/gl_tests/TransformFeedbackTest.cpp b/src/tests/gl_tests/TransformFeedbackTest.cpp
index 2e56147..e0471be 100644
--- a/src/tests/gl_tests/TransformFeedbackTest.cpp
+++ b/src/tests/gl_tests/TransformFeedbackTest.cpp
@@ -286,6 +286,66 @@
EXPECT_GL_NO_ERROR();
}
+// Test that XFB does not allow writing more vertices than fit in the bound buffers.
+// TODO(jmadill): Enable this test after fixing the last case where the buffer size changes after
+// calling glBeginTransformFeedback.
+TEST_P(TransformFeedbackTest, DISABLED_TooSmallBuffers)
+{
+ glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
+ glClear(GL_COLOR_BUFFER_BIT);
+ glEnable(GL_RASTERIZER_DISCARD);
+
+ // Set the program's transform feedback varyings (just gl_Position)
+ std::vector<std::string> tfVaryings;
+ tfVaryings.push_back("gl_Position");
+ compileDefaultProgram(tfVaryings, GL_INTERLEAVED_ATTRIBS);
+ GLint positionLocation = glGetAttribLocation(mProgram, "position");
+
+ glUseProgram(mProgram);
+
+ const GLfloat vertices[] = {
+ -1.0f, 1.0f, 0.5f, -1.0f, -1.0f, 0.5f, 1.0f, -1.0f, 0.5f,
+
+ -1.0f, 1.0f, 0.5f, 1.0f, -1.0f, 0.5f, 1.0f, 1.0f, 0.5f,
+ };
+
+ glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, vertices);
+ glEnableVertexAttribArray(positionLocation);
+
+ const size_t verticesToDraw = 6;
+ const size_t stride = sizeof(float) * 4;
+ const size_t bytesNeeded = stride * verticesToDraw;
+
+ glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, mTransformFeedbackBuffer);
+
+ // Set up the buffer to be the right size
+ uint8_t tfData[stride * verticesToDraw] = {0};
+ glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, bytesNeeded, &tfData, GL_STATIC_DRAW);
+
+ glBeginTransformFeedback(GL_POINTS);
+ glDrawArrays(GL_POINTS, 0, verticesToDraw);
+ EXPECT_GL_NO_ERROR();
+ glEndTransformFeedback();
+
+ // Set up the buffer to be too small
+ glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, bytesNeeded - 1, &tfData, GL_STATIC_DRAW);
+
+ glBeginTransformFeedback(GL_POINTS);
+ EXPECT_GL_NO_ERROR();
+ glDrawArrays(GL_POINTS, 0, verticesToDraw);
+ EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+ glEndTransformFeedback();
+
+ // Set up the buffer to be the right size but make it smaller after glBeginTransformFeedback
+ glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, bytesNeeded, &tfData, GL_STATIC_DRAW);
+ glBeginTransformFeedback(GL_POINTS);
+ glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, bytesNeeded - 1, &tfData, GL_STATIC_DRAW);
+ EXPECT_GL_NO_ERROR();
+ glDrawArrays(GL_POINTS, 0, verticesToDraw);
+ EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+ glEndTransformFeedback();
+}
+
// Test that buffer binding happens only on the current transform feedback object
TEST_P(TransformFeedbackTest, BufferBinding)
{