[Mesa-dev] [PATCH] glsl: be more strict when validating shader inputs

[Jason Ekstrand]

On May 12, 2016 9:42 PM, "Ilia Mirkin" <imirkin at alum.mit.edu> wrote:
>
> On Fri, May 13, 2016 at 12:41 AM, Jason Ekstrand <jason at jlekstrand.net>
wrote:
> >
> > On May 12, 2016 9:29 PM, "Ilia Mirkin" <imirkin at alum.mit.edu> wrote:
> >>
> >> interpolateAt* can only take input variables or an element of an input
> >> variable array. No structs.
> >>
> >> Further, GLSL 4.50 relaxes the requirement to allow swizzles, so enable
> >> that as well.
> >>
> >> This fixes the following dEQP tests:
> >>
> >>
> >>
dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at_sample.negative.interpolate_struct_member
> >>
> >>
dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at_centroid.negative.interpolate_struct_member
> >>
> >>
dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at_offset.negative.interpolate_struct_member
> >>
> >> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> >> ---
> >>  src/compiler/glsl/ast_function.cpp | 26 ++++++++++++++++++--------
> >>  1 file changed, 18 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/src/compiler/glsl/ast_function.cpp
> >> b/src/compiler/glsl/ast_function.cpp
> >> index 4db3dd0..281735b 100644
> >> --- a/src/compiler/glsl/ast_function.cpp
> >> +++ b/src/compiler/glsl/ast_function.cpp
> >> @@ -208,17 +208,27 @@ verify_parameter_modes(_mesa_glsl_parse_state
> >> *state,
> >>
> >>        /* Verify that shader_in parameters are shader inputs */
> >>        if (formal->data.must_be_shader_input) {
> >> -         ir_variable *var = actual->variable_referenced();
> >> -         if (!var || var->data.mode != ir_var_shader_in) {
> >> -            _mesa_glsl_error(&loc, state,
> >> -                             "parameter `%s` must be a shader input",
> >> -                             formal->name);
> >> -            return false;
> >> +         const ir_rvalue *val = actual;
> >> +
> >> +         // GLSL 4.50 allows swizzles, while earlier GLSL versions do
> >> not.
> >> +         if (val->ir_type == ir_type_swizzle) {
> >> +            if (!state->is_version(450, 0)) {
> >
> > What about GLSL 4.60?  Not that it exists yet but this doesn't look
like a
> > greater-equal.
>
> state->is_version == at least version X (in this case, at least
> desktop glsl version 450). So 460 should follow the same rules. (The
> second argument is for ES... 0 means "never").

I see.  (You can tell how much of this sort of code I've written...).

I don't know that I'd trust me but it looks fine as far add I can see.
Thanks for taking care of 4.50 while you were in the neighborhood.  For
what it's worth,

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

> >
> >> +               _mesa_glsl_error(&loc, state,
> >> +                                "parameter `%s` must not be swizzled",
> >> +                                formal->name);
> >> +               return false;
> >> +            }
> >> +            val = ((ir_swizzle *)val)->val;
> >> +         }
> >> +
> >> +         while (val->ir_type == ir_type_dereference_array) {
> >> +            val = ((ir_dereference_array *)val)->array;
> >
> > What about arrays of arrays?
>
> That's why it's a while loop.

Yeah... Saw that as soon as I sent the email.

> >
> >>           }
> >>
> >> -         if (actual->ir_type == ir_type_swizzle) {
> >> +         if (!val->as_dereference_variable() ||
> >> +             val->variable_referenced()->data.mode !=
ir_var_shader_in) {
> >>              _mesa_glsl_error(&loc, state,
> >> -                             "parameter `%s` must not be swizzled",
> >> +                             "parameter `%s` must be a shader input",
> >>                               formal->name);
> >>              return false;
> >>           }
> >> --
> >> 2.7.3
> >>
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160512/cadcf541/attachment.html>