[Mesa-dev] [PATCH v2] mesa: Deal with size differences between GLuint and GLhandleARB in GetAttachedObjectsARB

[Ian Romanick]

Please revert the version of this patch that has already landed.

On 01/21/2016 08:48 AM, Jeremy Huddleston Sequoia wrote:
> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu at apple.com>
> CC: Nicolai Hähnle <nhaehnle at gmail.com>
> CC: Matt Turner <mattst88 at gmail.com>
> ---
>  src/mesa/main/shaderapi.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index a988f41..7a0d19a 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -1374,10 +1374,26 @@ _mesa_DetachShader(GLuint program, GLuint shader)
>  
>  void GLAPIENTRY
>  _mesa_GetAttachedObjectsARB(GLhandleARB container, GLsizei maxCount,
> -                            GLsizei * count, GLhandleARB * obj)
> +                            GLsizei * count, GLhandleARB * objARB)
>  {
> +   int i;
> +   GLuint *obj;
> +
>     GET_CURRENT_CONTEXT(ctx);
> +
> +   obj = calloc(maxCount, sizeof(GLuint));

calloc is allowed to return NULL when passed 0.  glGetAttachedObjectsARB
is not supposed to generate an error in that condition.

If passed a negative value, glGetAttachedObjectsARB is supposed to
generate GL_INVALID_VALUE.  In this case, since calloc will treat the
negative value as a very large positive value, it will generate
GL_OUT_OF_MEMORY.

> +   if (!obj) {
> +      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glShaderSourceARB");
> +      return;
> +   }
> +
>     get_attached_shaders(ctx, container, maxCount, count, obj);
> +
> +   for (i = 0 ; i < *count; i++) {

count can be NULL, so this is completely bogus.

I will submit piglit tests for all three issues.

In addition to the bugs, this patch adds almost as much code to
_mesa_GetAttachedObjectsARB as just open-coding get_attached_shaders
would.  If we have to do anything, I feel like that's a much better option.

> +      objARB[i] = (GLhandleARB)obj[i];
> +   }
> +
> +   free(obj);
>  }
>  
>