[PATCH v2] etnaviv: fix shader miscompilation with more than 16 labels

Lucas Stach l.stach at pengutronix.de
Mon Jun 26 16:24:31 UTC 2017


The labels array may change its virtual address on a reallocation, so
it is invalid to cache pointers into the array. Rather than using the
pointer directly, remember the array index.

Fixes miscompilation of shaders in glmark2 ideas, leading to GPU hangs.

Fixes: c9e8b49b (etnaviv: gallium driver for Vivante GPUs)
Cc: mesa-stable at lists.freedesktop.org
Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
Reviewed-by: Christian Gmeiner <christian.gmeiner at gmail.com>
---
v2: Only fill in labels after checking instruction limit. Fixes out of
    bounds array access.
---
 src/gallium/drivers/etnaviv/etnaviv_compiler.c | 60 ++++++++++++++------------
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler.c b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
index eafb511bb813..af0f76b58649 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_compiler.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
@@ -119,10 +119,10 @@ enum etna_compile_frame_type {
  */
 struct etna_compile_frame {
    enum etna_compile_frame_type type;
-   struct etna_compile_label *lbl_else;
-   struct etna_compile_label *lbl_endif;
-   struct etna_compile_label *lbl_loop_bgn;
-   struct etna_compile_label *lbl_loop_end;
+   int lbl_else_idx;
+   int lbl_endif_idx;
+   int lbl_loop_bgn_idx;
+   int lbl_loop_end_idx;
 };
 
 struct etna_compile_file {
@@ -178,7 +178,7 @@ struct etna_compile {
    /* Fields for handling nested conditionals */
    struct etna_compile_frame frame_stack[ETNA_MAX_DEPTH];
    int frame_sp;
-   struct etna_compile_label *lbl_usage[ETNA_MAX_INSTRUCTIONS];
+   int lbl_usage[ETNA_MAX_INSTRUCTIONS];
 
    unsigned labels_count, labels_sz;
    struct etna_compile_label *labels;
@@ -990,7 +990,7 @@ etna_src_uniforms_conflict(struct etna_inst_src a, struct etna_inst_src b)
 }
 
 /* create a new label */
-static struct etna_compile_label *
+static unsigned int
 alloc_new_label(struct etna_compile *c)
 {
    struct etna_compile_label label = {
@@ -999,7 +999,7 @@ alloc_new_label(struct etna_compile *c)
 
    array_insert(c->labels, label);
 
-   return &c->labels[c->labels_count - 1];
+   return c->labels_count - 1;
 }
 
 /* place label at current instruction pointer */
@@ -1015,10 +1015,10 @@ label_place(struct etna_compile *c, struct etna_compile_label *label)
  * as the value becomes known.
  */
 static void
-label_mark_use(struct etna_compile *c, struct etna_compile_label *label)
+label_mark_use(struct etna_compile *c, int lbl_idx)
 {
    assert(c->inst_ptr < ETNA_MAX_INSTRUCTIONS);
-   c->lbl_usage[c->inst_ptr] = label;
+   c->lbl_usage[c->inst_ptr] = lbl_idx;
 }
 
 /* walk the frame stack and return first frame with matching type */
@@ -1099,8 +1099,8 @@ trans_if(const struct instr_translater *t, struct etna_compile *c,
    /* push IF to stack */
    f->type = ETNA_COMPILE_FRAME_IF;
    /* create "else" label */
-   f->lbl_else = alloc_new_label(c);
-   f->lbl_endif = NULL;
+   f->lbl_else_idx = alloc_new_label(c);
+   f->lbl_endif_idx = -1;
 
    /* We need to avoid the emit_inst() below becoming two instructions */
    if (etna_src_uniforms_conflict(src[0], imm_0))
@@ -1108,7 +1108,7 @@ trans_if(const struct instr_translater *t, struct etna_compile *c,
 
    /* mark position in instruction stream of label reference so that it can be
     * filled in in next pass */
-   label_mark_use(c, f->lbl_else);
+   label_mark_use(c, f->lbl_else_idx);
 
    /* create conditional branch to label if src0 EQ 0 */
    emit_inst(c, &(struct etna_inst){
@@ -1129,8 +1129,8 @@ trans_else(const struct instr_translater *t, struct etna_compile *c,
    assert(f->type == ETNA_COMPILE_FRAME_IF);
 
    /* create "endif" label, and branch to endif label */
-   f->lbl_endif = alloc_new_label(c);
-   label_mark_use(c, f->lbl_endif);
+   f->lbl_endif_idx = alloc_new_label(c);
+   label_mark_use(c, f->lbl_endif_idx);
    emit_inst(c, &(struct etna_inst) {
       .opcode = INST_OPCODE_BRANCH,
       .cond = INST_CONDITION_TRUE,
@@ -1138,7 +1138,7 @@ trans_else(const struct instr_translater *t, struct etna_compile *c,
    });
 
    /* mark "else" label at this position in instruction stream */
-   label_place(c, f->lbl_else);
+   label_place(c, &c->labels[f->lbl_else_idx]);
 }
 
 static void
@@ -1151,10 +1151,10 @@ trans_endif(const struct instr_translater *t, struct etna_compile *c,
 
    /* assign "endif" or "else" (if no ELSE) label to current position in
     * instruction stream, pop IF */
-   if (f->lbl_endif != NULL)
-      label_place(c, f->lbl_endif);
+   if (f->lbl_endif_idx != -1)
+      label_place(c, &c->labels[f->lbl_endif_idx]);
    else
-      label_place(c, f->lbl_else);
+      label_place(c, &c->labels[f->lbl_else_idx]);
 }
 
 static void
@@ -1166,10 +1166,10 @@ trans_loop_bgn(const struct instr_translater *t, struct etna_compile *c,
 
    /* push LOOP to stack */
    f->type = ETNA_COMPILE_FRAME_LOOP;
-   f->lbl_loop_bgn = alloc_new_label(c);
-   f->lbl_loop_end = alloc_new_label(c);
+   f->lbl_loop_bgn_idx = alloc_new_label(c);
+   f->lbl_loop_end_idx = alloc_new_label(c);
 
-   label_place(c, f->lbl_loop_bgn);
+   label_place(c, &c->labels[f->lbl_loop_bgn_idx]);
 
    c->num_loops++;
 }
@@ -1185,7 +1185,7 @@ trans_loop_end(const struct instr_translater *t, struct etna_compile *c,
 
    /* mark position in instruction stream of label reference so that it can be
     * filled in in next pass */
-   label_mark_use(c, f->lbl_loop_bgn);
+   label_mark_use(c, f->lbl_loop_bgn_idx);
 
    /* create branch to loop_bgn label */
    emit_inst(c, &(struct etna_inst) {
@@ -1195,7 +1195,7 @@ trans_loop_end(const struct instr_translater *t, struct etna_compile *c,
       /* imm is filled in later */
    });
 
-   label_place(c, f->lbl_loop_end);
+   label_place(c, &c->labels[f->lbl_loop_end_idx]);
 }
 
 static void
@@ -1207,7 +1207,7 @@ trans_brk(const struct instr_translater *t, struct etna_compile *c,
 
    /* mark position in instruction stream of label reference so that it can be
     * filled in in next pass */
-   label_mark_use(c, f->lbl_loop_end);
+   label_mark_use(c, f->lbl_loop_end_idx);
 
    /* create branch to loop_end label */
    emit_inst(c, &(struct etna_inst) {
@@ -1227,7 +1227,7 @@ trans_cont(const struct instr_translater *t, struct etna_compile *c,
 
    /* mark position in instruction stream of label reference so that it can be
     * filled in in next pass */
-   label_mark_use(c, f->lbl_loop_bgn);
+   label_mark_use(c, f->lbl_loop_bgn_idx);
 
    /* create branch to loop_end label */
    emit_inst(c, &(struct etna_inst) {
@@ -1998,8 +1998,9 @@ static void
 etna_compile_fill_in_labels(struct etna_compile *c)
 {
    for (int idx = 0; idx < c->inst_ptr; ++idx) {
-      if (c->lbl_usage[idx])
-         etna_assemble_set_imm(&c->code[idx * 4], c->lbl_usage[idx]->inst_idx);
+      if (c->lbl_usage[idx] != -1)
+         etna_assemble_set_imm(&c->code[idx * 4],
+                               c->labels[c->lbl_usage[idx]].inst_idx);
    }
 }
 
@@ -2301,6 +2302,8 @@ etna_compile_shader(struct etna_shader_variant *v)
    if (!c)
       return false;
 
+   memset(&c->lbl_usage, -1, ARRAY_SIZE(c->lbl_usage));
+
    const struct tgsi_token *tokens = v->shader->tokens;
 
    c->specs = specs;
@@ -2430,12 +2433,13 @@ etna_compile_shader(struct etna_shader_variant *v)
    etna_compile_add_z_div_if_needed(c);
    etna_compile_frag_rb_swap(c);
    etna_compile_add_nop_if_needed(c);
-   etna_compile_fill_in_labels(c);
 
    ret = etna_compile_check_limits(c);
    if (!ret)
       goto out;
 
+   etna_compile_fill_in_labels(c);
+
    /* fill in output structure */
    v->processor = c->info.processor;
    v->code_size = c->inst_ptr * 4;
-- 
2.11.0



More information about the etnaviv mailing list