broadcom/compiler: fix dynamic-stack-buffer-overflow error

When spilling a register, the number of temps can be increased when
introducing a temporal variable.

Those nodes are not elegible to be spilled, but we need to take care of
no accessing out-of-bounds of the arrays defined with a size equal to
the original number of temps.

Fixes address sanitizer error on
KHR-GLES3.shaders.uniform_block.random.all_shared_buffer.14 (and many
others).

v2 (Iago):
 - Add clarification in assertion.
 - Use `vir_get_temp` to increase num_temps.

v3 (Iago):
 - Update clarification

Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10643>
This commit is contained in:
Juan A. Suarez Romero
2021-04-30 18:44:02 +02:00
committed by Marge Bot
parent cf030ceef3
commit 54ec9c95cf

View File

@@ -164,10 +164,8 @@ v3d_choose_spill_node(struct v3d_compile *c, struct ra_graph *g,
} }
for (unsigned i = 0; i < c->num_temps; i++) { for (unsigned i = 0; i < c->num_temps; i++) {
int node = temp_to_node[i];
if (BITSET_TEST(c->spillable, i)) if (BITSET_TEST(c->spillable, i))
ra_set_node_spill_cost(g, node, spill_costs[i]); ra_set_node_spill_cost(g, temp_to_node[i], spill_costs[i]);
} }
return ra_get_best_spill_node(g); return ra_get_best_spill_node(g);
@@ -224,7 +222,7 @@ v3d_emit_tmu_spill(struct v3d_compile *c, struct qinst *inst,
struct qinst *position, uint32_t spill_offset) struct qinst *position, uint32_t spill_offset)
{ {
c->cursor = vir_after_inst(position); c->cursor = vir_after_inst(position);
inst->dst.index = c->num_temps++; inst->dst = vir_get_temp(c);
vir_MOV_dest(c, vir_reg(QFILE_MAGIC, vir_MOV_dest(c, vir_reg(QFILE_MAGIC,
V3D_QPU_WADDR_TMUD), V3D_QPU_WADDR_TMUD),
inst->dst); inst->dst);
@@ -604,6 +602,7 @@ tmu_spilling_allowed(struct v3d_compile *c, int thread_index)
struct qpu_reg * struct qpu_reg *
v3d_register_allocate(struct v3d_compile *c, bool *spilled) v3d_register_allocate(struct v3d_compile *c, bool *spilled)
{ {
uint32_t UNUSED start_num_temps = c->num_temps;
struct node_to_temp_map map[c->num_temps]; struct node_to_temp_map map[c->num_temps];
uint32_t temp_to_node[c->num_temps]; uint32_t temp_to_node[c->num_temps];
uint8_t class_bits[c->num_temps]; uint8_t class_bits[c->num_temps];
@@ -855,6 +854,12 @@ v3d_register_allocate(struct v3d_compile *c, bool *spilled)
return NULL; return NULL;
} }
/* Ensure that we are not accessing temp_to_node out of bounds. We
* should never trigger this assertion because `c->num_temps` only
* grows when we spill, in which case we return early and don't get
* here.
*/
assert(start_num_temps == c->num_temps);
struct qpu_reg *temp_registers = calloc(c->num_temps, struct qpu_reg *temp_registers = calloc(c->num_temps,
sizeof(*temp_registers)); sizeof(*temp_registers));