[Mesa-dev] [PATCH] mesa: check if any of the named builtins are available first

[Kenneth Graunke]

On Monday, July 3, 2017 2:08:12 PM PDT Ilia Mirkin wrote:
> _mesa_glsl_has_builtin_function is used to determine whether any variant
> of a builtin are available, for the purpose of enforcing the GLSL ES
> 3.00+ rule that overloads or overrides of builtins are disallowed.
> 
> However the builtin_builder contains information on all builtins,
> irrespective of parse state, or versions, or extension enablement. As a
> result we would say that a builtin existed even if it was not actually
> available.
> 
> To resolve this, first check if at least one signature is available for
> a builtin before returning true.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101666
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/compiler/glsl/ast_to_hir.cpp        |  2 +-
>  src/compiler/glsl/builtin_functions.cpp | 13 +++++++++++--
>  src/compiler/glsl/builtin_functions.h   |  3 ++-
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
> index b1e490eefd..c338ad79ca 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -5916,7 +5916,7 @@ ast_function::hir(exec_list *instructions,
>        /* Local shader has no exact candidates; check the built-ins. */
>        _mesa_glsl_initialize_builtin_functions();
>        if (state->language_version >= 300 &&
> -          _mesa_glsl_has_builtin_function(name)) {
> +          _mesa_glsl_has_builtin_function(state, name)) {
>           YYLTYPE loc = this->get_location();
>           _mesa_glsl_error(& loc, state,
>                            "A shader cannot redefine or overload built-in "
> diff --git a/src/compiler/glsl/builtin_functions.cpp b/src/compiler/glsl/builtin_functions.cpp
> index cc1432197b..84833bdd7d 100644
> --- a/src/compiler/glsl/builtin_functions.cpp
> +++ b/src/compiler/glsl/builtin_functions.cpp
> @@ -6227,14 +6227,23 @@ _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state *state,
>  }
>  
>  bool
> -_mesa_glsl_has_builtin_function(const char *name)
> +_mesa_glsl_has_builtin_function(_mesa_glsl_parse_state *state, const char *name)
>  {
>     ir_function *f;
> +   bool ret = false;
>     mtx_lock(&builtins_lock);
>     f = builtins.shader->symbols->get_function(name);
> +   if (f != NULL) {
> +      foreach_in_list(ir_function_signature, sig, &f->signatures) {
> +         if (sig->is_builtin_available(state)) {
> +            ret = true;
> +            break;
> +         }
> +      }
> +   }
>     mtx_unlock(&builtins_lock);
>  
> -   return f != NULL;
> +   return ret;
>  }
>  
>  gl_shader *
> diff --git a/src/compiler/glsl/builtin_functions.h b/src/compiler/glsl/builtin_functions.h
> index 14a52b9402..2053c82b74 100644
> --- a/src/compiler/glsl/builtin_functions.h
> +++ b/src/compiler/glsl/builtin_functions.h
> @@ -32,7 +32,8 @@ _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state *state,
>                                   const char *name, exec_list *actual_parameters);
>  
>  extern bool
> -_mesa_glsl_has_builtin_function(const char *name);
> +_mesa_glsl_has_builtin_function(_mesa_glsl_parse_state *state,
> +                                const char *name);
>  
>  extern gl_shader *
>  _mesa_glsl_get_builtin_function_shader(void);
> 

That's a good catch!  Thanks for fixing this!

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170705/8255efdc/attachment.sig>