From 052f4952f2836fe46792db85219001591458bce6 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: (cherry picked from commit 2b6993cb71a86e15af7523e1e436f2722e509dc1) --- .pick_status.json | 2 +- src/gallium/drivers/r300/ci/r300-r480-fails.txt | 1 - .../drivers/r300/ci/r300-rv370-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 +++++++++-------- 7 files changed, 25 insertions(+), 17 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 946ea1b4bc5..8c4dacb31f1 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -3734,7 +3734,7 @@ "description": "r300: fix r300_draw_elements() behavior", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "330d0607ed60fd3edca192e54b4246310f06652f", "notes": null 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-rv370-fails.txt b/src/gallium/drivers/r300/ci/r300-rv370-fails.txt index 0c43c21baff..cd0b1b7b85d 100644 --- a/src/gallium/drivers/r300/ci/r300-rv370-fails.txt +++ b/src/gallium/drivers/r300/ci/r300-rv370-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.completeness.renderable.texture.color0.rgba_half_float_oes,Fail dEQP-GLES2.functional.fbo.render.repeated_clear.tex2d_rgb,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 07fa9e6435b..21a8043dcb8 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 d7a784ac083..ff0962e835e 100644 --- a/src/gallium/drivers/r300/r300_context.h +++ b/src/gallium/drivers/r300/r300_context.h @@ -744,7 +744,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 858d1798b48..978d7d6c59d 100644 --- a/src/gallium/drivers/r300/r300_render.c +++ b/src/gallium/drivers/r300/r300_render.c @@ -601,6 +601,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, @@ -608,7 +609,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) { @@ -628,10 +629,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 f3749815773..32d6a2ec5a9 100644 --- a/src/gallium/drivers/r300/r300_render_translate.c +++ b/src/gallium/drivers/r300/r300_render_translate.c @@ -29,20 +29,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; @@ -52,12 +53,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; } @@ -67,12 +68,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; }