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/22048Tested-by: Alexis Hétu <sugoi@google.com> Reviewed-by: Nicolas Capens <nicolascapens@google.com>

Generate an error when trying to clear an incomplete Framebuffer
e83048e2 · Alexis Hetu · Alexis Hétu · d0a4800e · e83048e2 · e83048e2
Commit e83048e2 authored Oct 29, 2018 by Alexis Hetu Committed by Alexis Hétu Oct 30, 2018
Hide whitespace changes
Inline Side-by-side

Showing with 30 additions and 3 deletions

Context.cpp src/OpenGL/libGLESv2/Context.cpp +3 -3

unittests.cpp tests/GLESUnitTests/unittests.cpp +27 -0

No files found.
--- a/src/OpenGL/libGLESv2/Context.cpp
+++ b/src/OpenGL/libGLESv2/Context.cpp
@@ -3465,7 +3465,7 @@ void Context::clearColorBuffer(GLint drawbuffer, void *value, sw::Format format)
 	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 @@ void Context::clearDepthBuffer(const GLfloat value)
 	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 @@ void Context::clearStencilBuffer(const GLint value)
 	if(mState.stencilWritemask && !mState.rasterizerDiscardEnabled)
 	{
 		Framebuffer *framebuffer = getDrawFramebuffer();
-		if(!framebuffer)
+		if(!framebuffer || (framebuffer->completeness() != GL_FRAMEBUFFER_COMPLETE))
 		{
 			return error(GL_INVALID_FRAMEBUFFER_OPERATION);
 		}

--- a/tests/GLESUnitTests/unittests.cpp
+++ b/tests/GLESUnitTests/unittests.cpp
@@ -348,6 +348,33 @@ TEST_F(SwiftShaderTest, Initalization)
 	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)
 {