Skip to content

Commit

Permalink
r300: fix r300_draw_elements() behavior
Browse files Browse the repository at this point in the history
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: 330d060 ("gallium: remove pipe_index_buffer and set_index_buffer")
Signed-off-by: Patrick Lerda <[email protected]>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28523>
(cherry picked from commit 2b6993c)
  • Loading branch information
Patrick Lerda authored and 1ace committed Apr 21, 2024
1 parent c4b6f6b commit 052f495
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 17 deletions.
2 changes: 1 addition & 1 deletion .pick_status.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/gallium/drivers/r300/ci/r300-r480-fails.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/gallium/drivers/r300/ci/r300-rv370-fails.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/gallium/drivers/r300/ci/r300-rv530-nohiz-fails.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/gallium/drivers/r300/r300_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
17 changes: 13 additions & 4 deletions src/gallium/drivers/r300/r300_render.c
Original file line number Diff line number Diff line change
Expand Up @@ -601,14 +601,15 @@ 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,
&index_offset);
}

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) {
Expand All @@ -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. */
Expand Down
17 changes: 9 additions & 8 deletions src/gallium/drivers/r300/r300_render_translate.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down

0 comments on commit 052f495

Please sign in to comment.