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

diff --git a/src/OpenGL/libGLESv2/libGLESv2.cpp b/src/OpenGL/libGLESv2/libGLESv2.cpp
index ef00cc3..fd856e5 100644 (file)
--- 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;
                }

diff --git a/src/OpenGL/libGLESv2/libGLESv3.cpp b/src/OpenGL/libGLESv2/libGLESv3.cpp
index 2669739..6ba4c9d 100644 (file)
--- 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;
                }
        }
 }