[Mesa-dev] [PATCH 01/11 v2] mesa: Returns a GL_INVALID_VALUE error on several APIs when buffer size is negative

[Ian Romanick]

Sorry for the delay...

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

On 01/21/2015 04:32 PM, Eduardo Lima Mitev wrote:
> Section 2.3.1 (Errors) of the OpenGL 4.5 spec says:
> 
>     "If a negative number is provided where an argument of type sizei or
>     sizeiptr is specified, an INVALID_VALUE error is generated.
> 
> This patch adds checks for negative buffer size values passed to different APIs.
> It also moves up the check on other APIs that already had it, making it the first
> error check performed in the function, for consistency.
> 
> While there may be other APIs throughtout the code lacking this check (or at least
> not at the beginning of the function), this patch focuses on the cases that break
> the dEQP tests listed below. It could be a good excersize for the future to check
> all other cases, and improve consistency in the order of the checks throughout the
> whole Mesa code base.
> 
> This fixes 5 dEQP test:
> * dEQP-GLES3.functional.negative_api.state.get_attached_shaders
> * dEQP-GLES3.functional.negative_api.state.get_shader_source
> * dEQP-GLES3.functional.negative_api.state.get_active_uniform
> * dEQP-GLES3.functional.negative_api.state.get_active_attrib
> * dEQP-GLES3.functional.negative_api.shader.program_binary
> ---
>  src/mesa/main/shader_query.cpp  |  5 +++++
>  src/mesa/main/shaderapi.c       | 26 ++++++++++++++++++++------
>  src/mesa/main/uniform_query.cpp | 17 +++++++++++------
>  3 files changed, 36 insertions(+), 12 deletions(-)
> 
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 766ad29..df9081b 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -109,6 +109,11 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint desired_index,
>     GET_CURRENT_CONTEXT(ctx);
>     struct gl_shader_program *shProg;
>  
> +   if (maxLength < 0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE, "glGetActiveAttrib(maxLength < 0)");
> +      return;
> +   }
> +
>     shProg = _mesa_lookup_shader_program_err(ctx, program, "glGetActiveAttrib");
>     if (!shProg)
>        return;
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 118e8a7..52eab46 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -448,8 +448,16 @@ static void
>  get_attached_shaders(struct gl_context *ctx, GLuint program, GLsizei maxCount,
>                       GLsizei *count, GLuint *obj)
>  {
> -   struct gl_shader_program *shProg =
> +   struct gl_shader_program *shProg;
> +
> +   if (maxCount < 0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE, "glGetAttachedShaders(maxCount < 0)");
> +      return;
> +   }
> +
> +   shProg =
>        _mesa_lookup_shader_program_err(ctx, program, "glGetAttachedShaders");
> +
>     if (shProg) {
>        GLuint i;
>        for (i = 0; i < (GLuint) maxCount && i < shProg->NumShaders; i++) {
> @@ -786,6 +794,12 @@ get_shader_source(struct gl_context *ctx, GLuint shader, GLsizei maxLength,
>                    GLsizei *length, GLchar *sourceOut)
>  {
>     struct gl_shader *sh;
> +
> +   if (maxLength < 0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE, "glGetShaderSource(bufSize < 0)");
> +      return;
> +   }
> +
>     sh = _mesa_lookup_shader_err(ctx, shader, "glGetShaderSource");
>     if (!sh) {
>        return;
> @@ -1683,6 +1697,11 @@ _mesa_GetProgramBinary(GLuint program, GLsizei bufSize, GLsizei *length,
>     GLsizei length_dummy;
>     GET_CURRENT_CONTEXT(ctx);
>  
> +   if (bufSize < 0){
> +      _mesa_error(ctx, GL_INVALID_VALUE, "glGetProgramBinary(bufSize < 0)");
> +      return;
> +   }
> +
>     shProg = _mesa_lookup_shader_program_err(ctx, program, "glGetProgramBinary");
>     if (!shProg)
>        return;
> @@ -1712,11 +1731,6 @@ _mesa_GetProgramBinary(GLuint program, GLsizei bufSize, GLsizei *length,
>        return;
>     }
>  
> -   if (bufSize < 0){
> -      _mesa_error(ctx, GL_INVALID_VALUE, "glGetProgramBinary(bufSize < 0)");
> -      return;
> -   }
> -
>     *length = 0;
>     _mesa_error(ctx, GL_INVALID_OPERATION,
>                 "glGetProgramBinary(driver supports zero binary formats)");
> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
> index 32870d0..d36f506 100644
> --- a/src/mesa/main/uniform_query.cpp
> +++ b/src/mesa/main/uniform_query.cpp
> @@ -45,9 +45,14 @@ _mesa_GetActiveUniform(GLuint program, GLuint index,
>                         GLenum *type, GLcharARB *nameOut)
>  {
>     GET_CURRENT_CONTEXT(ctx);
> -   struct gl_shader_program *shProg =
> -      _mesa_lookup_shader_program_err(ctx, program, "glGetActiveUniform");
> +   struct gl_shader_program *shProg;
>  
> +   if (maxLength < 0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE, "glGetActiveUniform(maxLength < 0)");
> +      return;
> +   }
> +
> +   shProg = _mesa_lookup_shader_program_err(ctx, program, "glGetActiveUniform");
>     if (!shProg)
>        return;
>  
> @@ -85,16 +90,16 @@ _mesa_GetActiveUniformsiv(GLuint program,
>     struct gl_shader_program *shProg;
>     GLsizei i;
>  
> -   shProg = _mesa_lookup_shader_program_err(ctx, program, "glGetActiveUniform");
> -   if (!shProg)
> -      return;
> -
>     if (uniformCount < 0) {
>        _mesa_error(ctx, GL_INVALID_VALUE,
>  		  "glGetActiveUniformsiv(uniformCount < 0)");
>        return;
>     }
>  
> +   shProg = _mesa_lookup_shader_program_err(ctx, program, "glGetActiveUniform");
> +   if (!shProg)
> +      return;
> +
>     for (i = 0; i < uniformCount; i++) {
>        GLuint index = uniformIndices[i];
>  
>