From 2b6993cb71a86e15af7523e1e436f2722e509dc1 Mon Sep 17 00:00:00 2001 From: Patrick Lerda Date: Wed, 27 Mar 2024 23:19:38 +0100 Subject: [PATCH] r300: fix r300_draw_elements() behavior Indeed, the pointer processed by r300_upload_index_buffer() was not the right one. This is the reason why "deqp-gles2 --deqp-case=dEQP-GLES2.functional.draw.draw_elements.indices.user_ptr.index_byte" was failing (the logs are below). This change corrects this issue and makes the related deqp tests work properly. This change considers that r300_upload_index_buffer() sets indexBuffer to NULL. The indexBuffer resource should be properly freed once the buffer is processed. This is required to avoid another refcnt imbalance (another kind of memory leak). ==9962==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000721f at pc 0x7fd57b54a9a0 bp 0x7fffd2c39290 sp 0x7fffd2c38a40 READ of size 30 at 0x60200000721f thread T0 #0 0x7fd57b54a99f in __interceptor_memcpy (/usr/lib64/libasan.so.6.0.0+0x3c99f) #1 0x7fd570d10528 in u_upload_data ../src/gallium/auxiliary/util/u_upload_mgr.c:333 #2 0x7fd57114142b in r300_upload_index_buffer ../src/gallium/drivers/r300/r300_screen_buffer.c:44 #3 0x7fd57113943c in r300_draw_elements ../src/gallium/drivers/r300/r300_render.c:632 #4 0x7fd57113bbc4 in r300_draw_vbo ../src/gallium/drivers/r300/r300_render.c:840 #5 0x7fd570d212e2 in u_vbuf_draw_vbo ../src/gallium/auxiliary/util/u_vbuf.c:1487 #6 0x7fd56fceb873 in _mesa_validated_drawrangeelements ../src/mesa/main/draw.c:1709 #7 0x7fd56fcf28c5 in _mesa_DrawElementsBaseVertex ../src/mesa/main/draw.c:1852 Fixes: 330d0607ed6 ("gallium: remove pipe_index_buffer and set_index_buffer") Signed-off-by: Patrick Lerda Part-of: --- src/gallium/drivers/r300/ci/r300-r480-fails.txt | 1 - .../drivers/r300/ci/r300-rv380-fails.txt | 1 - .../drivers/r300/ci/r300-rv530-nohiz-fails.txt | 1 - src/gallium/drivers/r300/r300_context.h | 3 ++- src/gallium/drivers/r300/r300_render.c | 17 +++++++++++++---- .../drivers/r300/r300_render_translate.c | 17 +++++++++-------- 6 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/gallium/drivers/r300/ci/r300-r480-fails.txt b/src/gallium/drivers/r300/ci/r300-r480-fails.txt index d092bbe3b38..a18f099845b 100644 --- a/src/gallium/drivers/r300/ci/r300-r480-fails.txt +++ b/src/gallium/drivers/r300/ci/r300-r480-fails.txt @@ -3,7 +3,6 @@ dEQP-GLES2.functional.clipping.line.wide_line_clip_viewport_corner,Fail dEQP-GLES2.functional.clipping.point.wide_point_clip,Fail dEQP-GLES2.functional.clipping.point.wide_point_clip_viewport_center,Fail dEQP-GLES2.functional.clipping.point.wide_point_clip_viewport_corner,Fail -dEQP-GLES2.functional.draw.draw_elements.indices.user_ptr.index_byte,Fail dEQP-GLES2.functional.fbo.completeness.renderable.texture.color0.rgb_half_float_oes,Fail dEQP-GLES2.functional.fbo.render.repeated_clear.tex2d_rgb,Fail dEQP-GLES2.functional.fbo.render.repeated_clear.tex2d_rgba,Fail diff --git a/src/gallium/drivers/r300/ci/r300-rv380-fails.txt b/src/gallium/drivers/r300/ci/r300-rv380-fails.txt index 35e3dbde358..3e8fead58f1 100644 --- a/src/gallium/drivers/r300/ci/r300-rv380-fails.txt +++ b/src/gallium/drivers/r300/ci/r300-rv380-fails.txt @@ -3,7 +3,6 @@ dEQP-GLES2.functional.clipping.line.wide_line_clip_viewport_corner,Fail dEQP-GLES2.functional.clipping.point.wide_point_clip,Fail dEQP-GLES2.functional.clipping.point.wide_point_clip_viewport_center,Fail dEQP-GLES2.functional.clipping.point.wide_point_clip_viewport_corner,Fail -dEQP-GLES2.functional.draw.draw_elements.indices.user_ptr.index_byte,Fail dEQP-GLES2.functional.fbo.completeness.renderable.texture.color0.rgb_half_float_oes,Fail dEQP-GLES2.functional.fbo.render.repeated_clear.tex2d_rgb,Fail dEQP-GLES2.functional.fbo.render.repeated_clear.tex2d_rgba,Fail diff --git a/src/gallium/drivers/r300/ci/r300-rv530-nohiz-fails.txt b/src/gallium/drivers/r300/ci/r300-rv530-nohiz-fails.txt index 4d0ce1121ee..55dfa49640c 100644 --- a/src/gallium/drivers/r300/ci/r300-rv530-nohiz-fails.txt +++ b/src/gallium/drivers/r300/ci/r300-rv530-nohiz-fails.txt @@ -5,7 +5,6 @@ dEQP-GLES2.functional.clipping.point.wide_point_clip_viewport_corner,Fail dEQP-GLES2.functional.clipping.line.wide_line_clip_viewport_center,Fail dEQP-GLES2.functional.clipping.line.wide_line_clip_viewport_corner,Fail -dEQP-GLES2.functional.draw.draw_elements.indices.user_ptr.index_byte,Fail # "Framebuffer checked as complete, expected incomplete" dEQP-GLES2.functional.fbo.completeness.renderable.texture.color0.rgb_half_float_oes,Fail diff --git a/src/gallium/drivers/r300/r300_context.h b/src/gallium/drivers/r300/r300_context.h index 8a7baa44450..bd1be07ef1d 100644 --- a/src/gallium/drivers/r300/r300_context.h +++ b/src/gallium/drivers/r300/r300_context.h @@ -727,7 +727,8 @@ void r300_translate_index_buffer(struct r300_context *r300, const struct pipe_draw_info *info, struct pipe_resource **out_index_buffer, unsigned *index_size, unsigned index_offset, - unsigned *start, unsigned count); + unsigned *start, unsigned count, + const uint8_t **export_ptr); /* r300_render_stencilref.c */ void r300_plug_in_stencil_ref_fallback(struct r300_context *r300); diff --git a/src/gallium/drivers/r300/r300_render.c b/src/gallium/drivers/r300/r300_render.c index 9704be6836e..d3ed9f8ecc8 100644 --- a/src/gallium/drivers/r300/r300_render.c +++ b/src/gallium/drivers/r300/r300_render.c @@ -584,6 +584,7 @@ static void r300_draw_elements(struct r300_context *r300, unsigned short_count; int buffer_offset = 0, index_offset = 0; /* for index bias emulation */ uint16_t indices3[3]; + const uint8_t *local_ptr = info->index.user; if (draw->index_bias && !r300->screen->caps.is_r500) { r300_split_index_bias(r300, draw->index_bias, &buffer_offset, @@ -591,7 +592,7 @@ static void r300_draw_elements(struct r300_context *r300, } r300_translate_index_buffer(r300, info, &indexBuffer, - &indexSize, index_offset, &start, count); + &indexSize, index_offset, &start, count, &local_ptr); /* Fallback for misaligned ushort indices. */ if (indexSize == 2 && (start & 1) && indexBuffer) { @@ -611,10 +612,18 @@ static void r300_draw_elements(struct r300_context *r300, count, (uint8_t*)ptr); } } else { - if (info->has_user_indices) - r300_upload_index_buffer(r300, &indexBuffer, indexSize, + if (info->has_user_indices) { + struct pipe_resource* indexSaved = indexBuffer; + + if (local_ptr != info->index.user) + start = 0; + + r300_upload_index_buffer(r300, &indexBuffer, indexSize, &start, count, - info->index.user); + local_ptr); + + pipe_resource_reference(&indexSaved, NULL); + } } /* 19 dwords for emit_draw_elements. Give up if the function fails. */ diff --git a/src/gallium/drivers/r300/r300_render_translate.c b/src/gallium/drivers/r300/r300_render_translate.c index a143539157b..b579960c49a 100644 --- a/src/gallium/drivers/r300/r300_render_translate.c +++ b/src/gallium/drivers/r300/r300_render_translate.c @@ -12,20 +12,21 @@ void r300_translate_index_buffer(struct r300_context *r300, const struct pipe_draw_info *info, struct pipe_resource **out_buffer, unsigned *index_size, unsigned index_offset, - unsigned *start, unsigned count) + unsigned *start, unsigned count, + const uint8_t **export_ptr) { unsigned out_offset; - void *ptr; + void **ptr = (void **)export_ptr; switch (*index_size) { case 1: *out_buffer = NULL; u_upload_alloc(r300->uploader, 0, count * 2, 4, - &out_offset, out_buffer, &ptr); + &out_offset, out_buffer, ptr); util_shorten_ubyte_elts_to_userptr( &r300->context, info, PIPE_MAP_UNSYNCHRONIZED, index_offset, - *start, count, ptr); + *start, count, *ptr); *index_size = 2; *start = out_offset / 2; @@ -35,12 +36,12 @@ void r300_translate_index_buffer(struct r300_context *r300, if (index_offset) { *out_buffer = NULL; u_upload_alloc(r300->uploader, 0, count * 2, 4, - &out_offset, out_buffer, &ptr); + &out_offset, out_buffer, ptr); util_rebuild_ushort_elts_to_userptr(&r300->context, info, PIPE_MAP_UNSYNCHRONIZED, index_offset, *start, - count, ptr); + count, *ptr); *start = out_offset / 2; } @@ -50,12 +51,12 @@ void r300_translate_index_buffer(struct r300_context *r300, if (index_offset) { *out_buffer = NULL; u_upload_alloc(r300->uploader, 0, count * 4, 4, - &out_offset, out_buffer, &ptr); + &out_offset, out_buffer, ptr); util_rebuild_uint_elts_to_userptr(&r300->context, info, PIPE_MAP_UNSYNCHRONIZED, index_offset, *start, - count, ptr); + count, *ptr); *start = out_offset / 4; }