[Mesa-dev] [PATCH 3/3] mesa: generate GL_INVALID_VALUE when bufSize < 0
Ian Romanick
idr at freedesktop.org
Tue Apr 24 06:29:56 PDT 2012
On 04/22/2012 09:44 AM, nobled wrote:
> Though not explicit in the GL_ARB_robustness extension,
> the GL standard says:
>
> If a negative number is provided where an argument of type sizei or
> sizeiptr is specified, the error INVALID_VALUE is generated.
>
> While the extension says that GL_INVALID_OPERATION is given
> when the required space is "greater than<bufSize>", and this
Which error do other implementations generate? If there's a clear
trend, we should follow that. If there's not a clear trend, I can file
a bug against the spec for being ambiguous.
> would be technically true for all negative values, defer to the
> above language. This error check seems to apply even when a PBO
> is bound, while insufficient but non-negative values of bufSize
> are ignored.
>
> This also fixes a bug in _mesa_validate_pbo_access() where the
> signed value clientMemSize was blindly cast to an unsigned value.
> When bufSize/clientMemSize were negative, it would be cast to
> a value greater than INT_MAX and no error would have been given,
> not even the incorrect GL_INVALID_OPERATION. Oops.
>
> NOTE: This is a candidate for the 8.0 branch.
> ---
> src/mesa/main/eval.c | 18 ++++++++++++++++++
> src/mesa/main/pbo.c | 15 +++++++++++++++
> src/mesa/main/pixel.c | 6 ++++++
> src/mesa/main/readpix.c | 6 ++++++
> src/mesa/main/texgetimage.c | 16 +++++++++++++++-
> src/mesa/main/uniform_query.cpp | 9 ++++++++-
> 6 files changed, 68 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/main/eval.c b/src/mesa/main/eval.c
> index e651715..8c25295 100644
> --- a/src/mesa/main/eval.c
> +++ b/src/mesa/main/eval.c
> @@ -563,6 +563,12 @@ _mesa_GetnMapdvARB( GLenum target, GLenum query,
> GLsizei bufSize, GLdouble *v )
> return;
> }
>
> + if (bufSize< 0) {
> + _mesa_error( ctx, GL_INVALID_VALUE,
> + "glGetnMapdvARB(bufSize = %d)", bufSize );
> + return;
> + }
> +
> map1d = get_1d_map(ctx, target);
> map2d = get_2d_map(ctx, target);
> ASSERT(map1d || map2d);
> @@ -655,6 +661,12 @@ _mesa_GetnMapfvARB( GLenum target, GLenum query,
> GLsizei bufSize, GLfloat *v )
> return;
> }
>
> + if (bufSize< 0) {
> + _mesa_error( ctx, GL_INVALID_VALUE,
> + "glGetnMapfvARB(bufSize = %d)", bufSize );
> + return;
> + }
> +
> map1d = get_1d_map(ctx, target);
> map2d = get_2d_map(ctx, target);
> ASSERT(map1d || map2d);
> @@ -749,6 +761,12 @@ _mesa_GetnMapivARB( GLenum target, GLenum query,
> GLsizei bufSize, GLint *v )
> return;
> }
>
> + if (bufSize< 0) {
> + _mesa_error( ctx, GL_INVALID_VALUE,
> + "glGetnMapivARB(bufSize = %d)", bufSize );
> + return;
> + }
> +
> map1d = get_1d_map(ctx, target);
> map2d = get_2d_map(ctx, target);
> ASSERT(map1d || map2d);
> diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c
> index d8fa919..0fe5394 100644
> --- a/src/mesa/main/pbo.c
> +++ b/src/mesa/main/pbo.c
> @@ -71,6 +71,9 @@ _mesa_validate_pbo_access(GLuint dimensions,
> /* unsigned, to detect overflow/wrap-around */
> uintptr_t start, end, offset, size;
>
> + /* This should be error-checked by all callers beforehand. */
> + assert(clientMemSize>= 0);
> +
> /* If no PBO is bound, 'ptr' is a pointer to client memory containing
> 'clientMemSize' bytes.
> If a PBO is bound, 'ptr' is an offset into the bound PBO.
> @@ -180,6 +183,12 @@ _mesa_map_validate_pbo_source(struct gl_context *ctx,
> {
> ASSERT(dimensions == 1 || dimensions == 2 || dimensions == 3);
>
> + if (clientMemSize< 0) {
> + _mesa_error( ctx, GL_INVALID_VALUE,
> + "%s(bufSize = %d)", where, clientMemSize );
> + return NULL;
> + }
> +
> if (!_mesa_validate_pbo_access(dimensions, unpack, width, height, depth,
> format, type, clientMemSize, ptr)) {
> if (_mesa_is_bufferobj(unpack->BufferObj)) {
> @@ -276,6 +285,12 @@ _mesa_map_validate_pbo_dest(struct gl_context *ctx,
> {
> ASSERT(dimensions == 1 || dimensions == 2 || dimensions == 3);
>
> + if (clientMemSize< 0) {
> + _mesa_error( ctx, GL_INVALID_VALUE,
> + "%s(bufSize = %d)", where, clientMemSize );
> + return NULL;
> + }
> +
> if (!_mesa_validate_pbo_access(dimensions, unpack, width, height, depth,
> format, type, clientMemSize, ptr)) {
> if (_mesa_is_bufferobj(unpack->BufferObj)) {
> diff --git a/src/mesa/main/pixel.c b/src/mesa/main/pixel.c
> index 450c936..be6a19d 100644
> --- a/src/mesa/main/pixel.c
> +++ b/src/mesa/main/pixel.c
> @@ -153,6 +153,12 @@ validate_pbo_access(struct gl_context *ctx,
> {
> GLboolean ok;
>
> + if (clientMemSize< 0) {
> + _mesa_error(ctx, GL_INVALID_VALUE,
> + "glGetnPixelMap*vARB(bufSize = %d)", clientMemSize);
> + return GL_FALSE;
> + }
> +
> /* Note, need to use DefaultPacking and Unpack's buffer object */
> _mesa_reference_buffer_object(ctx,
> &ctx->DefaultPacking.BufferObj,
> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> index 31acfcb..d4fcc56 100644
> --- a/src/mesa/main/readpix.c
> +++ b/src/mesa/main/readpix.c
> @@ -690,6 +690,12 @@ _mesa_ReadnPixelsARB( GLint x, GLint y, GLsizei
> width, GLsizei height,
> return;
> }
>
> + if (bufSize< 0) {
> + _mesa_error( ctx, GL_INVALID_VALUE,
> + "glReadnPixelsARB(bufSize = %d)", bufSize );
> + return;
> + }
> +
> if (ctx->NewState)
> _mesa_update_state(ctx);
>
> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
> index 05b052a..ff03d6d 100644
> --- a/src/mesa/main/texgetimage.c
> +++ b/src/mesa/main/texgetimage.c
> @@ -659,6 +659,12 @@ getteximage_error_check(struct gl_context *ctx,
> GLenum target, GLint level,
> return GL_TRUE;
> }
>
> + if (clientMemSize< 0) {
> + _mesa_error( ctx, GL_INVALID_VALUE,
> + "glGetnTexImageARB(bufSize = %d)", clientMemSize );
> + return GL_TRUE;
> + }
> +
> err = _mesa_error_check_format_and_type(ctx, format, type);
> if (err != GL_NO_ERROR) {
> _mesa_error(ctx, err, "glGetTexImage(format/type)");
> @@ -717,6 +723,7 @@ getteximage_error_check(struct gl_context *ctx,
> GLenum target, GLint level,
> _mesa_error(ctx, GL_INVALID_OPERATION,
> "glGetTexImage(out of bounds PBO access)");
> } else {
> + assert(bufSize>= 0);
> _mesa_error(ctx, GL_INVALID_OPERATION,
> "glGetnTexImageARB(out of bounds access:"
> " bufSize (%d) is too small)", clientMemSize);
> @@ -822,6 +829,12 @@ getcompressedteximage_error_check(struct
> gl_context *ctx, GLenum target,
> "glGetCompressedTexImageARB(bad level = %d)", level);
> return GL_TRUE;
> }
> + if (clientMemSize< 0) {
> + _mesa_error(ctx, GL_INVALID_VALUE,
> + "glGetnCompressedTexImageARB"
> + "(bufSize = %d)", clientMemSize);
> + return GL_TRUE;
> + }
>
> if (_mesa_is_proxy_texture(target)) {
> _mesa_error(ctx, GL_INVALID_ENUM,
> @@ -857,8 +870,9 @@ getcompressedteximage_error_check(struct
> gl_context *ctx, GLenum target,
> texImage->Depth);
>
> if (!_mesa_is_bufferobj(ctx->Pack.BufferObj)) {
> + assert(clientMemSize>= 0);
> /* do bounds checking on writing to client memory */
> - if (clientMemSize< compressedSize) {
> + if ((GLuint)clientMemSize< compressedSize) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> "glGetnCompressedTexImageARB(out of bounds access:"
> " bufSize (%d) is too small)", clientMemSize);
> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
> index da41ee8..96fe70b 100644
> --- a/src/mesa/main/uniform_query.cpp
> +++ b/src/mesa/main/uniform_query.cpp
> @@ -187,6 +187,12 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint
> program, GLint location,
> struct gl_uniform_storage *uni;
> unsigned loc, offset;
>
> + if (bufSize< 0) {
> + _mesa_error( ctx, GL_INVALID_VALUE,
> + "glGetnUniform*vARB(bufSize = %d)", bufSize );
> + return;
> + }
> +
> if (!validate_uniform_parameters(ctx, shProg, location, 1,
> &loc,&offset, "glGetUniform", true))
> return;
> @@ -210,7 +216,8 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint
> program, GLint location,
> * with glGetUniformdv()...
> */
> unsigned bytes = sizeof(src[0]) * elements;
> - if (bufSize< 0 || bytes> (unsigned) bufSize) {
> + assert(bufSize>= 0);
> + if (bytes> (unsigned) bufSize) {
> _mesa_error( ctx, GL_INVALID_OPERATION,
> "glGetnUniform*vARB(out of bounds: bufSize is %d,"
> " but %u bytes are required)", bufSize, bytes );
More information about the mesa-dev
mailing list