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.
_______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
