[Mesa-dev] [PATCH] mesa: Use signed math for possibly negative values

[Chad Versace]

On 05/19/2011 05:13 AM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> Since RowStride might be negative, we want a signed result.  On 64-bit
> platforms, an unsigned 32-bit result added to a 64-bit pointer will
> give the wrong answer.
> 
> For x = y = 50, RowStride = -128, and format bytes = 4, the difference is:
> 
> Breakpoint 1, get_pointer_generic (ctx=0x686320, rb=0x9c3600, x=50, y=50)
>     at main/renderbuffer.c:99
> 99	   if (!rb->Data)
> (gdb) print (int *) rb->Data + (y * rb->RowStride + x)
> $1 = (int *) 0x7ffff2e3e2c8
> (gdb) print rb->Data + (y * rb->RowStride + x)*_mesa_get_format_bytes(rb->Format)
> $3 = (GLvoid *) 0x8000f2e3e2c8
> 
> The delta between 0x7ffff2e3e2c8 and 0x8000f2e3e2c8 is pretty big.
> Math is hard.
> 
> Fixes crashes in lots of depth/stencil piglit tests on 64-bit platforms.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=37351
> ---
>  src/mesa/main/renderbuffer.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/main/renderbuffer.c b/src/mesa/main/renderbuffer.c
> index fa884c0..b7ab49f 100644
> --- a/src/mesa/main/renderbuffer.c
> +++ b/src/mesa/main/renderbuffer.c
> @@ -96,11 +96,17 @@ static void *
>  get_pointer_generic(struct gl_context *ctx, struct gl_renderbuffer *rb,
>  		    GLint x, GLint y)
>  {
> +   intptr_t offset;
> +
>     if (!rb->Data)
>        return NULL;
>  
> -   return ((char *) rb->Data +
> -	   (y * rb->RowStride + x) * _mesa_get_format_bytes(rb->Format));
> +   /* Must use signed math for this!  Since RowStride might be negative, we
> +    * want a signed result.  On 64-bit platforms, an unsigned 32-bit result
> +    * added to a 64-bit pointer will give the wrong answer.
> +    */
> +   offset = (y * rb->RowStride + x) * (int)_mesa_get_format_bytes(rb->Format);
> +   return ((char *) rb->Data + offset);
>  }
>  
>  /* GetRow() implementation for formats where DataType matches the rb->Format.


Looks like all eyes were on this bug. I was just about a submit this
patch to the list myself.

Reviewed-by: Chad Versace <chad.versace at intel.com>
-- 
Chad Versace
chad at chad-versace.us