From f30cf3258e495a583e011e07d5b4a19031c5518f Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 1 Sep 2015 09:31:15 +0100 Subject: [PATCH] meta: Compute correct buffer size with SkipRows/SkipPixels If the user is specifying a subregion of a buffer using SKIP_ROWS and SKIP_PIXELS, we must compute the buffer size carefully as the end of the last row may be much shorter than stride*image_height*depth. The current code tries to memcpy from beyond the end of the user data, for example causing: ==28136== Invalid read of size 8 ==28136== at 0x4C2D94E: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:915) ==28136== by 0xB4ADFE3: brw_bo_write (brw_batch.c:1856) ==28136== by 0xB5B3531: brw_buffer_data (intel_buffer_objects.c:208) ==28136== by 0xB0F6275: _mesa_buffer_data (bufferobj.c:1600) ==28136== by 0xB0F6346: _mesa_BufferData (bufferobj.c:1631) ==28136== by 0xB37A1EE: create_texture_for_pbo (meta_tex_subimage.c:103) ==28136== by 0xB37A467: _mesa_meta_pbo_TexSubImage (meta_tex_subimage.c:176) ==28136== by 0xB5C8D61: intelTexSubImage (intel_tex_subimage.c:195) ==28136== by 0xB254AB4: _mesa_texture_sub_image (teximage.c:3654) ==28136== by 0xB254C9F: texsubimage (teximage.c:3712) ==28136== by 0xB2550E9: _mesa_TexSubImage2D (teximage.c:3853) ==28136== by 0x401CA0: UploadTexSubImage2D (teximage.c:171) ==28136== Address 0xd8bfbe0 is 0 bytes after a block of size 1,024 alloc'd ==28136== at 0x4C28C20: malloc (vg_replace_malloc.c:296) ==28136== by 0x402014: PerfDraw (teximage.c:270) ==28136== by 0x402648: Draw (glmain.c:182) ==28136== by 0x8385E63: ??? (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136== by 0x83896C8: fgEnumWindows (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136== by 0x838641C: glutMainLoopEvent (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136== by 0x8386C1C: glutMainLoop (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136== by 0x4019C1: main (glmain.c:262) ==28136== ==28136== Invalid read of size 8 ==28136== at 0x4C2D940: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:915) ==28136== by 0xB4ADFE3: brw_bo_write (brw_batch.c:1856) ==28136== by 0xB5B3531: brw_buffer_data (intel_buffer_objects.c:208) ==28136== by 0xB0F6275: _mesa_buffer_data (bufferobj.c:1600) ==28136== by 0xB0F6346: _mesa_BufferData (bufferobj.c:1631) ==28136== by 0xB37A1EE: create_texture_for_pbo (meta_tex_subimage.c:103) ==28136== by 0xB37A467: _mesa_meta_pbo_TexSubImage (meta_tex_subimage.c:176) ==28136== by 0xB5C8D61: intelTexSubImage (intel_tex_subimage.c:195) ==28136== by 0xB254AB4: _mesa_texture_sub_image (teximage.c:3654) ==28136== by 0xB254C9F: texsubimage (teximage.c:3712) ==28136== by 0xB2550E9: _mesa_TexSubImage2D (teximage.c:3853) ==28136== by 0x401CA0: UploadTexSubImage2D (teximage.c:171) ==28136== Address 0xd8bfbe8 is 8 bytes after a block of size 1,024 alloc'd ==28136== at 0x4C28C20: malloc (vg_replace_malloc.c:296) ==28136== by 0x402014: PerfDraw (teximage.c:270) ==28136== by 0x402648: Draw (glmain.c:182) ==28136== by 0x8385E63: ??? (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136== by 0x83896C8: fgEnumWindows (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136== by 0x838641C: glutMainLoopEvent (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136== by 0x8386C1C: glutMainLoop (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136== by 0x4019C1: main (glmain.c:262) ==28136== Fixes regression from commit 7f396189f073d626c5f7a2c232dac92b65f5a23f Author: Jason Ekstrand Date: Mon Jan 5 18:17:04 2015 -0800 meta: Add a BlitFramebuffers-based implementation of TexSubImage v2: However, the teximage we create does need to be width x full_height x 1 Signed-off-by: Chris Wilson Cc: Jason Ekstrand Cc: Neil Roberts Reviewed-by Neil Roberts --- src/mesa/drivers/common/meta_tex_subimage.c | 45 ++++++++++++++------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/mesa/drivers/common/meta_tex_subimage.c b/src/mesa/drivers/common/meta_tex_subimage.c index 16d8f5d4747..33c22aa139d 100644 --- a/src/mesa/drivers/common/meta_tex_subimage.c +++ b/src/mesa/drivers/common/meta_tex_subimage.c @@ -46,8 +46,9 @@ #include "varray.h" static struct gl_texture_image * -create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, - GLenum pbo_target, int width, int height, +create_texture_for_pbo(struct gl_context *ctx, + bool create_pbo, GLenum pbo_target, + int dims, int width, int height, int depth, GLenum format, GLenum type, const void *pixels, const struct gl_pixelstore_attrib *packing, GLuint *tmp_pbo, GLuint *tmp_tex) @@ -73,13 +74,18 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, return NULL; /* Account for SKIP_PIXELS, SKIP_ROWS, ALIGNMENT, and SKIP_IMAGES */ - pixels = _mesa_image_address3d(packing, pixels, - width, height, format, type, 0, 0, 0); + uint32_t first_pixel = _mesa_image_offset(dims, packing, width, height, + format, type, + 0, 0, 0); + uint32_t last_pixel = _mesa_image_offset(dims, packing, width, height, + format, type, + depth-1, height-1, width); row_stride = _mesa_image_row_stride(packing, width, format, type); if (_mesa_is_bufferobj(packing->BufferObj)) { *tmp_pbo = 0; buffer_obj = packing->BufferObj; + first_pixel += (intptr_t)pixels; } else { bool is_pixel_pack = pbo_target == GL_PIXEL_PACK_BUFFER; @@ -97,14 +103,18 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, * data to avoid unnecessary data copying in _mesa_BufferData(). */ if (is_pixel_pack) - _mesa_BufferData(pbo_target, row_stride * height, NULL, + _mesa_BufferData(pbo_target, + last_pixel - first_pixel, + NULL, GL_STREAM_READ); else - _mesa_BufferData(pbo_target, row_stride * height, pixels, + _mesa_BufferData(pbo_target, + last_pixel - first_pixel, + (char *)pixels + first_pixel, GL_STREAM_DRAW); buffer_obj = packing->BufferObj; - pixels = NULL; + first_pixel = 0; _mesa_BindBuffer(pbo_target, 0); } @@ -119,14 +129,21 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, internal_format = _mesa_get_format_base_format(pbo_format); + /* The texture is addressed as a single very-tall image, so we + * need to pack the multiple image depths together taking the + * inter-image padding into account. + */ + int image_height = packing->ImageHeight == 0 ? height : packing->ImageHeight; + int full_height = image_height * (depth - 1) + height; + tex_image = _mesa_get_tex_image(ctx, tex_obj, tex_obj->Target, 0); - _mesa_init_teximage_fields(ctx, tex_image, width, height, 1, + _mesa_init_teximage_fields(ctx, tex_image, width, full_height, 1, 0, internal_format, pbo_format); read_only = pbo_target == GL_PIXEL_UNPACK_BUFFER; if (!ctx->Driver.SetTextureStorageForBufferObject(ctx, tex_obj, buffer_obj, - (intptr_t)pixels, + first_pixel, row_stride, read_only)) { _mesa_DeleteTextures(1, tmp_tex); @@ -147,7 +164,7 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims, const struct gl_pixelstore_attrib *packing) { GLuint pbo = 0, pbo_tex = 0, fbos[2] = { 0, 0 }; - int full_height, image_height; + int image_height; struct gl_texture_image *pbo_tex_image; GLenum status; bool success = false; @@ -171,11 +188,10 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims, * property. */ image_height = packing->ImageHeight == 0 ? height : packing->ImageHeight; - full_height = image_height * (depth - 1) + height; pbo_tex_image = create_texture_for_pbo(ctx, create_pbo, GL_PIXEL_UNPACK_BUFFER, - width, full_height, + dims, width, height, depth, format, type, pixels, packing, &pbo, &pbo_tex); if (!pbo_tex_image) @@ -277,7 +293,7 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, GLuint dims, const struct gl_pixelstore_attrib *packing) { GLuint pbo = 0, pbo_tex = 0, fbos[2] = { 0, 0 }; - int full_height, image_height; + int image_height; struct gl_texture_image *pbo_tex_image; struct gl_renderbuffer *rb = NULL; GLenum dstBaseFormat = _mesa_unpack_format_to_base_format(format); @@ -324,10 +340,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, GLuint dims, * property. */ image_height = packing->ImageHeight == 0 ? height : packing->ImageHeight; - full_height = image_height * (depth - 1) + height; pbo_tex_image = create_texture_for_pbo(ctx, false, GL_PIXEL_PACK_BUFFER, - width, full_height * depth, + dims, width, height, depth, format, type, pixels, packing, &pbo, &pbo_tex); if (!pbo_tex_image) -- 2.30.2