From 57c1139e4fe21aeca7118b18eb3ba6fa43d7bf90 Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Thu, 1 Dec 2016 13:29:29 +0200 Subject: [PATCH] Upgrade NodePtr to RefPtr Allows Nodes to be moved from one Block to another without copying them. When targeting GLSL 1.30 LegacyConverter has a sequence where it flattens an interface block and later checks the type of those variables through their declaration pointer. This caused memory errors because the original nodes were destroyed in the flatten operation. --- source/programcompiler.cpp | 44 ++++++++++----------- source/programcompiler.h | 6 +-- source/programparser.cpp | 80 +++++++++++++++++++------------------- source/programparser.h | 30 +++++++------- source/programsyntax.cpp | 6 +-- source/programsyntax.h | 64 +++++++++--------------------- 6 files changed, 101 insertions(+), 129 deletions(-) diff --git a/source/programcompiler.cpp b/source/programcompiler.cpp index a68c8b96..86ea8da9 100644 --- a/source/programcompiler.cpp +++ b/source/programcompiler.cpp @@ -184,8 +184,8 @@ bool ProgramCompiler::optimize(Stage &stage) void ProgramCompiler::inject_block(Block &target, const Block &source) { - list >::iterator insert_point = target.body.begin(); - for(list >::const_iterator i=source.body.begin(); i!=source.body.end(); ++i) + list >::iterator insert_point = target.body.begin(); + for(list >::const_iterator i=source.body.begin(); i!=source.body.end(); ++i) target.body.insert(insert_point, (*i)->clone()); } @@ -281,7 +281,7 @@ void ProgramCompiler::Formatter::visit(Assignment &assign) void ProgramCompiler::Formatter::visit(FunctionCall &call) { formatted += format("%s(", call.name); - for(vector >::iterator i=call.arguments.begin(); i!=call.arguments.end(); ++i) + for(vector >::iterator i=call.arguments.begin(); i!=call.arguments.end(); ++i) { if(i!=call.arguments.begin()) formatted += ", "; @@ -308,7 +308,7 @@ void ProgramCompiler::Formatter::visit(Block &block) SetForScope set(indent, indent+(indent>0 || use_braces)); string spaces(indent*2, ' '); - for(list >::iterator i=block.body.begin(); i!=block.body.end(); ++i) + for(list >::iterator i=block.body.begin(); i!=block.body.end(); ++i) { if(i!=block.body.begin()) formatted += '\n'; @@ -394,7 +394,7 @@ void ProgramCompiler::Formatter::visit(InterfaceBlock &iface) void ProgramCompiler::Formatter::visit(FunctionDeclaration &func) { formatted += format("%s %s(", func.return_type, func.name); - for(vector >::iterator i=func.parameters.begin(); i!=func.parameters.end(); ++i) + for(vector >::iterator i=func.parameters.begin(); i!=func.parameters.end(); ++i) { if(i!=func.parameters.begin()) formatted += ", "; @@ -462,7 +462,7 @@ void ProgramCompiler::DeclarationCombiner::visit(Block &block) return; SetForScope set(toplevel, false); - for(list >::iterator i=block.body.begin(); i!=block.body.end(); ) + for(list >::iterator i=block.body.begin(); i!=block.body.end(); ) { remove_node = false; (*i)->visit(*this); @@ -676,15 +676,13 @@ ProgramCompiler::BlockModifier::BlockModifier(): void ProgramCompiler::BlockModifier::flatten_block(Block &block) { - for(list >::iterator i=block.body.begin(); i!=block.body.end(); ++i) - insert_nodes.push_back((*i)->clone()); + insert_nodes.insert(insert_nodes.end(), block.body.begin(), block.body.end()); remove_node = true; } -void ProgramCompiler::BlockModifier::apply_and_increment(Block &block, list >::iterator &i) +void ProgramCompiler::BlockModifier::apply_and_increment(Block &block, list >::iterator &i) { - for(list::iterator j=insert_nodes.begin(); j!=insert_nodes.end(); ++j) - block.body.insert(i, *j); + block.body.insert(i, insert_nodes.begin(), insert_nodes.end()); insert_nodes.clear(); if(remove_node) @@ -696,7 +694,7 @@ void ProgramCompiler::BlockModifier::apply_and_increment(Block &block, list >::iterator i=block.body.begin(); i!=block.body.end(); ) + for(list >::iterator i=block.body.begin(); i!=block.body.end(); ) { (*i)->visit(*this); apply_and_increment(block, i); @@ -730,15 +728,15 @@ void ProgramCompiler::InterfaceGenerator::apply(Stage &s) void ProgramCompiler::InterfaceGenerator::visit(Block &block) { SetForScope set(scope_level, scope_level+1); - for(list >::iterator i=block.body.begin(); i!=block.body.end(); ) + for(list >::iterator i=block.body.begin(); i!=block.body.end(); ) { (*i)->visit(*this); if(scope_level==1) { - for(map::iterator j=iface_declarations.begin(); j!=iface_declarations.end(); ++j) + for(map >::iterator j=iface_declarations.begin(); j!=iface_declarations.end(); ++j) { - list >::iterator k = block.body.insert(i, j->second); + list >::iterator k = block.body.insert(i, j->second); (*k)->visit(*this); } iface_declarations.clear(); @@ -847,9 +845,9 @@ void ProgramCompiler::InterfaceGenerator::visit(Passthrough &pass) for(map::const_iterator i=stage->in_variables.begin(); i!=stage->in_variables.end(); ++i) pass_vars.push_back(i->second); - for(map::const_iterator i=iface_declarations.begin(); i!=iface_declarations.end(); ++i) + for(map >::const_iterator i=iface_declarations.begin(); i!=iface_declarations.end(); ++i) if(i->second->interface=="in") - pass_vars.push_back(i->second); + pass_vars.push_back(i->second.get()); if(stage->previous) { @@ -1032,7 +1030,7 @@ void ProgramCompiler::ConstantConditionEliminator::visit(Assignment &assign) void ProgramCompiler::ConstantConditionEliminator::visit(VariableDeclaration &var) { if(var.constant || scope_level>1) - variable_values[&var] = &*var.init_expression; + variable_values[&var] = var.init_expression.get(); } void ProgramCompiler::ConstantConditionEliminator::visit(Conditional &cond) @@ -1180,7 +1178,7 @@ void ProgramCompiler::UnusedVariableLocator::visit(FunctionDeclaration &func) { assignments.push_back(BlockAssignmentMap()); - for(vector >::iterator i=func.parameters.begin(); i!=func.parameters.end(); ++i) + for(vector >::iterator i=func.parameters.begin(); i!=func.parameters.end(); ++i) (*i)->visit(*this); func.body.visit(*this); @@ -1286,10 +1284,10 @@ ProgramCompiler::NodeRemover::NodeRemover(const set &r): void ProgramCompiler::NodeRemover::visit(Block &block) { - for(list >::iterator i=block.body.begin(); i!=block.body.end(); ) + for(list >::iterator i=block.body.begin(); i!=block.body.end(); ) { (*i)->visit(*this); - if(to_remove.count(&**i)) + if(to_remove.count(i->get())) block.body.erase(i++); else ++i; @@ -1306,7 +1304,7 @@ void ProgramCompiler::NodeRemover::visit(VariableDeclaration &var) if(var.linked_declaration) var.linked_declaration->linked_declaration = 0; } - else if(var.init_expression && to_remove.count(&*var.init_expression)) + else if(var.init_expression && to_remove.count(var.init_expression.get())) var.init_expression = 0; } @@ -1347,7 +1345,7 @@ void ProgramCompiler::LegacyConverter::visit(FunctionCall &call) { if(call.name=="texture" && !call.declaration && !check_version(Version(1, 30))) { - vector >::iterator i = call.arguments.begin(); + vector >::iterator i = call.arguments.begin(); if(i!=call.arguments.end()) { (*i)->visit(*this); diff --git a/source/programcompiler.h b/source/programcompiler.h index 8d7d1ad1..398bb214 100644 --- a/source/programcompiler.h +++ b/source/programcompiler.h @@ -121,12 +121,12 @@ private: struct BlockModifier: Visitor { bool remove_node; - std::list insert_nodes; + std::list > insert_nodes; BlockModifier(); void flatten_block(ProgramSyntax::Block &); - void apply_and_increment(ProgramSyntax::Block &, std::list >::iterator &); + void apply_and_increment(ProgramSyntax::Block &, std::list >::iterator &); virtual void visit(ProgramSyntax::Block &); }; @@ -135,7 +135,7 @@ private: std::string in_prefix; std::string out_prefix; unsigned scope_level; - std::map iface_declarations; + std::map > iface_declarations; InterfaceGenerator(); diff --git a/source/programparser.cpp b/source/programparser.cpp index 596ed045..da3ee0b9 100644 --- a/source/programparser.cpp +++ b/source/programparser.cpp @@ -95,7 +95,7 @@ void ProgramParser::parse_source() iter = source.begin(); while(1) { - while(Node *statement = parse_global_declaration()) + while(RefPtr statement = parse_global_declaration()) cur_stage->content.body.push_back(statement); parse_token(); @@ -329,7 +329,7 @@ bool ProgramParser::is_identifier(const string &token) return re.match(token); } -Node *ProgramParser::parse_global_declaration() +RefPtr ProgramParser::parse_global_declaration() { string token = peek_token(); if(token=="import") @@ -344,12 +344,12 @@ Node *ProgramParser::parse_global_declaration() iface_lo->layout.qualifiers = layout->qualifiers; iface_lo->interface = parse_token(); expect(";"); - return iface_lo.release(); + return iface_lo; } else { - VariableDeclaration *var = parse_variable_declaration(); - var->layout = layout.release(); + RefPtr var = parse_variable_declaration(); + var->layout = layout; return var; } } @@ -377,7 +377,7 @@ Node *ProgramParser::parse_global_declaration() throw runtime_error(format("Syntax error at '%s': expected a global declaration", token)); } -Node *ProgramParser::parse_statement() +RefPtr ProgramParser::parse_statement() { string token = peek_token(); if(token=="if") @@ -396,13 +396,13 @@ Node *ProgramParser::parse_statement() expr->expression = parse_expression(); expect(";"); - return expr.release(); + return expr; } else throw runtime_error(format("Syntax error at '%s': expected a statement", token)); } -Import *ProgramParser::parse_import() +RefPtr ProgramParser::parse_import() { if(cur_stage->type!=SHARED) throw runtime_error("Imports are only allowed in the shared section"); @@ -411,10 +411,10 @@ Import *ProgramParser::parse_import() RefPtr import = new Import; import->module = parse_token(); expect(";"); - return import.release(); + return import; } -Layout *ProgramParser::parse_layout() +RefPtr ProgramParser::parse_layout() { expect("layout"); expect("("); @@ -439,7 +439,7 @@ Layout *ProgramParser::parse_layout() } expect(")"); - return layout.release(); + return layout; } void ProgramParser::parse_block(Block &block, bool require_braces) @@ -462,7 +462,7 @@ void ProgramParser::parse_block(Block &block, bool require_braces) expect("}"); } -Expression *ProgramParser::parse_expression(unsigned precedence) +RefPtr ProgramParser::parse_expression(unsigned precedence) { RefPtr left; VariableReference *left_var = 0; @@ -478,7 +478,7 @@ Expression *ProgramParser::parse_expression(unsigned precedence) if(token==";" || token==")" || token=="]" || token=="," || (oper && precedence && oper->precedence>=precedence)) { if(left) - return left.release(); + return left; else throw runtime_error(format("Parse error at '%s': expected an expression", token)); } @@ -488,12 +488,12 @@ Expression *ProgramParser::parse_expression(unsigned precedence) { if(!left_var) throw runtime_error(format("Parse error at '%s': function name must be an identifier", token)); - left = parse_function_call(left_var); + left = parse_function_call(*left_var); } else if(token==".") { RefPtr memacc = new MemberAccess; - memacc->left = left.release(); + memacc->left = left; parse_token(); memacc->member = expect_identifier(); left = memacc; @@ -503,11 +503,11 @@ Expression *ProgramParser::parse_expression(unsigned precedence) RefPtr unary = new UnaryExpression; unary->oper = parse_token(); unary->prefix = false; - unary->expression = left.release(); + unary->expression = left; left = unary; } else if(oper && oper->type==BINARY) - left = parse_binary(left.release(), oper); + left = parse_binary(left, oper); else throw runtime_error(format("Parse error at '%s': expected an operator", token)); left_var = 0; @@ -549,7 +549,7 @@ Expression *ProgramParser::parse_expression(unsigned precedence) } } -BinaryExpression *ProgramParser::parse_binary(Expression *left, const Operator *oper) +RefPtr ProgramParser::parse_binary(const RefPtr &left, const Operator *oper) { RefPtr binary = (oper->precedence==16 ? new Assignment : new BinaryExpression); binary->left = left; @@ -562,13 +562,13 @@ BinaryExpression *ProgramParser::parse_binary(Expression *left, const Operator * } else binary->right = parse_expression(oper->precedence+(oper->assoc==RIGHT_TO_LEFT)); - return binary.release(); + return binary; } -FunctionCall *ProgramParser::parse_function_call(VariableReference *var) +RefPtr ProgramParser::parse_function_call(const VariableReference &var) { RefPtr call = new FunctionCall; - call->name = var->name; + call->name = var.name; call->constructor = is_type(call->name); expect("("); while(peek_token()!=")") @@ -578,10 +578,10 @@ FunctionCall *ProgramParser::parse_function_call(VariableReference *var) call->arguments.push_back(parse_expression()); } expect(")"); - return call.release(); + return call; } -StructDeclaration *ProgramParser::parse_struct_declaration() +RefPtr ProgramParser::parse_struct_declaration() { expect("struct"); RefPtr strct = new StructDeclaration; @@ -591,10 +591,10 @@ StructDeclaration *ProgramParser::parse_struct_declaration() expect(";"); declared_types.insert(strct->name); - return strct.release(); + return strct; } -VariableDeclaration *ProgramParser::parse_variable_declaration() +RefPtr ProgramParser::parse_variable_declaration() { RefPtr var = new VariableDeclaration; @@ -632,10 +632,10 @@ VariableDeclaration *ProgramParser::parse_variable_declaration() var->init_expression = parse_expression(); expect(";"); - return var.release(); + return var; } -FunctionDeclaration *ProgramParser::parse_function_declaration() +RefPtr ProgramParser::parse_function_declaration() { RefPtr func = new FunctionDeclaration; @@ -650,7 +650,7 @@ FunctionDeclaration *ProgramParser::parse_function_declaration() RefPtr var = new VariableDeclaration; var->type = expect_type(); var->name = expect_identifier(); - func->parameters.push_back(var.release()); + func->parameters.push_back(var); } expect(")"); @@ -665,10 +665,10 @@ FunctionDeclaration *ProgramParser::parse_function_declaration() else throw runtime_error(format("Parse error at '%s': expected '{' or ';'", token)); - return func.release(); + return func; } -InterfaceBlock *ProgramParser::parse_interface_block() +RefPtr ProgramParser::parse_interface_block() { RefPtr iface = new InterfaceBlock; @@ -689,10 +689,10 @@ InterfaceBlock *ProgramParser::parse_interface_block() expect(";"); } - return iface.release(); + return iface; } -Conditional *ProgramParser::parse_conditional() +RefPtr ProgramParser::parse_conditional() { expect("if"); expect("("); @@ -709,10 +709,10 @@ Conditional *ProgramParser::parse_conditional() parse_block(cond->else_body, false); } - return cond.release(); + return cond; } -Iteration *ProgramParser::parse_iteration() +RefPtr ProgramParser::parse_iteration() { expect("for"); expect("("); @@ -725,7 +725,7 @@ Iteration *ProgramParser::parse_iteration() RefPtr expr = new ExpressionStatement; expr->expression = parse_expression(); expect(";"); - loop->init_statement = expr.release(); + loop->init_statement = expr; } loop->condition = parse_expression(); expect(";"); @@ -734,10 +734,10 @@ Iteration *ProgramParser::parse_iteration() parse_block(loop->body, false); - return loop.release(); + return loop; } -Passthrough *ProgramParser::parse_passthrough() +RefPtr ProgramParser::parse_passthrough() { expect("passthrough"); RefPtr pass = new Passthrough; @@ -748,17 +748,17 @@ Passthrough *ProgramParser::parse_passthrough() expect("]"); } expect(";"); - return pass.release(); + return pass; } -Return *ProgramParser::parse_return() +RefPtr ProgramParser::parse_return() { expect("return"); RefPtr ret = new Return; if(peek_token()!=";") ret->expression = parse_expression(); expect(";"); - return ret.release(); + return ret; } } // namespace GL diff --git a/source/programparser.h b/source/programparser.h index 4e8e38f0..0a46c09e 100644 --- a/source/programparser.h +++ b/source/programparser.h @@ -74,22 +74,22 @@ private: bool is_type(const std::string &); bool is_identifier(const std::string &); - ProgramSyntax::Node *parse_global_declaration(); - ProgramSyntax::Node *parse_statement(); - ProgramSyntax::Import *parse_import(); - ProgramSyntax::Layout *parse_layout(); + RefPtr parse_global_declaration(); + RefPtr parse_statement(); + RefPtr parse_import(); + RefPtr parse_layout(); void parse_block(ProgramSyntax::Block &, bool); - ProgramSyntax::Expression *parse_expression(unsigned = 0); - ProgramSyntax::BinaryExpression *parse_binary(ProgramSyntax::Expression *, const Operator *); - ProgramSyntax::FunctionCall *parse_function_call(ProgramSyntax::VariableReference *); - ProgramSyntax::StructDeclaration *parse_struct_declaration(); - ProgramSyntax::VariableDeclaration *parse_variable_declaration(); - ProgramSyntax::FunctionDeclaration *parse_function_declaration(); - ProgramSyntax::InterfaceBlock *parse_interface_block(); - ProgramSyntax::Conditional *parse_conditional(); - ProgramSyntax::Iteration *parse_iteration(); - ProgramSyntax::Passthrough *parse_passthrough(); - ProgramSyntax::Return *parse_return(); + RefPtr parse_expression(unsigned = 0); + RefPtr parse_binary(const RefPtr &, const Operator *); + RefPtr parse_function_call(const ProgramSyntax::VariableReference &); + RefPtr parse_struct_declaration(); + RefPtr parse_variable_declaration(); + RefPtr parse_function_declaration(); + RefPtr parse_interface_block(); + RefPtr parse_conditional(); + RefPtr parse_iteration(); + RefPtr parse_passthrough(); + RefPtr parse_return(); }; } // namespace GL diff --git a/source/programsyntax.cpp b/source/programsyntax.cpp index c6e0f6d4..e67784fe 100644 --- a/source/programsyntax.cpp +++ b/source/programsyntax.cpp @@ -192,7 +192,7 @@ void NodeVisitor::visit(Assignment &assign) void TraversingVisitor::visit(Block &block) { - for(list >::iterator i=block.body.begin(); i!=block.body.end(); ++i) + for(list >::iterator i=block.body.begin(); i!=block.body.end(); ++i) (*i)->visit(*this); } @@ -219,7 +219,7 @@ void TraversingVisitor::visit(BinaryExpression &binary) void TraversingVisitor::visit(FunctionCall &call) { - for(vector >::iterator i=call.arguments.begin(); i!=call.arguments.end(); ++i) + for(vector >::iterator i=call.arguments.begin(); i!=call.arguments.end(); ++i) (*i)->visit(*this); } @@ -255,7 +255,7 @@ void TraversingVisitor::visit(InterfaceBlock &iface) void TraversingVisitor::visit(FunctionDeclaration &func) { - for(vector >::iterator i=func.parameters.begin(); i!=func.parameters.end(); ++i) + for(vector >::iterator i=func.parameters.begin(); i!=func.parameters.end(); ++i) (*i)->visit(*this); func.body.visit(*this); } diff --git a/source/programsyntax.h b/source/programsyntax.h index c502ba47..9a0cc6ea 100644 --- a/source/programsyntax.h +++ b/source/programsyntax.h @@ -5,6 +5,7 @@ #include #include #include +#include #include "extension.h" namespace Msp { @@ -24,40 +25,13 @@ public: virtual void visit(NodeVisitor &) = 0; }; -template -class NodePtr -{ -private: - T *node; - -public: - NodePtr(T *n = 0): node(n) { } - NodePtr(const NodePtr &p): node(clone(p.node)) { } - NodePtr &operator=(const NodePtr &p) { delete node; node = clone(p.node); return *this; } -#if __cplusplus>=201103L - NodePtr(NodePtr &&p): node(p.node) { p.node = 0; } - NodePtr &operator=(NodePtr &&p) { delete node; node = p.node; p.node = 0; return *this; } -#endif - ~NodePtr() { delete node; } - -private: - static T *clone(T *n) { return n ? n->clone() : 0; } - -public: - T *operator->() { return node; } - const T *operator->() const { return node; } - T &operator*() { return *node; } - const T &operator*() const { return *node; } - operator void *() const { return node; } -}; - struct StructDeclaration; struct VariableDeclaration; struct FunctionDeclaration; struct Block: Node { - std::list > body; + std::list > body; bool use_braces; std::map types; std::map variables; @@ -83,7 +57,7 @@ struct Literal: Expression struct ParenthesizedExpression: Expression { - NodePtr expression; + RefPtr expression; virtual ParenthesizedExpression *clone() const { return new ParenthesizedExpression(*this); } virtual void visit(NodeVisitor &); @@ -102,7 +76,7 @@ struct VariableReference: Expression struct MemberAccess: Expression { - NodePtr left; + RefPtr left; std::string member; VariableDeclaration *declaration; @@ -113,7 +87,7 @@ struct MemberAccess: Expression struct UnaryExpression: Expression { std::string oper; - NodePtr expression; + RefPtr expression; bool prefix; UnaryExpression(); @@ -124,9 +98,9 @@ struct UnaryExpression: Expression struct BinaryExpression: Expression { - NodePtr left; + RefPtr left; std::string oper; - NodePtr right; + RefPtr right; std::string after; virtual BinaryExpression *clone() const { return new BinaryExpression(*this); } @@ -149,7 +123,7 @@ struct FunctionCall: Expression std::string name; FunctionDeclaration *declaration; bool constructor; - std::vector > arguments; + std::vector > arguments; FunctionCall(); @@ -159,7 +133,7 @@ struct FunctionCall: Expression struct ExpressionStatement: Node { - NodePtr expression; + RefPtr expression; virtual ExpressionStatement *clone() const { return new ExpressionStatement(*this); } virtual void visit(NodeVisitor &); @@ -216,10 +190,10 @@ struct VariableDeclaration: Node StructDeclaration *type_declaration; std::string name; bool array; - NodePtr array_size; - NodePtr init_expression; + RefPtr array_size; + RefPtr init_expression; VariableDeclaration *linked_declaration; - NodePtr layout; + RefPtr layout; VariableDeclaration(); @@ -245,7 +219,7 @@ struct FunctionDeclaration: Node { std::string return_type; std::string name; - std::vector > parameters; + std::vector > parameters; FunctionDeclaration *definition; Block body; @@ -258,7 +232,7 @@ struct FunctionDeclaration: Node struct Conditional: Node { - NodePtr condition; + RefPtr condition; Block body; Block else_body; @@ -268,9 +242,9 @@ struct Conditional: Node struct Iteration: Node { - NodePtr init_statement; - NodePtr condition; - NodePtr loop_expression; + RefPtr init_statement; + RefPtr condition; + RefPtr loop_expression; Block body; virtual Iteration *clone() const { return new Iteration(*this); } @@ -279,7 +253,7 @@ struct Iteration: Node struct Passthrough: Node { - NodePtr subscript; + RefPtr subscript; virtual Passthrough *clone() const { return new Passthrough(*this); } virtual void visit(NodeVisitor &); @@ -287,7 +261,7 @@ struct Passthrough: Node struct Return: Node { - NodePtr expression; + RefPtr expression; virtual Return *clone() const { return new Return(*this); } virtual void visit(NodeVisitor &); -- 2.43.0