Fix validating glFramebuffer* attachment.

The OpenGL ES 3.0 spec states: An INVALID_OPERATION error is generated if attachment is COLOR_- ATTACHMENTm where m is greater than or equal to the value of MAX_COLOR_- ATTACHMENTS. An INVALID_ENUM error is generated if attachment is not one of the attachments in table 4.6, and attachment is not COLOR_ATTACHMENTm where m is greater than or equal to the value of MAX_COLOR_ATTACHMENTS. Bug b/116776063 Change-Id: Iaabbcd3689d08ebdde2046440cf24554e9a160c2 Reviewed-on: https://swiftshader-review.googlesource.com/c/21908Tested-by: Nicolas Capens <nicolascapens@google.com> Reviewed-by: Alexis Hétu <sugoi@google.com>

Fix validating glFramebuffer* attachment.
4e344194 · Nicolas Capens · Nicolas Capens · ca090827 · 4e344194 · 4e344194
Commit 4e344194 authored Oct 24, 2018 by Nicolas Capens Committed by Nicolas Capens Oct 25, 2018
Hide whitespace changes
Inline Side-by-side

Showing with 46 additions and 44 deletions

libGLESv2.cpp src/OpenGL/libGLESv2/libGLESv2.cpp +34 -9

libGLESv3.cpp src/OpenGL/libGLESv2/libGLESv3.cpp +12 -35

No files found.
--- a/src/OpenGL/libGLESv2/libGLESv2.cpp
+++ b/src/OpenGL/libGLESv2/libGLESv2.cpp
@@ -1927,10 +1927,16 @@ void FramebufferRenderbuffer(GLenum target, GLenum attachment, GLenum renderbuff
 			framebuffer->setStencilbuffer(GL_RENDERBUFFER, renderbuffer);
 			break;
 		default:
-			if((attachment - GL_COLOR_ATTACHMENT0) >= MAX_COLOR_ATTACHMENTS)
+			if(attachment < GL_COLOR_ATTACHMENT0 || attachment > GL_COLOR_ATTACHMENT31)
 			{
 				return error(GL_INVALID_ENUM);
 			}
+
+			if((attachment - GL_COLOR_ATTACHMENT0) >= MAX_COLOR_ATTACHMENTS)
+			{
+				return error(GL_INVALID_OPERATION);
+			}
+
 			framebuffer->setColorbuffer(GL_RENDERBUFFER, renderbuffer, attachment - GL_COLOR_ATTACHMENT0);
 			break;
 		}
@@ -2036,10 +2042,16 @@ void FramebufferTexture2D(GLenum target, GLenum attachment, GLenum textarget, GL
 			framebuffer->setStencilbuffer(textarget, texture, level);
 			break;
 		default:
-			if((attachment - GL_COLOR_ATTACHMENT0) >= MAX_COLOR_ATTACHMENTS)
+			if(attachment < GL_COLOR_ATTACHMENT0 || attachment > GL_COLOR_ATTACHMENT31)
 			{
 				return error(GL_INVALID_ENUM);
 			}
+
+			if((attachment - GL_COLOR_ATTACHMENT0) >= MAX_COLOR_ATTACHMENTS)
+			{
+				return error(GL_INVALID_OPERATION);
+			}
+
 			framebuffer->setColorbuffer(textarget, texture, attachment - GL_COLOR_ATTACHMENT0, level);
 			break;
 		}
@@ -2614,14 +2626,21 @@ void GetFramebufferAttachmentParameteriv(GLenum target, GLenum attachment, GLenu
 			}
 			break;
 		default:
-			if((unsigned int)(attachment - GL_COLOR_ATTACHMENT0) < MAX_COLOR_ATTACHMENTS)
+			if(framebufferName == 0)
 			{
-				if(framebufferName == 0)
-				{
-					return error(GL_INVALID_OPERATION);
-				}
+				return error(GL_INVALID_OPERATION);
+			}
+
+			if(attachment < GL_COLOR_ATTACHMENT0 || attachment > GL_COLOR_ATTACHMENT31)
+			{
+				return error(GL_INVALID_ENUM);
+			}
+
+			if((attachment - GL_COLOR_ATTACHMENT0) >= MAX_COLOR_ATTACHMENTS)
+			{
+				return error(GL_INVALID_OPERATION);
 			}
-			else return error(GL_INVALID_ENUM);
+			break;
 		}

 		es2::Framebuffer *framebuffer = context->getFramebuffer(framebufferName);
@@ -6063,10 +6082,16 @@ void FramebufferTexture3DOES(GLenum target, GLenum attachment, GLenum textarget,
 		case GL_DEPTH_ATTACHMENT:   framebuffer->setDepthbuffer(textarget, texture, level);   break;
 		case GL_STENCIL_ATTACHMENT: framebuffer->setStencilbuffer(textarget, texture, level); break;
 		default:
-			if((attachment - GL_COLOR_ATTACHMENT0) >= MAX_COLOR_ATTACHMENTS)
+			if(attachment < GL_COLOR_ATTACHMENT0 || attachment > GL_COLOR_ATTACHMENT31)
 			{
 				return error(GL_INVALID_ENUM);
 			}
+
+			if((attachment - GL_COLOR_ATTACHMENT0) >= MAX_COLOR_ATTACHMENTS)
+			{
+				return error(GL_INVALID_OPERATION);
+			}
+
 			framebuffer->setColorbuffer(textarget, texture, attachment - GL_COLOR_ATTACHMENT0, level);
 			break;
 		}

--- a/src/OpenGL/libGLESv2/libGLESv3.cpp
+++ b/src/OpenGL/libGLESv2/libGLESv3.cpp
@@ -1294,40 +1294,6 @@ void FramebufferTextureLayer(GLenum target, GLenum attachment, GLuint texture, G

 		switch(attachment)
 		{
-		case GL_COLOR_ATTACHMENT0:
-		case GL_COLOR_ATTACHMENT1:
-		case GL_COLOR_ATTACHMENT2:
-		case GL_COLOR_ATTACHMENT3:
-		case GL_COLOR_ATTACHMENT4:
-		case GL_COLOR_ATTACHMENT5:
-		case GL_COLOR_ATTACHMENT6:
-		case GL_COLOR_ATTACHMENT7:
-		case GL_COLOR_ATTACHMENT8:
-		case GL_COLOR_ATTACHMENT9:
-		case GL_COLOR_ATTACHMENT10:
-		case GL_COLOR_ATTACHMENT11:
-		case GL_COLOR_ATTACHMENT12:
-		case GL_COLOR_ATTACHMENT13:
-		case GL_COLOR_ATTACHMENT14:
-		case GL_COLOR_ATTACHMENT15:
-		case GL_COLOR_ATTACHMENT16:
-		case GL_COLOR_ATTACHMENT17:
-		case GL_COLOR_ATTACHMENT18:
-		case GL_COLOR_ATTACHMENT19:
-		case GL_COLOR_ATTACHMENT20:
-		case GL_COLOR_ATTACHMENT21:
-		case GL_COLOR_ATTACHMENT22:
-		case GL_COLOR_ATTACHMENT23:
-		case GL_COLOR_ATTACHMENT24:
-		case GL_COLOR_ATTACHMENT25:
-		case GL_COLOR_ATTACHMENT26:
-		case GL_COLOR_ATTACHMENT27:
-		case GL_COLOR_ATTACHMENT28:
-		case GL_COLOR_ATTACHMENT29:
-		case GL_COLOR_ATTACHMENT30:
-		case GL_COLOR_ATTACHMENT31:
-			framebuffer->setColorbuffer(textarget, texture, attachment - GL_COLOR_ATTACHMENT0, level, layer);
-			break;
 		case GL_DEPTH_ATTACHMENT:
 			framebuffer->setDepthbuffer(textarget, texture, level, layer);
 			break;
@@ -1339,7 +1305,18 @@ void FramebufferTextureLayer(GLenum target, GLenum attachment, GLuint texture, G
 			framebuffer->setStencilbuffer(textarget, texture, level, layer);
 			break;
 		default:
-			return error(GL_INVALID_ENUM);
+			if(attachment < GL_COLOR_ATTACHMENT0 || attachment > GL_COLOR_ATTACHMENT31)
+			{
+				return error(GL_INVALID_ENUM);
+			}
+
+			if((attachment - GL_COLOR_ATTACHMENT0) >= MAX_COLOR_ATTACHMENTS)
+			{
+				return error(GL_INVALID_OPERATION);
+			}
+
+			framebuffer->setColorbuffer(textarget, texture, attachment - GL_COLOR_ATTACHMENT0, level, layer);
+			break;
 		}
 	}
 }