[Mesa-dev] [PATCH 6/6] mesa/main: fix incorrect detph-error

[Juan A. Suarez Romero]

On Thu, 2018-11-22 at 17:48 +0100, Erik Faye-Lund wrote:
> If glGetTexImage or glGetnTexImage is called with a level that doesn't
> exist, we get an error message on this form:
> 
> Mesa: User error: GL_INVALID_VALUE in glGetTexImage(depth = 0)
> 
> This is clearly nonsensical, because these APIs don't even have a
> depth-parameter. The reason is that get_texture_image_dims() return
> all-zero dimensions for non-existent texture-images, and we go on to
> validate these dimensions as if they were user-input, because
> glGetTextureSubImage requires checking.
> 
> So let's split this logic in two, so glGetTextureSubImage can have
> stricter input-validation. All arguments that are no longer validated
> are generated internally by mesa, so there's no use in validating them.
> 
> Fixes: 42891dbaa12 "gettextsubimage: verify zoffset and depth are correct"
> Signed-off-by: Erik Faye-Lund <erik.faye-lund at collabora.com>

This patch fixes 42891dbaa12 "gettextsubimage: verify zoffset and depth are
correct" which is in 18.2 branch.

Nevertheless, I'm going to reject this patch as it requires other patches not in
the branch, at least 854202f70e6 ("mesa/main: factor out tex-image error-
checking") and 84bc5738401 ("mesa/main: factor out common error-checking");
maybe others too.


If you think the patch must be added, including 854202f70e6 and 84bc5738401,
then let me know. It could be that we are a bit late to add them in the next
release; but I'll ensure it will be added in the after one.

	J.A.


> ---
>  src/mesa/main/texgetimage.c | 57 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 50 insertions(+), 7 deletions(-)
> 
> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
> index 190f53d62fe..dabfcd06a52 100644
> --- a/src/mesa/main/texgetimage.c
> +++ b/src/mesa/main/texgetimage.c
> @@ -1255,7 +1255,6 @@ static bool
>  getteximage_error_check(struct gl_context *ctx,
>                          struct gl_texture_object *texObj,
>                          GLenum target, GLint level,
> -                        GLint xoffset, GLint yoffset, GLint zoffset,
>                          GLsizei width, GLsizei height, GLsizei depth,
>                          GLenum format, GLenum type, GLsizei bufSize,
>                          GLvoid *pixels, const char *caller)
> @@ -1269,6 +1268,49 @@ getteximage_error_check(struct gl_context *ctx,
>        return true;
>     }
>  
> +   if (width == 0 || height == 0 || depth == 0) {
> +      /* Not an error, but nothing to do.  Return 'true' so that the
> +       * caller simply returns.
> +       */
> +      return true;
> +   }
> +
> +   if (pbo_error_check(ctx, target, width, height, depth,
> +                       format, type, bufSize, pixels, caller)) {
> +      return true;
> +   }
> +
> +   texImage = select_tex_image(texObj, target, level, 0);
> +   if (teximage_error_check(ctx, texImage, format, caller)) {
> +      return true;
> +   }
> +
> +   return false;
> +}
> +
> +
> +/**
> + * Do error checking for all (non-compressed) get-texture-image functions.
> + * \return true if any error, false if no errors.
> + */
> +static bool
> +gettexsubimage_error_check(struct gl_context *ctx,
> +                           struct gl_texture_object *texObj,
> +                           GLenum target, GLint level,
> +                           GLint xoffset, GLint yoffset, GLint zoffset,
> +                           GLsizei width, GLsizei height, GLsizei depth,
> +                           GLenum format, GLenum type, GLsizei bufSize,
> +                           GLvoid *pixels, const char *caller)
> +{
> +   struct gl_texture_image *texImage;
> +
> +   assert(texObj);
> +
> +   if (common_error_check(ctx, texObj, target, level, width, height, depth,
> +                          format, type, bufSize, pixels, caller)) {
> +      return true;
> +   }
> +
>     if (dimensions_error_check(ctx, texObj, target, level,
>                                xoffset, yoffset, zoffset,
>                                width, height, depth, caller)) {
> @@ -1417,7 +1459,7 @@ _mesa_GetnTexImage(GLenum target, GLint level, GLenum format, GLenum type,
>     get_texture_image_dims(texObj, target, level, &width, &height, &depth);
>  
>     if (getteximage_error_check(ctx, texObj, target, level,
> -                               0, 0, 0, width, height, depth,
> +                               width, height, depth,
>                                 format, type, bufSize, pixels, caller)) {
>        return;
>     }
> @@ -1448,7 +1490,7 @@ _mesa_GetTexImage(GLenum target, GLint level, GLenum format, GLenum type,
>     get_texture_image_dims(texObj, target, level, &width, &height, &depth);
>  
>     if (getteximage_error_check(ctx, texObj, target, level,
> -                               0, 0, 0, width, height, depth,
> +                               width, height, depth,
>                                 format, type, INT_MAX, pixels, caller)) {
>        return;
>     }
> @@ -1482,7 +1524,7 @@ _mesa_GetTextureImage(GLuint texture, GLint level, GLenum format, GLenum type,
>                            &width, &height, &depth);
>  
>     if (getteximage_error_check(ctx, texObj, texObj->Target, level,
> -                               0, 0, 0, width, height, depth,
> +                               width, height, depth,
>                                 format, type, bufSize, pixels, caller)) {
>        return;
>     }
> @@ -1515,9 +1557,10 @@ _mesa_GetTextureSubImage(GLuint texture, GLint level,
>        return;
>     }
>  
> -   if (getteximage_error_check(ctx, texObj, texObj->Target, level,
> -                               xoffset, yoffset, zoffset, width, height, depth,
> -                               format, type, bufSize, pixels, caller)) {
> +   if (gettexsubimage_error_check(ctx, texObj, texObj->Target, level,
> +                                  xoffset, yoffset, zoffset,
> +                                  width, height, depth,
> +                                  format, type, bufSize, pixels, caller)) {
>        return;
>     }
>