[Mesa-dev] [PATCH] glsl/linker: fix output variable overlap check

[Nicolai Hähnle]

From: Nicolai Hähnle <nicolai.haehnle at amd.com>

Prevent an overflow caused by too many output variables. To limit the
scope of the issue, write to the assigned array only for the non-ES
fragment shader path, which is the only place where it's needed.

Since the function will bail with an error when output variables with
overlapping components are found, (max # of FS outputs) * 4 is an upper
limit to the space we need.

Found by address sanitizer.

Fixes dEQP-GLES3.functional.attribute_location.bind_aliasing.*

Cc: mesa-stable at lists.freedesktop.org
---
 src/compiler/glsl/linker.cpp | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 5c3f1d12bbc..ddd8301a739 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -2647,26 +2647,28 @@ assign_attribute_or_color_locations(void *mem_ctx,
       {
          const temp_attr *const l = (const temp_attr *) a;
          const temp_attr *const r = (const temp_attr *) b;
 
          /* Reversed because we want a descending order sort below. */
          return r->slots - l->slots;
       }
    } to_assign[32];
    assert(max_index <= 32);
 
-   /* Temporary array for the set of attributes that have locations assigned.
+   /* Temporary array for the set of attributes that have locations assigned,
+    * for the purpose of checking overlapping slots/components of (non-ES)
+    * fragment shader outputs.
     */
-   ir_variable *assigned[16];
+   ir_variable *assigned[12 * 4];
+   unsigned assigned_attr = 0;
 
    unsigned num_attr = 0;
-   unsigned assigned_attr = 0;
 
    foreach_in_list(ir_instruction, node, sh->ir) {
       ir_variable *const var = node->as_variable();
 
       if ((var == NULL) || (var->data.mode != (unsigned) direction))
          continue;
 
       if (var->data.explicit_location) {
          var->data.is_unmatched_generic_inout = 0;
          if ((var->data.location >= (int)(max_index + generic_base))
@@ -2878,20 +2880,26 @@ assign_attribute_or_color_locations(void *mem_ctx,
                         if (assigned_component_mask & component_mask) {
                            linker_error(prog, "overlapping component is "
                                         "assigned to %ss %s and %s "
                                         "(component=%d)\n",
                                         string, assigned[i]->name, var->name,
                                         var->data.location_frac);
                            return false;
                         }
                      }
                   }
+
+                  /* At most one variable per fragment output component should
+                   * reach this. */
+                  assert(assigned_attr < ARRAY_SIZE(assigned));
+                  assigned[assigned_attr] = var;
+                  assigned_attr++;
                } else if (target_index == MESA_SHADER_FRAGMENT ||
                           (prog->IsES && prog->data->Version >= 300)) {
                   linker_error(prog, "overlapping location is assigned "
                                "to %s `%s' %d %d %d\n", string, var->name,
                                used_locations, use_mask, attr);
                   return false;
                } else {
                   linker_warning(prog, "overlapping location is assigned "
                                  "to %s `%s' %d %d %d\n", string, var->name,
                                  used_locations, use_mask, attr);
@@ -2917,23 +2925,20 @@ assign_attribute_or_color_locations(void *mem_ctx,
              *
              * Mark this attribute slot as taking up twice as much space
              * so we can count it properly against limits.  According to
              * issue (3) of the GL_ARB_vertex_attrib_64bit behavior, this
              * is optional behavior, but it seems preferable.
              */
             if (var->type->without_array()->is_dual_slot())
                double_storage_locations |= (use_mask << attr);
          }
 
-         assigned[assigned_attr] = var;
-         assigned_attr++;
-
          continue;
       }
 
       if (num_attr >= max_index) {
          linker_error(prog, "too many %s (max %u)",
                       target_index == MESA_SHADER_VERTEX ?
                       "vertex shader inputs" : "fragment shader outputs",
                       max_index);
          return false;
       }
-- 
2.11.0