From f2ad2730858046b08e8147297fa01bf3499b86a3 Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Thu, 18 Mar 2021 13:23:18 +0200 Subject: [PATCH] Add a bunch of validation for declarations in GLSL This is why I renamed the old DeclarationValidator a few commits ago, because that name is a better fit here. --- source/glsl/compiler.cpp | 2 +- source/glsl/validate.cpp | 256 +++++++++++++++++++++++++++++++++++---- source/glsl/validate.h | 31 ++++- 3 files changed, 262 insertions(+), 27 deletions(-) diff --git a/source/glsl/compiler.cpp b/source/glsl/compiler.cpp index 40dfede3..9c37787c 100644 --- a/source/glsl/compiler.cpp +++ b/source/glsl/compiler.cpp @@ -300,7 +300,7 @@ void Compiler::resolve(Stage &stage, unsigned flags) bool Compiler::validate(Stage &stage) { - TypeValidator().apply(stage); + DeclarationValidator().apply(stage); IdentifierValidator().apply(stage); ReferenceValidator().apply(stage); ExpressionValidator().apply(stage); diff --git a/source/glsl/validate.cpp b/source/glsl/validate.cpp index e025a61b..154189aa 100644 --- a/source/glsl/validate.cpp +++ b/source/glsl/validate.cpp @@ -38,29 +38,139 @@ void Validator::add_info(Node &node, const string &message) } -TypeValidator::TypeValidator(): - in_struct(false) +DeclarationValidator::DeclarationValidator(): + scope(GLOBAL), + iface_layout(0), + iface_block(0), + variable(0) { } -void TypeValidator::visit(BasicTypeDeclaration &type) +const char *DeclarationValidator::describe_variable(ScopeType scope) { - if(type.kind==BasicTypeDeclaration::VECTOR) + switch(scope) { - BasicTypeDeclaration::Kind base_kind = BasicTypeDeclaration::VOID; - if(BasicTypeDeclaration *basic_base = dynamic_cast(type.base_type)) - base_kind = basic_base->kind; - if(base_kind!=BasicTypeDeclaration::BOOL && base_kind!=BasicTypeDeclaration::INT && base_kind!=BasicTypeDeclaration::FLOAT) - error(type, format("Invalid base type '%s' for vector type '%s'", type.base, type.name)); + case GLOBAL: return "global variable"; + case STRUCT: return "struct member"; + case INTERFACE_BLOCK: return "interface block member"; + case FUNCTION_PARAM: return "function parameter"; + case FUNCTION: return "local variable"; + default: return "variable"; } - else if(type.kind==BasicTypeDeclaration::ARRAY) +} + +void DeclarationValidator::visit(Layout &layout) +{ + for(vector::const_iterator i=layout.qualifiers.begin(); i!=layout.qualifiers.end(); ++i) { - if(BasicTypeDeclaration *basic_base = dynamic_cast(type.base_type)) - if(basic_base->kind==BasicTypeDeclaration::VOID) - error(type, format("Invalid base type '%s' for array type '%s'", type.base, type.name)); + bool allowed = false; + string err_descr; + bool value = true; + if(i->name=="location") + allowed = (variable && scope==GLOBAL); + else if(i->name=="binding" || i->name=="set") + { + if(variable) + { + TypeDeclaration *type = variable->type_declaration; + while(BasicTypeDeclaration *basic = dynamic_cast(type)) + type = basic->base_type; + allowed = (scope==GLOBAL && dynamic_cast(type)); + err_descr = "variable of non-opaque type"; + } + else if(iface_block) + allowed = true; + } + else if(i->name=="constant_id") + { + allowed = (variable && scope==GLOBAL); + if(allowed) + { + if(!variable->constant) + { + allowed = false; + err_descr = "non-constant variable"; + } + else + { + BasicTypeDeclaration *basic = dynamic_cast(variable->type_declaration); + if(!basic || basic->kindkind>BasicTypeDeclaration::INT) + { + allowed = false; + err_descr = format("variable of type '%s'", + (variable->type_declaration ? variable->type_declaration->name : variable->type)); + } + } + } + } + else if(i->name=="offset") + allowed = (variable && scope==INTERFACE_BLOCK); + else if(i->name=="points") + { + allowed = (stage->type==Stage::GEOMETRY && iface_layout && (iface_layout->interface=="in" || iface_layout->interface=="out")); + value = false; + } + else if(i->name=="lines" || i->name=="lines_adjacency" || i->name=="triangles" || i->name=="triangles_adjacency") + { + allowed = (stage->type==Stage::GEOMETRY && iface_layout && iface_layout->interface=="in"); + value = false; + } + else if(i->name=="line_strip" || i->name=="triangle_strip") + { + allowed = (stage->type==Stage::GEOMETRY && iface_layout && iface_layout->interface=="out"); + value = false; + } + else if(i->name=="invocations") + allowed = (stage->type==Stage::GEOMETRY && iface_layout && iface_layout->interface=="in"); + else if(i->name=="max_vertices") + allowed = (stage->type==Stage::GEOMETRY && iface_layout && iface_layout->interface=="out"); + else if(i->name=="std140" || i->name=="std430") + { + allowed = (iface_block && !variable && iface_block->interface=="uniform"); + value = false; + } + + if(!allowed) + { + if(err_descr.empty()) + { + if(variable) + err_descr = describe_variable(scope); + else if(iface_block) + err_descr = "interface block"; + else if(iface_layout) + err_descr = format("interface '%s'", iface_layout->interface); + else + err_descr = "unknown declaration"; + } + error(layout, format("Layout qualifier '%s' not allowed on %s", i->name, err_descr)); + } + else if(value && !i->has_value) + error(layout, format("Layout qualifier '%s' requires a value", i->name)); + else if(!value && i->has_value) + error(layout, format("Layout qualifier '%s' does not allow a value", i->name)); } } -void TypeValidator::visit(ImageTypeDeclaration &type) +void DeclarationValidator::visit(InterfaceLayout &layout) +{ + SetForScope set_layout(iface_layout, &layout); + TraversingVisitor::visit(layout); +} + +void DeclarationValidator::visit(BasicTypeDeclaration &type) +{ + BasicTypeDeclaration *basic_base = dynamic_cast(type.base_type); + BasicTypeDeclaration::Kind base_kind = (basic_base ? basic_base->kind : BasicTypeDeclaration::VOID); + + if(type.kind==BasicTypeDeclaration::VECTOR && (base_kindBasicTypeDeclaration::FLOAT)) + error(type, format("Invalid base type '%s' for vector type '%s'", type.base, type.name)); + else if(type.kind==BasicTypeDeclaration::MATRIX && base_kind!=BasicTypeDeclaration::VECTOR) + error(type, format("Invalid base type '%s' for matrix type '%s'", type.base, type.name)); + else if(type.kind==BasicTypeDeclaration::ARRAY && basic_base && base_kind==BasicTypeDeclaration::VOID) + error(type, format("Invalid base type '%s' for array type '%s'", type.base, type.name)); +} + +void DeclarationValidator::visit(ImageTypeDeclaration &type) { BasicTypeDeclaration::Kind base_kind = BasicTypeDeclaration::VOID; if(BasicTypeDeclaration *basic_base = dynamic_cast(type.base_type)) @@ -69,23 +179,94 @@ void TypeValidator::visit(ImageTypeDeclaration &type) error(type, format("Invalid base type '%s' for image type '%s'", type.base, type.name)); } -void TypeValidator::visit(StructDeclaration &strct) +void DeclarationValidator::visit(StructDeclaration &strct) { - SetFlag set_struct(in_struct); + SetForScope set_scope(scope, (scope!=INTERFACE_BLOCK ? STRUCT : scope)); TraversingVisitor::visit(strct); } -void TypeValidator::visit(VariableDeclaration &var) +void DeclarationValidator::visit(VariableDeclaration &var) { - if(in_struct) + SetForScope set_var(variable, &var); + + const char *descr = describe_variable(scope); + + if(var.layout) { - if(var.layout) - error(var, "Struct members can't have layouts"); - if(var.init_expression) - error(var, "Struct members can't have initializers"); + if(scope!=GLOBAL && scope!=INTERFACE_BLOCK) + error(var, format("Layout qualifier not allowed on %s", descr)); + else + var.layout->visit(*this); } - TraversingVisitor::visit(var); + if(var.constant) + { + if(scope==STRUCT || scope==INTERFACE_BLOCK) + error(var, format("Constant qualifier not allowed on %s", descr)); + } + + if(!var.interpolation.empty() || !var.sampling.empty()) + { + if(var.interface!="in" && stage->type==Stage::VERTEX) + error(var, "Interpolation qualifier not allowed on vertex input"); + else if(var.interface!="out" && stage->type==Stage::FRAGMENT) + error(var, "Interpolation qualifier not allowed on fragment output"); + else if((var.interface!="in" && var.interface!="out") || (scope==FUNCTION_PARAM || scope==FUNCTION)) + error(var, "Interpolation qualifier not allowed on non-interpolated variable"); + } + + if(!var.interface.empty()) + { + if(iface_block && var.interface!=iface_block->interface) + error(var, format("Mismatched interface qualifier '%s' inside '%s' block", var.interface, iface_block->interface)); + else if(scope==STRUCT || scope==FUNCTION) + error(var, format("Interface qualifier not allowed on %s", descr)); + } + + TypeDeclaration *type = var.type_declaration; + while(BasicTypeDeclaration *basic = dynamic_cast(type)) + type = basic->base_type; + if(dynamic_cast(type)) + { + if(scope!=GLOBAL && scope!=FUNCTION_PARAM) + error(var, format("Type '%s' not allowed on %s", type->name, descr)); + else if(scope==GLOBAL && var.interface!="uniform") + error(var, format("Type '%s' only allowed with uniform interface", type->name)); + } + + if(var.init_expression) + { + if(scope==GLOBAL && !var.constant) + error(var, format("Initializer not allowed on non-constant %s", descr)); + else if(scope!=GLOBAL && scope!=FUNCTION) + error(var, format("Initializer not allowed on %s", descr)); + else + var.init_expression->visit(*this); + } +} + +void DeclarationValidator::visit(InterfaceBlock &iface) +{ + SetForScope set_scope(scope, INTERFACE_BLOCK); + SetForScope set_iface(iface_block, &iface); + + if(stage->type==Stage::VERTEX && iface.interface=="in") + error(iface, "Interface block not allowed on vertex shader input"); + else if(stage->type==Stage::FRAGMENT && iface.interface=="out") + error(iface, "Interface block not allowed on fragment shader output"); + + TraversingVisitor::visit(iface); + if(iface.struct_declaration) + iface.struct_declaration->visit(*this); +} + +void DeclarationValidator::visit(FunctionDeclaration &func) +{ + SetForScope set_scope(scope, FUNCTION_PARAM); + for(NodeArray::const_iterator i=func.parameters.begin(); i!=func.parameters.end(); ++i) + (*i)->visit(*this); + scope = FUNCTION; + func.body.visit(*this); } @@ -291,6 +472,10 @@ void ReferenceValidator::visit(FunctionDeclaration &func) } +ExpressionValidator::ExpressionValidator(): + current_function(0) +{ } + void ExpressionValidator::visit(Swizzle &swizzle) { unsigned size = 0; @@ -399,6 +584,31 @@ void ExpressionValidator::visit(VariableDeclaration &var) TraversingVisitor::visit(var); } +void ExpressionValidator::visit(FunctionDeclaration &func) +{ + SetForScope set_func(current_function, &func); + TraversingVisitor::visit(func); +} + +void ExpressionValidator::visit(Return &ret) +{ + if(current_function && current_function->return_type_declaration) + { + TypeDeclaration *return_type = current_function->return_type_declaration; + BasicTypeDeclaration *basic_return = dynamic_cast(return_type); + if(ret.expression) + { + if(ret.expression->type && ret.expression->type!=return_type) + error(ret, format("Return expression type '%s' is incompatible with declared return type '%s'", + ret.expression->type->name, return_type->name)); + } + else if(!basic_return || basic_return->kind!=BasicTypeDeclaration::VOID) + error(ret, "Return statement without an expression in a function not returning 'void'"); + } + + TraversingVisitor::visit(ret); +} + } // namespace SL } // namespace GL } // namespace Msp diff --git a/source/glsl/validate.h b/source/glsl/validate.h index 6cd5675a..f1e3f512 100644 --- a/source/glsl/validate.h +++ b/source/glsl/validate.h @@ -24,21 +24,39 @@ protected: void add_info(Node &, const std::string &); }; -class TypeValidator: private Validator +class DeclarationValidator: private Validator { private: - bool in_struct; + enum ScopeType + { + GLOBAL, + STRUCT, + INTERFACE_BLOCK, + FUNCTION_PARAM, + FUNCTION + }; + + ScopeType scope; + InterfaceLayout *iface_layout; + InterfaceBlock *iface_block; + VariableDeclaration *variable; public: - TypeValidator(); + DeclarationValidator(); void apply(Stage &s) { stage = &s; s.content.visit(*this); } private: + static const char *describe_variable(ScopeType); + + virtual void visit(Layout &); + virtual void visit(InterfaceLayout &); virtual void visit(BasicTypeDeclaration &); virtual void visit(ImageTypeDeclaration &); virtual void visit(StructDeclaration &); virtual void visit(VariableDeclaration &); + virtual void visit(InterfaceBlock &); + virtual void visit(FunctionDeclaration &); }; class IdentifierValidator: private Validator @@ -90,7 +108,12 @@ private: class ExpressionValidator: private Validator { +private: + FunctionDeclaration *current_function; + public: + ExpressionValidator(); + void apply(Stage &s) { stage = &s; s.content.visit(*this); } private: @@ -100,6 +123,8 @@ private: virtual void visit(Assignment &); virtual void visit(TernaryExpression &); virtual void visit(VariableDeclaration &); + virtual void visit(FunctionDeclaration &); + virtual void visit(Return &); }; } // namespace SL -- 2.43.0