[Mesa-dev] [PATCH] glsl: fix xfb_offset unsized array validation

[Timothy Arceri]

This partially fixes CTS test:
GL44-CTS.enhanced_layouts.xfb_get_program_resource_api

The test now fails at a tes evaluation shader with unsized output arrays.

The ARB_enhanced_layouts spec says:

   "It is a compile-time error to apply xfb_offset to the declaration of an
   unsized array."

So this seems like a bug in the CTS.
---
 src/compiler/glsl/ast_to_hir.cpp    | 23 +++++++++++++++--------
 src/compiler/glsl/ir.cpp            | 23 +++++++++++++++++++++++
 src/compiler/glsl/ir.h              |  3 +++
 src/compiler/glsl/link_varyings.cpp | 23 -----------------------
 4 files changed, 41 insertions(+), 31 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 7b464e0..b6db669 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3422,11 +3422,11 @@ apply_layout_qualifier_to_variable(const struct ast_type_qualifier *qual,
    if (qual->flags.q.explicit_xfb_offset) {
       unsigned qual_xfb_offset;
       unsigned component_size = var->type->contains_double() ? 8 : 4;
-
+      const glsl_type *t = get_varying_type(var, state->stage);
       if (process_qualifier_constant(state, loc, "xfb_offset",
                                      qual->offset, &qual_xfb_offset) &&
           validate_xfb_offset_qualifier(loc, state, (int) qual_xfb_offset,
-                                        var->type, component_size)) {
+                                        t, component_size)) {
          var->data.offset = qual_xfb_offset;
          var->data.explicit_xfb_offset = true;
       }
@@ -7309,12 +7309,6 @@ ast_interface_block::hir(exec_list *instructions,
                                         packing,
                                         this->block_name);
 
-   unsigned component_size = block_type->contains_double() ? 8 : 4;
-   int xfb_offset =
-      layout.flags.q.explicit_xfb_offset ? (int) qual_xfb_offset : -1;
-   validate_xfb_offset_qualifier(&loc, state, xfb_offset, block_type,
-                                 component_size);
-
    if (!state->symbols->add_interface(block_type->name, block_type, var_mode)) {
       YYLTYPE loc = this->get_location();
       _mesa_glsl_error(&loc, state, "interface block `%s' with type `%s' "
@@ -7453,6 +7447,13 @@ ast_interface_block::hir(exec_list *instructions,
                                       var_mode);
       }
 
+      unsigned component_size = block_type->contains_double() ? 8 : 4;
+      int xfb_offset =
+         layout.flags.q.explicit_xfb_offset ? (int) qual_xfb_offset : -1;
+      const glsl_type *t = get_varying_type(var, state->stage);
+      validate_xfb_offset_qualifier(&loc, state, xfb_offset, t,
+                                    component_size);
+
       var->data.matrix_layout = matrix_layout == GLSL_MATRIX_LAYOUT_INHERITED
          ? GLSL_MATRIX_LAYOUT_COLUMN_MAJOR : matrix_layout;
 
@@ -7528,6 +7529,12 @@ ast_interface_block::hir(exec_list *instructions,
        */
       assert(this->array_specifier == NULL);
 
+      unsigned component_size = block_type->contains_double() ? 8 : 4;
+      int xfb_offset =
+         layout.flags.q.explicit_xfb_offset ? (int) qual_xfb_offset : -1;
+      validate_xfb_offset_qualifier(&loc, state, xfb_offset, block_type,
+                                    component_size);
+
       for (unsigned i = 0; i < num_variables; i++) {
          ir_variable *var =
             new(state) ir_variable(fields[i].type,
diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
index 5bb3ac3..70859a7 100644
--- a/src/compiler/glsl/ir.cpp
+++ b/src/compiler/glsl/ir.cpp
@@ -2021,3 +2021,26 @@ mode_string(const ir_variable *var)
    assert(!"Should not get here.");
    return "invalid variable";
 }
+
+/**
+ * Get the varying type stripped of the outermost array if we're processing
+ * a stage whose varyings are arrays indexed by a vertex number (such as
+ * geometry shader inputs).
+ */
+const glsl_type *
+get_varying_type(const ir_variable *var, gl_shader_stage stage)
+{
+   const glsl_type *type = var->type;
+
+   if (!var->data.patch &&
+       ((var->data.mode == ir_var_shader_out &&
+         stage == MESA_SHADER_TESS_CTRL) ||
+        (var->data.mode == ir_var_shader_in &&
+         (stage == MESA_SHADER_TESS_CTRL || stage == MESA_SHADER_TESS_EVAL ||
+          stage == MESA_SHADER_GEOMETRY)))) {
+      assert(type->is_array());
+      type = type->fields.array;
+   }
+
+   return type;
+}
diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
index 6236245..eee7bf9 100644
--- a/src/compiler/glsl/ir.h
+++ b/src/compiler/glsl/ir.h
@@ -2625,6 +2625,9 @@ is_gl_identifier(const char *s)
    return s && s[0] == 'g' && s[1] == 'l' && s[2] == '_';
 }
 
+const glsl_type *
+get_varying_type(const ir_variable *var, gl_shader_stage stage);
+
 extern "C" {
 #endif /* __cplusplus */
 
diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index 4b2a672..49e9157 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -40,29 +40,6 @@
 #include "program.h"
 
 
-/**
- * Get the varying type stripped of the outermost array if we're processing
- * a stage whose varyings are arrays indexed by a vertex number (such as
- * geometry shader inputs).
- */
-static const glsl_type *
-get_varying_type(const ir_variable *var, gl_shader_stage stage)
-{
-   const glsl_type *type = var->type;
-
-   if (!var->data.patch &&
-       ((var->data.mode == ir_var_shader_out &&
-         stage == MESA_SHADER_TESS_CTRL) ||
-        (var->data.mode == ir_var_shader_in &&
-         (stage == MESA_SHADER_TESS_CTRL || stage == MESA_SHADER_TESS_EVAL ||
-          stage == MESA_SHADER_GEOMETRY)))) {
-      assert(type->is_array());
-      type = type->fields.array;
-   }
-
-   return type;
-}
-
 static void
 create_xfb_varying_names(void *mem_ctx, const glsl_type *t, char **name,
                          size_t name_length, unsigned *count,
-- 
2.5.5