[Mesa-dev] [PATCH 11/16] glsl: fix binding validation for interface blocks

[Timothy Arceri]

On Sat, 2015-07-18 at 11:25 +1000, Timothy Arceri wrote:
> ---
>  src/glsl/ast_to_hir.cpp | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 1e3686c..341f9c4 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2041,10 +2041,10 @@ validate_matrix_layout_for_type(struct 
> _mesa_glsl_parse_state *state,
>  static bool
>  validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
>                             YYLTYPE *loc,
> -                           ir_variable *var,
> +                           const glsl_type *type,
>                             const ast_type_qualifier *qual)
>  {
> -   if (var->data.mode != ir_var_uniform) {
> +   if (!qual->flags.q.uniform) {

I forgot to rebase before sending this out this is line is now:

if (!qual->flags.q.uniform && !qual->flags.q.buffer) {

>        _mesa_glsl_error(loc, state,
>                         "the \"binding\" qualifier only applies to 
> uniforms");
>        return false;
> @@ -2056,10 +2056,11 @@ validate_binding_qualifier(struct 
> _mesa_glsl_parse_state *state,
>     }
>  
>     const struct gl_context *const ctx = state->ctx;
> -   unsigned elements = var->type->is_array() ? var->type->length : 1;
> +   unsigned elements = type->is_array() ? type->length : 1;
>     unsigned max_index = qual->binding + elements - 1;
> +   const glsl_type *base_type = type->without_array();
>  
> -   if (var->type->is_interface()) {
> +   if (base_type->is_interface()) {
>        /* UBOs.  From page 60 of the GLSL 4.20 specification:
>         * "If the binding point for any uniform block instance is less than 
> zero,
>         *  or greater than or equal to the implementation-dependent maximum
> @@ -2077,8 +2078,7 @@ validate_binding_qualifier(struct 
> _mesa_glsl_parse_state *state,
>                            ctx->Const.MaxUniformBufferBindings);
>           return false;
>        }
> -   } else if (var->type->is_sampler() ||
> -              (var->type->is_array() && var->type->fields.array
> ->is_sampler())) {
> +   } else if (base_type->is_sampler()) {
>        /* Samplers.  From page 63 of the GLSL 4.20 specification:
>         * "If the binding is less than zero, or greater than or equal to the
>         *  implementation-dependent maximum supported number of units, a
> @@ -2095,7 +2095,7 @@ validate_binding_qualifier(struct 
> _mesa_glsl_parse_state *state,
>  
>           return false;
>        }
> -   } else if (var->type->contains_atomic()) {
> +   } else if (base_type->contains_atomic()) {
>        assert(ctx->Const.MaxAtomicBufferBindings <= 
> MAX_COMBINED_ATOMIC_BUFFERS);
>        if (unsigned(qual->binding) >= ctx->Const.MaxAtomicBufferBindings) {
>           _mesa_glsl_error(loc, state, "layout(binding = %d) exceeds the "
> @@ -2651,7 +2651,7 @@ apply_type_qualifier_to_variable(const struct 
> ast_type_qualifier *qual,
>     }
>  
>     if (qual->flags.q.explicit_binding &&
> -       validate_binding_qualifier(state, loc, var, qual)) {
> +       validate_binding_qualifier(state, loc, var->type, qual)) {
>        var->data.explicit_binding = true;
>        var->data.binding = qual->binding;
>     }
> @@ -5752,6 +5752,8 @@ ast_interface_block::hir(exec_list *instructions,
>                                          num_variables,
>                                          packing,
>                                          this->block_name);
> +   if (this->layout.flags.q.explicit_binding)
> +      validate_binding_qualifier(state, &loc, block_type, &this->layout);
>  
>     if (!state->symbols->add_interface(block_type->name, block_type, 
> var_mode)) {
>        YYLTYPE loc = this->get_location();
> @@ -5849,6 +5851,10 @@ ast_interface_block::hir(exec_list *instructions,
>                               "not allowed");
>           }
>  
> +         if (this->layout.flags.q.explicit_binding)
> +            validate_binding_qualifier(state, &loc, block_array_type,
> +                                       &this->layout);
> +
>           var = new(state) ir_variable(block_array_type,
>                                        this->instance_name,
>                                        var_mode);