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

[Ilia Mirkin]

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").

>
>> +               _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.

>
>>           }
>>
>> -         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