[Mesa-dev] [PATCH] glsl/linker: check same name is not used in block and outside

[Tapani Pälli]

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

On 30.01.2018 17:11, Juan A. Suarez Romero wrote:
> According with OpenGL GLSL 3.20 spec, section 4.3.9:
> 
>    "It is a link-time error if any particular shader interface
>     contains:
>       - two different blocks, each having no instance name, and each
>         having a member of the same name, or
>       - a variable outside a block, and a block with no instance name,
>         where the variable has the same name as a member in the block."
> 
> This fixes a previous commit 9b894c8 ("glsl/linker: link-error using the
> same name in unnamed block and outside") that covered this case, but
> did not take in account that precision qualifiers are ignored when
> comparing blocks with no instance name.
> 
> With this commit, the original tests
> KHR-GL*.shaders.uniform_block.common.name_matching keep fixed, and also
> dEQP-GLES31.functional.shaders.linkage.uniform.block.differing_precision
> regression is fixed, which was broken by previous commit.
> 
> Fixes: 9b894c8 ("glsl/linker: link-error using the same name in unnamed block and outside")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104668
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104777
> CC: Mark Janes <mark.a.janes at intel.com>
> CC: "18.0" <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> ---
>   src/compiler/glsl/linker.cpp | 54 +++++++++++++++++++++++++-------------------
>   1 file changed, 31 insertions(+), 23 deletions(-)
> 
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index ce101935b01..65b22fdba8a 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -1111,29 +1111,6 @@ cross_validate_globals(struct gl_shader_program *prog,
>               return;
>            }
>   
> -         /* In OpenGL GLSL 4.20 spec, section 4.3.9, page 57:
> -          *
> -          *   "It is a link-time error if any particular shader interface
> -          *    contains:
> -          *
> -          *    - two different blocks, each having no instance name, and each
> -          *      having a member of the same name, or
> -          *
> -          *    - a variable outside a block, and a block with no instance name,
> -          *      where the variable has the same name as a member in the block."
> -          */
> -         if (var->data.mode == existing->data.mode &&
> -             var->get_interface_type() != existing->get_interface_type()) {
> -            linker_error(prog, "declarations for %s `%s` are in "
> -                         "%s and %s\n",
> -                         mode_string(var), var->name,
> -                         existing->get_interface_type() ?
> -                           existing->get_interface_type()->name : "outside a block",
> -                         var->get_interface_type() ?
> -                           var->get_interface_type()->name : "outside a block");
> -
> -            return;
> -         }
>            /* Only in GLSL ES 3.10, the precision qualifier should not match
>             * between block members defined in matched block names within a
>             * shader interface.
> @@ -1155,6 +1132,37 @@ cross_validate_globals(struct gl_shader_program *prog,
>                                 mode_string(var), var->name);
>               }
>            }
> +
> +         /* In OpenGL GLSL 3.20 spec, section 4.3.9:
> +          *
> +          *   "It is a link-time error if any particular shader interface
> +          *    contains:
> +          *
> +          *    - two different blocks, each having no instance name, and each
> +          *      having a member of the same name, or
> +          *
> +          *    - a variable outside a block, and a block with no instance name,
> +          *      where the variable has the same name as a member in the block."
> +          */
> +         if (var->get_interface_type() != existing->get_interface_type()) {
> +            if (!var->get_interface_type() || !existing->get_interface_type()) {
> +               linker_error(prog, "declarations for %s `%s` are inside block "
> +                            "`%s` and outside a block",
> +                            mode_string(var), var->name,
> +                            var->get_interface_type()
> +                               ? var->get_interface_type()->name
> +                               : existing->get_interface_type()->name);
> +               return;
> +            } else if (strcmp(var->get_interface_type()->name,
> +                              existing->get_interface_type()->name) != 0) {
> +               linker_error(prog, "declarations for %s `%s` are inside blocks "
> +                            "`%s` and `%s`",
> +                            mode_string(var), var->name,
> +                            existing->get_interface_type()->name,
> +                            var->get_interface_type()->name);
> +               return;
> +            }
> +         }
>         } else
>            variables->add_variable(var);
>      }
>