[Mesa-dev] [PATCH] glsl: Validate aux storage qualifier combination with other qualifiers.

[Chris Forbes]

We've been allowing `centroid` and `sample` in all kinds of weird places
where they're not valid.

Insist that `sample` is combined with `in` or `out`;
and that `centroid` is combined with `in`, `out`, or the deprecated
`varying`.

V2: Validate this in a more sensible place. This does require an extra
case for uniform blocks members and struct members, though, since they
don't go through the normal path.

V3: Improve error message wording; eliminate redundant error generation
for inputs in VS or outputs in FS.

Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
---
 src/glsl/ast_to_hir.cpp | 76 ++++++++++++++++++++++++-------------------------
 1 file changed, 37 insertions(+), 39 deletions(-)

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index d1c77f1..00a59a5 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2649,6 +2649,36 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
    const bool uses_deprecated_qualifier = qual->flags.q.attribute
       || qual->flags.q.varying;
 
+
+   /* Validate auxiliary storage qualifiers */
+
+   /* From section 4.3.4 of the GLSL 1.30 spec:
+    *    "It is an error to use centroid in in a vertex shader."
+    *
+    * From section 4.3.4 of the GLSL ES 3.00 spec:
+    *    "It is an error to use centroid in or interpolation qualifiers in
+    *    a vertex shader input."
+    */
+
+   /* Section 4.3.6 of the GLSL 1.30 specification states:
+    * "It is an error to use centroid out in a fragment shader."
+    *
+    * The GL_ARB_shading_language_420pack extension specification states:
+    * "It is an error to use auxiliary storage qualifiers or interpolation
+    *  qualifiers on an output in a fragment shader."
+    */
+   if (qual->flags.q.sample && (!is_varying_var(var, state->stage) || uses_deprecated_qualifier)) {
+      _mesa_glsl_error(loc, state,
+                       "sample qualifier may only be used on `in` or `out` "
+                       "variables between shader stages");
+   }
+   if (qual->flags.q.centroid && !is_varying_var(var, state->stage)) {
+      _mesa_glsl_error(loc, state,
+                       "centroid qualifier may only be used with `in', "
+                       "`out' or `varying' variables between shader stages");
+   }
+
+
    /* Is the 'layout' keyword used with parameters that allow relaxed checking.
     * Many implementations of GL_ARB_fragment_coord_conventions_enable and some
     * implementations (only Mesa?) GL_ARB_explicit_attrib_location_enable
@@ -3606,45 +3636,6 @@ ast_declarator_list::hir(exec_list *instructions,
       }
 
 
-      /* From section 4.3.4 of the GLSL 1.30 spec:
-       *    "It is an error to use centroid in in a vertex shader."
-       *
-       * From section 4.3.4 of the GLSL ES 3.00 spec:
-       *    "It is an error to use centroid in or interpolation qualifiers in
-       *    a vertex shader input."
-       */
-      if (state->is_version(130, 300)
-          && this->type->qualifier.flags.q.centroid
-          && this->type->qualifier.flags.q.in
-          && state->stage == MESA_SHADER_VERTEX) {
-
-         _mesa_glsl_error(&loc, state,
-                          "'centroid in' cannot be used in a vertex shader");
-      }
-
-      if (state->stage == MESA_SHADER_VERTEX
-          && this->type->qualifier.flags.q.sample
-          && this->type->qualifier.flags.q.in) {
-
-         _mesa_glsl_error(&loc, state,
-                        "'sample in' cannot be used in a vertex shader");
-      }
-
-      /* Section 4.3.6 of the GLSL 1.30 specification states:
-       * "It is an error to use centroid out in a fragment shader."
-       *
-       * The GL_ARB_shading_language_420pack extension specification states:
-       * "It is an error to use auxiliary storage qualifiers or interpolation
-       *  qualifiers on an output in a fragment shader."
-       */
-      if (state->stage == MESA_SHADER_FRAGMENT &&
-          this->type->qualifier.flags.q.out &&
-          this->type->qualifier.has_auxiliary_storage()) {
-         _mesa_glsl_error(&loc, state,
-                          "auxiliary storage qualifiers cannot be used on "
-                          "fragment shader outputs");
-      }
-
       /* Precision qualifiers exists only in GLSL versions 1.00 and >= 1.30.
        */
       if (this->type->qualifier.precision != ast_precision_none) {
@@ -5041,6 +5032,13 @@ ast_process_structure_or_interface_block(exec_list *instructions,
                              "with uniform interface blocks");
          }
 
+         if ((qual->flags.q.uniform || !is_interface) &&
+             qual->has_auxiliary_storage()) {
+            _mesa_glsl_error(&loc, state,
+                             "auxiliary storage qualifiers cannot be used "
+                             "in uniform blocks or structures.");
+         }
+
          if (field_type->is_matrix() ||
              (field_type->is_array() && field_type->fields.array->is_matrix())) {
             fields[i].row_major = block_row_major;
-- 
2.0.0