Commit cf853b30 by Jamie Madill Committed by Angle LUCI CQ

Add missing buffer validation to BeginTransformFeedback.

A change to Vulkan exposed this missing validation. In the Vulkan back-end we do some caching on BeginXFB, which would perform an invalid memory access. Adding the missing validation correctly traps the error before we reach the back-end. Bug: chromium:1171685 Bug: angleproject:4622 Change-Id: I5c92575a07149e431c4f260a4555ff196822c64e Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2937022Reviewed-by: 's avatarTim Van Patten <timvp@google.com> Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
parent 05d5e0d3
...@@ -527,6 +527,7 @@ MSG kTransfomFeedbackAlreadyActive = "Transform feedback is already active."; ...@@ -527,6 +527,7 @@ MSG kTransfomFeedbackAlreadyActive = "Transform feedback is already active.";
MSG kTransformFeedbackActiveDelete = "Attempt to delete an active transform feedback."; MSG kTransformFeedbackActiveDelete = "Attempt to delete an active transform feedback.";
MSG kTransformFeedbackActiveDuringLink = "Cannot link program while program is associated with an active transform feedback object."; MSG kTransformFeedbackActiveDuringLink = "Cannot link program while program is associated with an active transform feedback object.";
MSG kTransformFeedbackBufferDoubleBound = "A transform feedback buffer that would be written to is also bound to a non-transform-feedback target, which would cause undefined behavior."; MSG kTransformFeedbackBufferDoubleBound = "A transform feedback buffer that would be written to is also bound to a non-transform-feedback target, which would cause undefined behavior.";
MSG kTransformFeedbackBufferMissing = "Every binding point used in transform feedback mode must have a buffer object bound.";
MSG kTransformFeedbackBufferMultipleOutputs = "Transform feedback has a buffer bound to multiple outputs."; MSG kTransformFeedbackBufferMultipleOutputs = "Transform feedback has a buffer bound to multiple outputs.";
MSG kTransformFeedbackBufferTooSmall = "Not enough space in bound transform feedback buffers."; MSG kTransformFeedbackBufferTooSmall = "Not enough space in bound transform feedback buffers.";
MSG kTransformFeedbackDoesNotExist = "Transform feedback object that does not exist."; MSG kTransformFeedbackDoesNotExist = "Transform feedback object that does not exist.";
......
...@@ -2684,7 +2684,7 @@ bool ValidateBeginTransformFeedback(const Context *context, PrimitiveMode primit ...@@ -2684,7 +2684,7 @@ bool ValidateBeginTransformFeedback(const Context *context, PrimitiveMode primit
for (size_t i = 0; i < transformFeedback->getIndexedBufferCount(); i++) for (size_t i = 0; i < transformFeedback->getIndexedBufferCount(); i++)
{ {
const auto &buffer = transformFeedback->getIndexedBuffer(i); const OffsetBindingPointer<Buffer> &buffer = transformFeedback->getIndexedBuffer(i);
if (buffer.get()) if (buffer.get())
{ {
if (buffer->isMapped()) if (buffer->isMapped())
...@@ -2704,20 +2704,30 @@ bool ValidateBeginTransformFeedback(const Context *context, PrimitiveMode primit ...@@ -2704,20 +2704,30 @@ bool ValidateBeginTransformFeedback(const Context *context, PrimitiveMode primit
} }
const ProgramExecutable *programExecutable = context->getState().getProgramExecutable(); const ProgramExecutable *programExecutable = context->getState().getProgramExecutable();
if (programExecutable) if (!programExecutable)
{ {
if (programExecutable->getLinkedTransformFeedbackVaryings().size() == 0) context->validationError(GL_INVALID_OPERATION, kProgramNotBound);
{ return false;
context->validationError(GL_INVALID_OPERATION, kNoTransformFeedbackOutputVariables);
return false;
}
} }
else
if (programExecutable->getLinkedTransformFeedbackVaryings().empty())
{ {
context->validationError(GL_INVALID_OPERATION, kProgramNotBound); context->validationError(GL_INVALID_OPERATION, kNoTransformFeedbackOutputVariables);
return false; return false;
} }
size_t programXfbCount = programExecutable->getTransformFeedbackBufferCount();
for (size_t programXfbIndex = 0; programXfbIndex < programXfbCount; ++programXfbIndex)
{
const OffsetBindingPointer<Buffer> &buffer =
transformFeedback->getIndexedBuffer(programXfbIndex);
if (!buffer.get())
{
context->validationError(GL_INVALID_OPERATION, kTransformFeedbackBufferMissing);
return false;
}
}
return true; return true;
} }
......
...@@ -3258,6 +3258,46 @@ void main() ...@@ -3258,6 +3258,46 @@ void main()
} }
} }
// Tests transform feedback with no buffer bound.
TEST_P(TransformFeedbackTest, MissingBuffer)
{
constexpr char kVS[] = R"(#version 300 es
in vec2 position;
in float attrib;
out float varyingAttrib;
void main() {
gl_Position = vec4(position, 0, 1);
varyingAttrib = attrib;
})";
const std::vector<std::string> tfVaryings = {"varyingAttrib"};
ANGLE_GL_PROGRAM_TRANSFORM_FEEDBACK(prog, kVS, essl3_shaders::fs::Green(), tfVaryings,
GL_INTERLEAVED_ATTRIBS);
glUseProgram(prog);
GLTransformFeedback xfb;
glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, xfb);
glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, 0);
std::vector<float> attribData;
for (unsigned int cnt = 0; cnt < 100; ++cnt)
{
attribData.push_back(static_cast<float>(cnt));
}
GLint attribLocation = glGetAttribLocation(prog, "attrib");
ASSERT_NE(-1, attribLocation);
glVertexAttribPointer(attribLocation, 1, GL_FLOAT, GL_FALSE, 4, &attribData[0]);
glEnableVertexAttribArray(attribLocation);
// GLES 3.1 spec: 12.1. TRANSFORM FEEDBACK
// The error INVALID_OPERATION is generated by BeginTransformFeedback if any binding point used
// in transform feedback mode does not have a buffer object bound.
glBeginTransformFeedback(GL_TRIANGLES);
EXPECT_GL_ERROR(GL_INVALID_OPERATION);
}
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(TransformFeedbackTest); GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(TransformFeedbackTest);
ANGLE_INSTANTIATE_TEST_ES3(TransformFeedbackTest); ANGLE_INSTANTIATE_TEST_ES3(TransformFeedbackTest);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment