Check that transform feedback will not overflow its buffers.

src/libANGLE/Context.cpp

@@ -129,20 +129,13 @@ gl::Error GetQueryObjectParameter(gl::Query *query, GLenum pname, T *params)
     }
 }
 
-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 @@ void Context::drawArrays(GLenum mode, GLint first, GLsizei count)
 
     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 @@ void Context::drawArraysInstanced(GLenum mode, GLint first, GLsizei count, GLsiz
     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)

src/libANGLE/ErrorStrings.h

@@ -189,6 +189,7 @@ ERRMSG(TextureNotPow2, "The texture is a non-power-of-two texture.");
 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 "

src/libANGLE/MemoryProgramCache.cpp

@@ -428,6 +428,8 @@ LinkResult MemoryProgramCache::Deserialize(const Context *context,
                   "Too many shader types");
     state->mLinkedShaderStages = stream.readInt<unsigned long>();
 
+    state->updateTransformFeedbackStrides();
+
     return program->getImplementation()->load(context, infoLog, &stream);
 }
 

src/libANGLE/Program.cpp

@@ -1229,6 +1229,30 @@ void Program::updateLinkedShaderStages()
     }
 }
 
+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 @@ void Program::gatherTransformFeedbackVaryings(const ProgramMergedVaryings &varyi
             }
         }
     }
+    mState.updateTransformFeedbackStrides();
 }
 
 ProgramMergedVaryings Program::getMergedVaryings(const Context *context) const

src/libANGLE/Program.h

@@ -358,6 +358,8 @@ class ProgramState final : angle::NonCopyable
     friend class MemoryProgramCache;
     friend class Program;
 
+    void updateTransformFeedbackStrides();
+
     std::string mLabel;
 
     sh::WorkGroupSize mComputeShaderLocalSize;
@@ -431,6 +433,9 @@ class ProgramState final : angle::NonCopyable
     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 @@ class Program final : angle::NonCopyable, public LabeledObject
     ComponentTypeMask getAttributesTypeMask() const { return mState.mAttributesTypeMask; }
     AttributesMask getAttributesMask() const { return mState.mAttributesMask; }
 
+    const std::vector<GLsizei> &getTransformFeedbackStrides() const
+    {
+        return mState.mTransformFeedbackStrides;
+    }
+
   private:
     ~Program() override;
 

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 @@ void TransformFeedback::begin(const Context *context, GLenum primitiveMode, Prog
     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 @@ GLenum TransformFeedback::getPrimitiveMode() const
     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)

src/libANGLE/TransformFeedback.h

@@ -44,6 +44,8 @@ class TransformFeedbackState final : angle::NonCopyable
     bool mActive;
     GLenum mPrimitiveMode;
     bool mPaused;
+    GLsizeiptr mVerticesDrawn;
+    GLsizeiptr mVertexCapacity;
 
     Program *mProgram;
 
@@ -68,6 +70,14 @@ class TransformFeedback final : public RefCountObject, public LabeledObject
     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;
 

src/libANGLE/angletypes.cpp

@@ -342,4 +342,33 @@ bool ComponentTypeMask::Validate(unsigned long outputTypes,
     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

src/libANGLE/angletypes.h

@@ -331,6 +331,11 @@ using DrawBuffersArray = std::array<T, IMPLEMENTATION_MAX_DRAW_BUFFERS>;
 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

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 @@ bool ValidateDrawBase(Context *context, GLenum mode, GLsizei count)
             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 @@ bool ValidateDrawArraysCommon(Context *context,
     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))

src/tests/gl_tests/TransformFeedbackTest.cpp

@@ -286,6 +286,66 @@ TEST_P(TransformFeedbackTest, RecordAndDraw)
     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)
 {