This patch regressed piglit and gl cts tests for the i965 driver https://bugs.freedesktop.org/show_bug.cgi?id=102904
Nicolai Hähnle <[email protected]> writes: > From: Nicolai Hähnle <[email protected]> > > 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: [email protected] > --- > 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 > > _______________________________________________ > mesa-dev mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
