[Mesa-dev] [PATCH] glsl: prevent qualifiers modification of predeclared variables

[andrey simiklit]

Hello,

+ idr at freedesktop.org
and add missing bugzilla link
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108247

Regards,
Andrii.

On Thu, Oct 4, 2018 at 5:08 PM <asimiklit.work at gmail.com> wrote:

> From: Andrii Simiklit <andrii.simiklit at globallogic.com>
>
> GLSL 3.7 (Identifiers):
> "However, as noted in the specification, there are some cases where
> previously declared variables can be redeclared to change or add some
> property, and predeclared "gl_" names are allowed to be redeclared in a
> shader only for these specific purposes.  More generally, it is an error to
> redeclare a variable, including those starting "gl_"."
>
> This patch should fix piglit tests:
> 'clip-distance-redeclare-without-inout.frag'
> 'clip-distance-redeclare-without-inout.vert'
> and leads to regression in clip-distance-out-values.shader_test
> but probably a fix should be made in the test.
>
> As far as I understood following mailing thread:
> https://lists.freedesktop.org/archives/piglit/2013-October/007935.html
> looks like we have accepted to remove an ability to change qualifiers
> but have not done it yet. Unless I missed something)
>
> Fixes: 8e6cb9fe51a2 "glsl: Refactor AST-to-HIR code handling variable
>                                          redeclarations"
> Signed-off-by: Andrii Simiklit <andrii.simiklit at globallogic.com>
> ---
>  src/compiler/glsl/ast_to_hir.cpp | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/src/compiler/glsl/ast_to_hir.cpp
> b/src/compiler/glsl/ast_to_hir.cpp
> index 93e7c8ec33..e26ae6b92a 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -4240,10 +4240,15 @@ get_variable_being_redeclared(ir_variable
> **var_ptr, YYLTYPE loc,
>      */
>     if (earlier->type->is_unsized_array() && var->type->is_array()
>         && (var->type->fields.array == earlier->type->fields.array)) {
> -      /* FINISHME: This doesn't match the qualifiers on the two
> -       * FINISHME: declarations.  It's not 100% clear whether this is
> -       * FINISHME: required or not.
> -       */
> +
> +      if ((strcmp("gl_ClipDistance", var->name) == 0) &&
> +          earlier->data.mode != var->data.mode) {
> +         _mesa_glsl_error(&loc, state,
> +                          "redeclaration of '%s %s' with incorrect
> qualifiers '%s'.",
> +                          mode_string(earlier),
> +                          var->name,
> +                          mode_string(var));
> +      }
>
>        const int size = var->type->array_size();
>        check_builtin_array_max_size(var->name, size, loc, state);
> --
> 2.17.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181005/9156ee6b/attachment.html>