Generate an error when trying to clear an incomplete Framebuffer Calling ClearBuffer* on an incomplete framebuffer was crashing because a depth clear assumes 4 bytes per pixel and the type of an incomplete framebuffer could be anything, which is potentially smaller than 4 bytes, so memory was written out of bounds. ClearBuffer* now also checks for completeness, just like clear already does. From the OpenGL ES 3.0 spec, section 4.4.4.4 "Effects of Framebuffer Completeness on Framebuffer Operations Attempting to render to or read from a framebuffer which is not framebuffer complete will generate an INVALID_FRAMEBUFFER_OPERATION error. This means that rendering commands (see section 2.6) ... will generate the error INVALID_FRAMEBUFFER_OPERATION if called while the framebuffer is not framebuffer complete" And from the OpenGL ES 3.0 spec, section 2.6 Rendering Commands "GL commands performing rendering into a framebuffer are called rendering commands, and include the drawing commands Draw* ..., as well as these additional commands: • BlitFramebuffer • Clear • ClearBuffer*" Bug b/117048995 Change-Id: I01fd2ad2829b4c9e0aac817620f65c789b11356e Reviewed-on: https://swiftshader-review.googlesource.com/c/22048 Tested-by: Alexis Hétu <sugoi@google.com> Reviewed-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/OpenGL/libGLESv2/Context.cpp b/src/OpenGL/libGLESv2/Context.cpp index 866912c..18aa8d9 100644 --- a/src/OpenGL/libGLESv2/Context.cpp +++ b/src/OpenGL/libGLESv2/Context.cpp
@@ -3465,7 +3465,7 @@ if(rgbaMask && !mState.rasterizerDiscardEnabled) { Framebuffer *framebuffer = getDrawFramebuffer(); - if(!framebuffer) + if(!framebuffer || (framebuffer->completeness() != GL_FRAMEBUFFER_COMPLETE)) { return error(GL_INVALID_FRAMEBUFFER_OPERATION); } @@ -3507,7 +3507,7 @@ if(mState.depthMask && !mState.rasterizerDiscardEnabled) { Framebuffer *framebuffer = getDrawFramebuffer(); - if(!framebuffer) + if(!framebuffer || (framebuffer->completeness() != GL_FRAMEBUFFER_COMPLETE)) { return error(GL_INVALID_FRAMEBUFFER_OPERATION); } @@ -3535,7 +3535,7 @@ if(mState.stencilWritemask && !mState.rasterizerDiscardEnabled) { Framebuffer *framebuffer = getDrawFramebuffer(); - if(!framebuffer) + if(!framebuffer || (framebuffer->completeness() != GL_FRAMEBUFFER_COMPLETE)) { return error(GL_INVALID_FRAMEBUFFER_OPERATION); }
diff --git a/tests/GLESUnitTests/unittests.cpp b/tests/GLESUnitTests/unittests.cpp index 41d4fb9..e51bf99 100644 --- a/tests/GLESUnitTests/unittests.cpp +++ b/tests/GLESUnitTests/unittests.cpp
@@ -348,6 +348,33 @@ Uninitialize(); } +// Test attempting to clear an incomplete framebuffer +TEST_F(SwiftShaderTest, ClearIncomplete) +{ + Initialize(3, false); + + GLfloat zero_float = 0; + GLuint renderbuffer; + glGenRenderbuffers(1, &renderbuffer); + GLuint framebuffer; + glGenFramebuffers(1, &framebuffer); + + glBindRenderbuffer(GL_RENDERBUFFER, renderbuffer); + EXPECT_GLENUM_EQ(GL_NO_ERROR, glGetError()); + glRenderbufferStorage(GL_RENDERBUFFER, GL_R8I, 43, 27); + EXPECT_GLENUM_EQ(GL_NO_ERROR, glGetError()); + glBindFramebuffer(GL_READ_FRAMEBUFFER, framebuffer); + EXPECT_GLENUM_EQ(GL_NO_ERROR, glGetError()); + glFramebufferRenderbuffer(GL_READ_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, renderbuffer); + EXPECT_GLENUM_EQ(GL_NO_ERROR, glGetError()); + glBindFramebuffer(GL_FRAMEBUFFER, framebuffer); + EXPECT_GLENUM_EQ(GL_NO_ERROR, glGetError()); + glClearBufferfv(GL_DEPTH, 0, &zero_float); + EXPECT_GLENUM_EQ(GL_INVALID_FRAMEBUFFER_OPERATION, glGetError()); + + Uninitialize(); +} + // Test unrolling of a loop TEST_F(SwiftShaderTest, UnrollLoop) {