On Tue, 2015-09-01 at 12:34 +0200, Iago Toral wrote:
> Hey,
>
> On Tue, 2015-08-18 at 12:52 +1000, Dave Airlie wrote:
> > Hey,
> >
> > while running CTS under valgrind I got to see a lot of
> >
> > ==32256== Invalid read of size 2
> > ==32256== at 0x5B53F07: convert_ushort (format_utils.c:1155)
> > ==32256== by 0x5B8523A: _mesa_swizzle_and_convert (format_utils.c:1453)
> > ==32256== by 0x5B11151: _mesa_format_convert (format_utils.c:354)
> > ==32256== by 0x5C07054: texstore_rgba (texstore.c:806)
> > ==32256== by 0x5C073C8: _mesa_texstore (texstore.c:930)
> > ==32256== by 0x5C078B9: store_texsubimage (texstore.c:1068)
> > ==32256== by 0x5C07AC5: _mesa_store_texsubimage (texstore.c:1132)
> > ==32256== by 0x5C9A05F: st_TexSubImage (st_cb_texture.c:856)
> > ==32256== by 0x5C9A196: st_TexImage (st_cb_texture.c:880)
> > ==32256== by 0x5BF1BCC: teximage (teximage.c:3387)
> > ==32256== by 0x5BF1D67: _mesa_TexImage2D (teximage.c:3426)
> > ==32256== by 0x4CDCA15: glTexImage2D (glapi_mapi_tmp.h:2926)
> > ==32256== Address 0xa2b188a is 0 bytes after a block of size 42 alloc'd
> > ==32256== at 0x4A06C50: malloc (in
> > /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> > ==32256== by 0x5C06D97: texstore_rgba (texstore.c:734)
> > ==32256== by 0x5C073C8: _mesa_texstore (texstore.c:930)
> > ==32256== by 0x5C078B9: store_texsubimage (texstore.c:1068)
> > ==32256== by 0x5C07AC5: _mesa_store_texsubimage (texstore.c:1132)
> > ==32256== by 0x5C9A05F: st_TexSubImage (st_cb_texture.c:856)
> > ==32256== by 0x5C9A196: st_TexImage (st_cb_texture.c:880)
> > ==32256== by 0x5BF1BCC: teximage (teximage.c:3387)
> > ==32256== by 0x5BF1D67: _mesa_TexImage2D (teximage.c:3426)
> > ==32256== by 0x4CDCA15: glTexImage2D (glapi_mapi_tmp.h:2926)
> > ==32256== by 0x151483D: glwTexImage2D (glwImpl.inl:482)
> > ==32256== by 0xF1BB0B: packedPixelsPixelRectangleInner
> > (GTFTestPackedPixels.c:3666)
> > ==32256==
> >
> > which lead to the malloc for the SwapBytes case, being too small. It
> > appears the srcRowStride is worked out later at 16-bytes for a width 7
> > ushort format, but the byte swap doesn't allocate enough space,
> >
> > can you guys take a look and suggest a fix, I'm a bit lost there.
>
> sorry for the late reply, I was on holidays...
>
> If we see a srcRowStride of 16-bytes for a width of 7, I guess there are
> some packing options involved. We create the swapped image like this:
>
> if (srcPacking->SwapBytes) {
> int bytesPerPixel = _mesa_bytes_per_pixel(srcFormat, srcType);
> int swapsPerPixel = bytesPerPixel / swapSize;
> int elementCount = srcWidth * srcHeight * srcDepth;
> assert(bytesPerPixel % swapSize == 0);
> tempImage = malloc(elementCount * bytesPerPixel);
> if (!tempImage)
> return GL_FALSE;
> if (swapSize == 2)
> _mesa_swap2_copy(tempImage, (GLushort *) srcAddr,
> elementCount * swapsPerPixel);
> else
> _mesa_swap4_copy(tempImage, (GLuint *) srcAddr,
> elementCount * swapsPerPixel);
> srcAddr = tempImage;
> }
>
> I see that this is not considering the packing options of the original
> image when allocating the swapped image (or when swapping its data), so
> I guess that is the problem.
>
> After that we will do something like:
>
> srcRowStride =
> _mesa_image_row_stride(srcPacking, srcWidth, srcFormat, srcType);
>
> which expects the swapped image to have the same packing options as the
> original (i.e. it uses srcPacking).
>
> I think that we need to use _mesa_image_row_stride() to compute the size
> of the swapped image and then figure out how to incorporate that into
> the las parameter in the calls to _mesa_swap_copy() too.
>
> Dave, do you have any piglit tests or other samples that I can use to
> reproduce the problem? (I don't have access to the conformance test
> suite).
Or maybe an apitrace of a test reproducing the issue...
Iago
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
