From 0f18945cb612493d787377d8cbb138c18738f683 Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Tue, 28 Apr 2015 14:25:56 +0200 Subject: glsl: Do not allow reads from write-only buffer variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The error location won't be right, but fixing that would require to check for this as we process each type of AST node that can involve a variable read. v2: - Limit the check to buffer variables, image variables have different semantics involved. Reviewed-by: Tapani Pälli Reviewed-by: Kristian Høgsberg --- src/glsl/ast_to_hir.cpp | 56 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) (limited to 'src/glsl') diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index cad4c0300d7..6b2e140cf43 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -67,6 +67,48 @@ static void remove_per_vertex_blocks(exec_list *instructions, _mesa_glsl_parse_state *state, ir_variable_mode mode); +/** + * Visitor class that finds the first instance of any write-only variable that + * is ever read, if any + */ +class read_from_write_only_variable_visitor : public ir_hierarchical_visitor +{ +public: + read_from_write_only_variable_visitor() : found(NULL) + { + } + + virtual ir_visitor_status visit(ir_dereference_variable *ir) + { + if (this->in_assignee) + return visit_continue; + + ir_variable *var = ir->variable_referenced(); + /* We can have image_write_only set on both images and buffer variables, + * but in the former there is a distinction between reads from + * the variable itself (write_only) and from the memory they point to + * (image_write_only), while in the case of buffer variables there is + * no such distinction, that is why this check here is limited to + * buffer variables alone. + */ + if (!var || var->data.mode != ir_var_shader_storage) + return visit_continue; + + if (var->data.image_write_only) { + found = var; + return visit_stop; + } + + return visit_continue; + } + + ir_variable *get_variable() { + return found; + } + +private: + ir_variable *found; +}; void _mesa_ast_to_hir(exec_list *instructions, struct _mesa_glsl_parse_state *state) @@ -162,6 +204,20 @@ _mesa_ast_to_hir(exec_list *instructions, struct _mesa_glsl_parse_state *state) */ remove_per_vertex_blocks(instructions, state, ir_var_shader_in); remove_per_vertex_blocks(instructions, state, ir_var_shader_out); + + /* Check that we don't have reads from write-only variables */ + read_from_write_only_variable_visitor v; + v.run(instructions); + ir_variable *error_var = v.get_variable(); + if (error_var) { + /* It would be nice to have proper location information, but for that + * we would need to check this as we process each kind of AST node + */ + YYLTYPE loc; + memset(&loc, 0, sizeof(loc)); + _mesa_glsl_error(&loc, state, "Read from write-only variable `%s'", + error_var->name); + } } -- cgit v1.2.3