From 54ec9c95cff7d98221b361c49ee2d43a4c149e88 Mon Sep 17 00:00:00 2001 From: "Juan A. Suarez Romero" Date: Fri, 30 Apr 2021 18:44:02 +0200 Subject: [PATCH] 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 Signed-off-by: Juan A. Suarez Romero Part-of: --- src/broadcom/compiler/vir_register_allocate.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/broadcom/compiler/vir_register_allocate.c b/src/broadcom/compiler/vir_register_allocate.c index 1e4f4e9fe1e..6089179ce49 100644 --- a/src/broadcom/compiler/vir_register_allocate.c +++ b/src/broadcom/compiler/vir_register_allocate.c @@ -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++) { - int node = temp_to_node[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); @@ -224,7 +222,7 @@ v3d_emit_tmu_spill(struct v3d_compile *c, struct qinst *inst, struct qinst *position, uint32_t spill_offset) { 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, V3D_QPU_WADDR_TMUD), inst->dst); @@ -604,6 +602,7 @@ tmu_spilling_allowed(struct v3d_compile *c, int thread_index) struct qpu_reg * 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]; uint32_t temp_to_node[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; } + /* 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, sizeof(*temp_registers));