[Mesa-dev] [PATCH 7/8] glsl: In ir_set_program_inouts, handle indexing outside array/matrix bounds.

[Ian Romanick]

On 07/31/2013 02:17 PM, Paul Berry wrote:
> According to GLSL, indexing into an array or matrix with an
> out-of-range constant results in a compile error.  However, indexing
> with an out-of-range value that isn't constant merely results in
> undefined results.
>
> Since optimization passes (e.g. loop unrolling) can convert
> non-constant array indices into constant array indices, it's possible
> that ir_set_program_inouts will encounter a constant array index that
> is out of range; if this happens, just mark the whole array as used.
> ---
>   src/glsl/ir_set_program_inouts.cpp | 31 ++++++++++++++++++++++++++-----
>   1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/src/glsl/ir_set_program_inouts.cpp b/src/glsl/ir_set_program_inouts.cpp
> index b4588a7..018342b 100644
> --- a/src/glsl/ir_set_program_inouts.cpp
> +++ b/src/glsl/ir_set_program_inouts.cpp
> @@ -171,13 +171,34 @@ ir_set_program_inouts_visitor::try_mark_partial_variable(ir_variable *var,
>      if (!index_as_constant)
>         return false;
>
> -   int width = 1;
> -   if (type->is_array() && type->fields.array->is_matrix()) {
> -      width = type->fields.array->matrix_columns;
> +   unsigned elem_width;
> +   unsigned num_elems;
> +   if (type->is_array()) {
> +      num_elems = type->length;
> +      if (type->fields.array->is_matrix())
> +         elem_width = type->fields.array->matrix_columns;
> +      else
> +         elem_width = 1;
> +   } else {
> +      num_elems = type->matrix_columns;
> +      elem_width = 1;
>      }
>
> -   mark(this->prog, var, index_as_constant->value.u[0] * width, width,
> -        this->shader_type == GL_FRAGMENT_SHADER);
> +   if (index_as_constant->value.u[0] >= num_elems) {
> +      /* Constant index outside the bounds of the matrix/array.  This could
> +       * arise as a result of constant folding of a legal GLSL program.
> +       *
> +       * Even though the spec says that indexing outside the bounds of a
> +       * matrix/array results in undefined behaviour, we don't want to pass
> +       * out-of-range values to mark() (since this could result in slots that
> +       * don't exist being marked as used), so just let the caller mark the
> +       * whole variable as used.
> +       */

At least for the loop-unroll case, I seem to recall other compilers 
generating a link-time error.  I may be misremembering.  I'd like to 
have a follow-on patch (eventually) that either emits a warning or an 
error here.  I think the trick will be making the message useful. :(

> +      return false;
> +   }
> +
> +   mark(this->prog, var, index_as_constant->value.u[0] * elem_width,
> +        elem_width, this->shader_type == GL_FRAGMENT_SHADER);
>      return true;
>   }
>
>