[Mesa-dev] [PATCH] i965: Fix segfault in WebGL Conformance on Ivybridge

[Chad Versace]

On Tue 11 Nov 2014, Ian Romanick wrote:
>On 11/10/2014 04:02 PM, Chad Versace wrote:
>>
>>        map->stride = mt->pitch;
>> -      map->ptr = base + y * map->stride + x * mt->cpp;
>> +
>> +      /* The variables in below pointer arithmetic are 32-bit. The arithmetic
>> +       * overflows for large textures.  Therefore the cast to intptr_t is
>> +       * needed.
>> +       *
>> +       * TODO(chadv): Fix this everywhere in i965 by fixing the signature of
>> +       * intel_miptree_get_image_offset() to use intptr_t.
>> +       */
>> +      map->ptr = base + (intptr_t) y * (intptr_t) map->stride
>> +                      + (intptr_t) x * (intptr_t) mt->cpp;
>
>I don't even want to know how long it took to track that down. :(

It took way too long... :(  I spent a lot of time on the wrong trail, 
trying to find errors in Mesa Core's pack/unpack routines. The bisect 
gave me the clue to where the problem was.

>Is it sufficient to just use unsigned?

No. This expression is doing pointer arithmetic. Let's move into the 
21st century and just use intptr_t and uintptr_t for pointer arithmetic.

Just using a 32-bit unsigned here may fix this particular bug, but still
leaves the door open for future buckets-of-fun debugging sessions.

Let's do the right thing and use a C99 pointer-width type.

> y and map->stride are the
>problematic values here.  Isn't y bounded by (basically) 2*8192 and
>map->stride bounded by 16*8192?  That sounds like 0x80000000, which is
>negative.

>It seems like we could have similar problems in many places, and I'd
>like to figure out what the correct coding practices are to avoid it.

I believe the correct coding practice is to always use C99 pointer-width 
types (inptr_t, uintptr_t, ptrdiff_t) when doing pointer arithmetic.