fdo-mirrors/mesa
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/mesa/mesa.git synced 2026-05-03 16:28:08 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

mesa/st: Make sure to unbind cb0 on transition away from gs/tess shaders.

Browse source 
This atom tries to unbind cb0 when it's not used any more (the params &&
params->NumParameters check), but if you transitioned to not having a
gs/tess enabled at all, you'd skip unbinding it.  This was mostly
harmless, since if you don't have a GS, why are you looking at GS
constants?  However, if a new program came along that didn't use cb0 at
all, we wouldn't end up in this atom to get the disable, and now you have
a GS enabled but a GS constbuf pointing at potentially freed data.

Dereferencing the freed cb0 data ended up happening in freedreno's
fallback UBO upload path with this combination of tests (which execute in
that order):

dEQP-GLES31.functional.shaders.opaque_type_indexing.sampler.uniform.geometry.sampler2darray
dEQP-GLES31.functional.shaders.opaque_type_indexing.ubo.const_literal_fragment
dEQP-GLES31.functional.shaders.opaque_type_indexing.ubo.dynamically_uniform_geometry

and it seems also affected softpipe as well.

Reviewed-by: Rob Clark <robdclark@chromium.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9070>
This commit is contained in:
Eric Anholt 2021-02-15 14:52:12 -08:00 • committed by Marge Bot
parent a687e71afd
commit 82392e0bb9
5 changed files with 33 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
ci-expects/softpipe/deqp-softpipe-asan-fails.txt
View file

@ -81,7 +81,6 @@ dEQP-GLES31.functional.shaders.linkage.es32.tessellation.varying.types.uvec2,Mis
dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.uniform_geometry,Missing
dEQP-GLES31.functional.shaders.opaque_type_indexing.sampler.const_expression.tessellation_control.isampler2d,Missing
dEQP-GLES31.functional.shaders.opaque_type_indexing.sampler.const_literal.compute.isampler2darray,Missing
dEQP-GLES31.functional.shaders.opaque_type_indexing.ubo.dynamically_uniform_geometry,Crash
dEQP-GLES31.functional.ssbo.layout.2_level_array.packed.row_major_mat4x2,Missing
dEQP-GLES31.functional.ssbo.layout.basic_unsized_array.packed.uvec3,Missing
dEQP-GLES31.functional.ssbo.layout.random.nested_structs.22,Missing

56
src/mesa/state_tracker/st_atom_constbuf.c
View file

@ -48,16 +48,34 @@
#include "st_program.h"
#include "st_cb_bufferobjects.h"
/* Unbinds the CB0 if it's not used by the current program to avoid leaving
* dangling pointers to old (potentially deleted) shaders in the driver.
*/
static void
st_unbind_unused_cb0(struct st_context *st, enum pipe_shader_type shader_type)
{
if (st->state.constbuf0_enabled_shader_mask & (1 << shader_type)) {
struct pipe_context *pipe = st->pipe;
pipe->set_constant_buffer(pipe, shader_type, 0, false, NULL);
st->state.constbuf0_enabled_shader_mask &= ~(1 << shader_type);
}
}
/**
* Pass the given program parameters to the graphics pipe as a
* constant buffer.
*/
void
st_upload_constants(struct st_context *st, struct gl_program *prog)
st_upload_constants(struct st_context *st, struct gl_program *prog, gl_shader_stage stage)
{
gl_shader_stage stage = prog->info.stage;
struct gl_program_parameter_list *params = prog->Parameters;
enum pipe_shader_type shader_type = pipe_shader_type_from_mesa(stage);
if (!prog) {
st_unbind_unused_cb0(st, shader_type);
return;
}
struct gl_program_parameter_list *params = prog->Parameters;
assert(shader_type == PIPE_SHADER_VERTEX ||
shader_type == PIPE_SHADER_FRAGMENT ||
@ -179,12 +197,8 @@ st_upload_constants(struct st_context *st, struct gl_program *prog)
}
st->state.constbuf0_enabled_shader_mask |= 1 << shader_type;
} else if (st->state.constbuf0_enabled_shader_mask & (1 << shader_type)) {
/* Unbind. */
struct pipe_context *pipe = st->pipe;
pipe->set_constant_buffer(pipe, shader_type, 0, false, NULL);
st->state.constbuf0_enabled_shader_mask &= ~(1 << shader_type);
} else {
st_unbind_unused_cb0(st, shader_type);
}
}
@ -195,7 +209,7 @@ st_upload_constants(struct st_context *st, struct gl_program *prog)
void
st_update_vs_constants(struct st_context *st)
{
st_upload_constants(st, &st->vp->Base);
st_upload_constants(st, &st->vp->Base, MESA_SHADER_VERTEX);
}
/**
@ -204,7 +218,7 @@ st_update_vs_constants(struct st_context *st)
void
st_update_fs_constants(struct st_context *st)
{
st_upload_constants(st, &st->fp->Base);
st_upload_constants(st, &st->fp->Base, MESA_SHADER_FRAGMENT);
}
@ -213,10 +227,7 @@ st_update_fs_constants(struct st_context *st)
void
st_update_gs_constants(struct st_context *st)
{
struct st_program *gp = st->gp;
if (gp)
st_upload_constants(st, &gp->Base);
st_upload_constants(st, st->gp ? &st->gp->Base : NULL, MESA_SHADER_GEOMETRY);
}
/* Tessellation control shader:
@ -224,10 +235,7 @@ st_update_gs_constants(struct st_context *st)
void
st_update_tcs_constants(struct st_context *st)
{
struct st_program *tcp = st->tcp;
if (tcp)
st_upload_constants(st, &tcp->Base);
st_upload_constants(st, st->tcp ? &st->tcp->Base : NULL, MESA_SHADER_TESS_CTRL);
}
/* Tessellation evaluation shader:
@ -235,10 +243,7 @@ st_update_tcs_constants(struct st_context *st)
void
st_update_tes_constants(struct st_context *st)
{
struct st_program *tep = st->tep;
if (tep)
st_upload_constants(st, &tep->Base);
st_upload_constants(st, st->tep ? &st->tep->Base : NULL, MESA_SHADER_TESS_EVAL);
}
/* Compute shader:
@ -246,10 +251,7 @@ st_update_tes_constants(struct st_context *st)
void
st_update_cs_constants(struct st_context *st)
{
struct st_program *cp = st->cp;
if (cp)
st_upload_constants(st, &cp->Base);
st_upload_constants(st, st->cp ? &st->cp->Base : NULL, MESA_SHADER_COMPUTE);
}
static void

2
src/mesa/state_tracker/st_atom_constbuf.h
View file

@ -35,7 +35,7 @@ struct gl_program_parameter_list;
struct st_context;
void st_upload_constants(struct st_context *st, struct gl_program *prog);
void st_upload_constants(struct st_context *st, struct gl_program *prog, gl_shader_stage stage);
#endif /* ST_ATOM_CONSTBUF_H */

2
src/mesa/state_tracker/st_cb_bitmap.c
View file

@ -195,7 +195,7 @@ setup_render_state(struct gl_context *ctx,
GLfloat colorSave[4];
COPY_4V(colorSave, ctx->Current.Attrib[VERT_ATTRIB_COLOR0]);
COPY_4V(ctx->Current.Attrib[VERT_ATTRIB_COLOR0], color);
st_upload_constants(st, &st->fp->Base);
st_upload_constants(st, &st->fp->Base, MESA_SHADER_FRAGMENT);
COPY_4V(ctx->Current.Attrib[VERT_ATTRIB_COLOR0], colorSave);
}

4
src/mesa/state_tracker/st_cb_drawpixels.c
View file

@ -1388,7 +1388,7 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y,
/* compiling a new fragment shader variant added new state constants
* into the constant buffer, we need to update them
*/
st_upload_constants(st, &st->fp->Base);
st_upload_constants(st, &st->fp->Base, MESA_SHADER_FRAGMENT);
}
{
@ -1781,7 +1781,7 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint srcy,
/* compiling a new fragment shader variant added new state constants
* into the constant buffer, we need to update them
*/
st_upload_constants(st, &st->fp->Base);
st_upload_constants(st, &st->fp->Base, MESA_SHADER_FRAGMENT);
} else if (type == GL_DEPTH) {
rbRead = st_renderbuffer(ctx->ReadBuffer->
Attachment[BUFFER_DEPTH].Renderbuffer);