[Mesa-dev] [PATCH 3/6] glsl/linker: always validate explicit location among inputs

[Timothy Arceri]

Nice catch!

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

On 2/2/19 5:05 am, Andres Gomez wrote:
> Outputs are always validated when having explicit locations and we
> were trusting its outcome to catch similar problems with the inputs
> since, in case of having undefined outputs for existing inputs, we
> would be already reporting a linker error.
> 
> However, consider this case:
> 
>    " Shader stage n:
>      ---------------
> 
>      ...
> 
>      layout(location = 0) out float a;
> 
>      ...
> 
>      Shader stage n+1:
>      -----------------
> 
>      ...
> 
>      layout(location = 0) in float b;
>      layout(location = 0) in float c;
> 
>      ...
>    "
> 
> Currently, this won't report a linker error even though location
> aliasing is happening for the inputs.
> 
> Therefore, we also need to validate the inputs independently from the
> outcome of the outputs validation.
> 
> Cc: Timothy Arceri <tarceri at itsqueeze.com>
> Cc: Iago Toral Quiroga <itoral at igalia.com>
> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> Signed-off-by: Andres Gomez <agomez at igalia.com>
> ---
>   src/compiler/glsl/link_varyings.cpp | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
> index 3f41832ac93..e5f7d3e322a 100644
> --- a/src/compiler/glsl/link_varyings.cpp
> +++ b/src/compiler/glsl/link_varyings.cpp
> @@ -715,7 +715,8 @@ cross_validate_outputs_to_inputs(struct gl_context *ctx,
>                                    gl_linked_shader *consumer)
>   {
>      glsl_symbol_table parameters;
> -   struct explicit_location_info explicit_locations[MAX_VARYING][4] = { 0 };
> +   struct explicit_location_info output_explicit_locations[MAX_VARYING][4] = { 0 };
> +   struct explicit_location_info input_explicit_locations[MAX_VARYING][4] = { 0 };
>   
>      /* Find all shader outputs in the "producer" stage.
>       */
> @@ -733,7 +734,7 @@ cross_validate_outputs_to_inputs(struct gl_context *ctx,
>             * differently because they do not need to have matching names.
>             */
>            if (!validate_explicit_variable_location(ctx,
> -                                                  explicit_locations,
> +                                                  output_explicit_locations,
>                                                     var, prog, producer)) {
>               return;
>            }
> @@ -791,6 +792,12 @@ cross_validate_outputs_to_inputs(struct gl_context *ctx,
>                  compute_variable_location_slot(input, consumer->Stage);
>               unsigned slot_limit = idx + num_elements;
>   
> +            if (!validate_explicit_variable_location(ctx,
> +                                                     input_explicit_locations,
> +                                                     input, prog, consumer)) {
> +               return;
> +            }
> +
>               while (idx < slot_limit) {
>                  if (idx >= MAX_VARYING) {
>                     linker_error(prog,
> @@ -799,7 +806,7 @@ cross_validate_outputs_to_inputs(struct gl_context *ctx,
>                     return;
>                  }
>   
> -               output = explicit_locations[idx][input->data.location_frac].var;
> +               output = output_explicit_locations[idx][input->data.location_frac].var;
>   
>                  if (output == NULL ||
>                      input->data.location != output->data.location) {
>