From: Juerg Billeter Date: Wed, 23 Jan 2008 15:26:07 +0000 (+0000) Subject: build control flow graph, report error for missing return statement in X-Git-Tag: VALA_0_1_7~197 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=92a2bd696c9d174bbd579b44ea6823a88bef9a97;p=thirdparty%2Fvala.git build control flow graph, report error for missing return statement in 2008-01-23 Juerg Billeter * vala/Makefile.am, vala/valabasicblock.vala, vala/valacfgbuilder.vala, vala/valadostatement.vala, vala/valaforstatement.vala, vala/valaifstatement.vala, vala/valamemorymanager.vala, vala/valamethod.vala, vala/valasemanticanalyzer.vala, vala/valasymbolresolver.vala, vala/valawhilestatement.vala, gobject/valaccodegenerator.vala, compiler/valacompiler.vala: build control flow graph, report error for missing return statement in non-void methods, and report warning for unreachable code, fixes bug 508480 * tests/exceptions.vala: add missing return statement svn path=/trunk/; revision=894 --- diff --git a/ChangeLog b/ChangeLog index c325d5a3a..9e625d966 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2008-01-23 Jürg Billeter + + * vala/Makefile.am, vala/valabasicblock.vala, vala/valacfgbuilder.vala, + vala/valadostatement.vala, vala/valaforstatement.vala, + vala/valaifstatement.vala, vala/valamemorymanager.vala, + vala/valamethod.vala, vala/valasemanticanalyzer.vala, + vala/valasymbolresolver.vala, vala/valawhilestatement.vala, + gobject/valaccodegenerator.vala, compiler/valacompiler.vala: build + control flow graph, report error for missing return statement in + non-void methods, and report warning for unreachable code, + fixes bug 508480 + + * tests/exceptions.vala: add missing return statement + 2008-01-23 Jürg Billeter * tests/Makefile.am, tests/testrunner.sh: honor EXEEXT to fix tests diff --git a/compiler/valacompiler.vala b/compiler/valacompiler.vala index 039fadeb8..5d1d2406f 100644 --- a/compiler/valacompiler.vala +++ b/compiler/valacompiler.vala @@ -269,6 +269,13 @@ class Vala.Compiler : Object { return quit (); } + var cfg_builder = new CFGBuilder (); + cfg_builder.build_cfg (context); + + if (Report.get_errors () > 0) { + return quit (); + } + var memory_manager = new MemoryManager (); memory_manager.analyze (context); diff --git a/gobject/valaccodegenerator.vala b/gobject/valaccodegenerator.vala index 0edceec62..d899b60b8 100644 --- a/gobject/valaccodegenerator.vala +++ b/gobject/valaccodegenerator.vala @@ -1341,6 +1341,8 @@ public class Vala.CCodeGenerator : CodeGenerator { } public override void visit_if_statement (IfStatement! stmt) { + stmt.accept_children (this); + if (stmt.false_statement != null) { stmt.ccodenode = new CCodeIfStatement ((CCodeExpression) stmt.condition.ccodenode, (CCodeStatement) stmt.true_statement.ccodenode, (CCodeStatement) stmt.false_statement.ccodenode); } else { @@ -1496,18 +1498,24 @@ public class Vala.CCodeGenerator : CodeGenerator { } public override void visit_while_statement (WhileStatement! stmt) { + stmt.accept_children (this); + stmt.ccodenode = new CCodeWhileStatement ((CCodeExpression) stmt.condition.ccodenode, (CCodeStatement) stmt.body.ccodenode); create_temp_decl (stmt, stmt.condition.temp_vars); } public override void visit_do_statement (DoStatement! stmt) { + stmt.accept_children (this); + stmt.ccodenode = new CCodeDoStatement ((CCodeStatement) stmt.body.ccodenode, (CCodeExpression) stmt.condition.ccodenode); create_temp_decl (stmt, stmt.condition.temp_vars); } public override void visit_for_statement (ForStatement! stmt) { + stmt.accept_children (this); + var cfor = new CCodeForStatement ((CCodeExpression) stmt.condition.ccodenode, (CCodeStatement) stmt.body.ccodenode); stmt.ccodenode = cfor; diff --git a/tests/exceptions.vala b/tests/exceptions.vala index e9e9f5482..1da885f93 100644 --- a/tests/exceptions.vala +++ b/tests/exceptions.vala @@ -23,6 +23,8 @@ class Maman.Bar : Object { foo (); stdout.printf (" BAD"); + + return 0; } public void good () throws BarError { diff --git a/vala/Makefile.am b/vala/Makefile.am index 1ddb74410..3ad6f35f7 100644 --- a/vala/Makefile.am +++ b/vala/Makefile.am @@ -24,6 +24,7 @@ libvalacore_la_VALASOURCES = \ valaattribute.vala \ valaattributeprocessor.vala \ valabaseaccess.vala \ + valabasicblock.vala \ valabinaryexpression.vala \ valabindingprovider.vala \ valablock.vala \ @@ -31,6 +32,7 @@ libvalacore_la_VALASOURCES = \ valabreakstatement.vala \ valacastexpression.vala \ valacatchclause.vala \ + valacfgbuilder.vala \ valacharacterliteral.vala \ valaclass.vala \ valaclasstype.vala \ diff --git a/vala/valabasicblock.vala b/vala/valabasicblock.vala new file mode 100644 index 000000000..bee017147 --- /dev/null +++ b/vala/valabasicblock.vala @@ -0,0 +1,65 @@ +/* valabasicblock.vala + * + * Copyright (C) 2008 Jürg Billeter + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Author: + * Jürg Billeter + */ + +using GLib; +using Gee; + +/** + * Represents a basic block, i.e. a straight-line piece of code without any + * jumps or jump targets. + */ +public class Vala.BasicBlock : Object { + private Gee.List nodes = new ArrayList (); + + private Gee.List predecessors = new ArrayList (); + private Gee.List successors = new ArrayList (); + + public BasicBlock () { + } + + public BasicBlock.entry () { + } + + public BasicBlock.exit () { + } + + public void add_node (CodeNode node) { + nodes.add (node); + } + + public void connect (BasicBlock target) { + if (!successors.contains (target)) { + successors.add (target); + } + if (!target.predecessors.contains (this)) { + target.predecessors.add (this); + } + } + + public Gee.List get_predecessors () { + return new ReadOnlyList (predecessors); + } + + public Gee.List get_successors () { + return new ReadOnlyList (successors); + } +} diff --git a/vala/valacfgbuilder.vala b/vala/valacfgbuilder.vala new file mode 100644 index 000000000..b6a882b41 --- /dev/null +++ b/vala/valacfgbuilder.vala @@ -0,0 +1,395 @@ +/* valacfgbuilder.vala + * + * Copyright (C) 2008 Jürg Billeter + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Author: + * Jürg Billeter + */ + +using GLib; +using Gee; + +/** + * Code visitor building the control flow graph. + */ +public class Vala.CFGBuilder : CodeVisitor { + private CodeContext context; + private BasicBlock current_block; + private bool unreachable_reported; + private Method current_method; + private Gee.List breakable_stack = new ArrayList (); + private Gee.List continuable_stack = new ArrayList (); + + public CFGBuilder () { + } + + /** + * Build control flow graph in the specified context. + * + * @param context a code context + */ + public void build_cfg (CodeContext! context) { + this.context = context; + + /* we're only interested in non-pkg source files */ + var source_files = context.get_source_files (); + foreach (SourceFile file in source_files) { + if (!file.pkg) { + file.accept (this); + } + } + } + + public override void visit_source_file (SourceFile! source_file) { + source_file.accept_children (this); + } + + public override void visit_class (Class! cl) { + cl.accept_children (this); + } + + public override void visit_struct (Struct! st) { + st.accept_children (this); + } + + public override void visit_interface (Interface! iface) { + iface.accept_children (this); + } + + public override void visit_enum (Enum! en) { + en.accept_children (this); + } + + public override void visit_method (Method! m) { + if (m.body == null) { + return; + } + + var old_method = current_method; + current_method = m; + + m.entry_block = new BasicBlock.entry (); + m.exit_block = new BasicBlock.exit (); + + current_block = new BasicBlock (); + m.entry_block.connect (current_block); + + m.accept_children (this); + + if (current_block != null) { + // end of method body reachable + + if (!(m.return_type is VoidType)) { + Report.error (m.source_reference, "missing return statement at end of method body"); + m.error = true; + } + + current_block.connect (m.exit_block); + } + + current_method = old_method; + } + + public override void visit_block (Block! b) { + b.accept_children (this); + } + + public override void visit_declaration_statement (DeclarationStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + current_block.add_node (stmt); + } + + public override void visit_expression_statement (ExpressionStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + current_block.add_node (stmt); + } + + public override void visit_if_statement (IfStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + // condition + current_block.add_node (stmt.condition); + + // true block + var last_block = current_block; + current_block = new BasicBlock (); + last_block.connect (current_block); + stmt.true_statement.accept (this); + + // false block + var last_true_block = current_block; + current_block = new BasicBlock (); + last_block.connect (current_block); + if (stmt.false_statement != null) { + stmt.false_statement.accept (this); + } + + // after if/else + var last_false_block = current_block; + // reachable? + if (last_true_block != null || last_false_block != null) { + current_block = new BasicBlock (); + if (last_true_block != null) { + last_true_block.connect (current_block); + } + if (last_false_block != null) { + last_false_block.connect (current_block); + } + } + } + + public override void visit_while_statement (WhileStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + var condition_block = new BasicBlock (); + continuable_stack.add (condition_block); + var after_loop_block = new BasicBlock (); + breakable_stack.add (after_loop_block); + + // condition + var last_block = current_block; + last_block.connect (condition_block); + condition_block.add_node (stmt.condition); + + // loop block + current_block = new BasicBlock (); + condition_block.connect (current_block); + stmt.body.accept (this); + // end of loop block reachable? + if (current_block != null) { + current_block.connect (condition_block); + } + + // after loop + condition_block.connect (after_loop_block); + current_block = after_loop_block; + + continuable_stack.remove_at (continuable_stack.size - 1); + breakable_stack.remove_at (breakable_stack.size - 1); + } + + public override void visit_do_statement (DoStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + var condition_block = new BasicBlock (); + continuable_stack.add (condition_block); + var after_loop_block = new BasicBlock (); + breakable_stack.add (after_loop_block); + + // loop block + var last_block = current_block; + var loop_block = new BasicBlock (); + last_block.connect (loop_block); + current_block = loop_block; + stmt.body.accept (this); + + // condition + // reachable? + if (current_block != null || condition_block.get_predecessors ().size > 0) { + if (current_block != null) { + last_block = current_block; + last_block.connect (condition_block); + } + condition_block.add_node (stmt.condition); + condition_block.connect (loop_block); + current_block = condition_block; + } + + // after loop + // reachable? + if (current_block != null || after_loop_block.get_predecessors ().size > 0) { + if (current_block != null) { + last_block = current_block; + last_block.connect (after_loop_block); + } + current_block = after_loop_block; + } + + continuable_stack.remove_at (continuable_stack.size - 1); + breakable_stack.remove_at (breakable_stack.size - 1); + } + + public override void visit_for_statement (ForStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + // initializer + foreach (Expression init_expr in stmt.get_initializer ()) { + current_block.add_node (init_expr); + } + + var iterator_block = new BasicBlock (); + continuable_stack.add (iterator_block); + var after_loop_block = new BasicBlock (); + breakable_stack.add (after_loop_block); + + // condition + var condition_block = new BasicBlock (); + current_block.connect (condition_block); + condition_block.add_node (stmt.condition); + + // loop block + current_block = new BasicBlock (); + condition_block.connect (current_block); + stmt.body.accept (this); + + // iterator + // reachable? + if (current_block != null || iterator_block.get_predecessors ().size > 0) { + if (current_block != null) { + current_block.connect (iterator_block); + } + foreach (Expression it_expr in stmt.get_iterator ()) { + iterator_block.add_node (it_expr); + } + iterator_block.connect (condition_block); + } + + // after loop + condition_block.connect (after_loop_block); + current_block = after_loop_block; + + continuable_stack.remove_at (continuable_stack.size - 1); + breakable_stack.remove_at (breakable_stack.size - 1); + } + + public override void visit_foreach_statement (ForeachStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + var loop_block = new BasicBlock (); + continuable_stack.add (loop_block); + var after_loop_block = new BasicBlock (); + breakable_stack.add (after_loop_block); + + // loop block + var last_block = current_block; + last_block.connect (loop_block); + current_block = loop_block; + stmt.body.accept (this); + if (current_block != null) { + current_block.connect (loop_block); + } + + // after loop + last_block.connect (after_loop_block); + if (current_block != null) { + current_block.connect (after_loop_block); + } + current_block = after_loop_block; + + continuable_stack.remove_at (continuable_stack.size - 1); + breakable_stack.remove_at (breakable_stack.size - 1); + } + + public override void visit_break_statement (BreakStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + current_block.add_node (stmt); + current_block.connect (top_breakable ()); + current_block = null; + unreachable_reported = false; + } + + public override void visit_continue_statement (ContinueStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + current_block.add_node (stmt); + current_block.connect (top_continuable ()); + current_block = null; + unreachable_reported = false; + } + + public override void visit_return_statement (ReturnStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + current_block.add_node (stmt); + current_block.connect (current_method.exit_block); + current_block = null; + unreachable_reported = false; + } + + public override void visit_throw_statement (ThrowStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + current_block.add_node (stmt); + // TODO connect to catch blocks instead of exit block if appropriate + current_block.connect (current_method.exit_block); + current_block = null; + unreachable_reported = false; + } + + public override void visit_try_statement (TryStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + stmt.body.accept (this); + + // TODO exceptional control flow + } + + public override void visit_lock_statement (LockStatement! stmt) { + if (unreachable (stmt)) { + return; + } + + stmt.body.accept (this); + } + + private bool unreachable (CodeNode node) { + if (current_block == null) { + if (!unreachable_reported) { + Report.warning (node.source_reference, "unreachable code detected"); + unreachable_reported = true; + } + return true; + } + + return false; + } + + private BasicBlock top_breakable () { + return breakable_stack.get (breakable_stack.size - 1); + } + + private BasicBlock top_continuable () { + return continuable_stack.get (continuable_stack.size - 1); + } +} diff --git a/vala/valadostatement.vala b/vala/valadostatement.vala index 3d2379594..e5fdd22c6 100644 --- a/vala/valadostatement.vala +++ b/vala/valadostatement.vala @@ -1,6 +1,6 @@ /* valadostatement.vala * - * Copyright (C) 2006-2007 Jürg Billeter + * Copyright (C) 2006-2008 Jürg Billeter * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -65,15 +65,17 @@ public class Vala.DoStatement : CodeNode, Statement { */ public DoStatement (construct Block! body, construct Expression! condition, construct SourceReference source_reference = null) { } - + public override void accept (CodeVisitor! visitor) { + visitor.visit_do_statement (this); + } + + public override void accept_children (CodeVisitor! visitor) { body.accept (visitor); condition.accept (visitor); visitor.visit_end_full_expression (condition); - - visitor.visit_do_statement (this); } public override void replace_expression (Expression! old_node, Expression! new_node) { diff --git a/vala/valaforstatement.vala b/vala/valaforstatement.vala index 7d6327bde..68676e0d1 100644 --- a/vala/valaforstatement.vala +++ b/vala/valaforstatement.vala @@ -1,6 +1,6 @@ /* valaforstatement.vala * - * Copyright (C) 2006-2007 Jürg Billeter + * Copyright (C) 2006-2008 Jürg Billeter * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -109,6 +109,10 @@ public class Vala.ForStatement : CodeNode, Statement { } public override void accept (CodeVisitor! visitor) { + visitor.visit_for_statement (this); + } + + public override void accept_children (CodeVisitor! visitor) { foreach (Expression init_expr in initializer) { init_expr.accept (visitor); visitor.visit_end_full_expression (init_expr); @@ -124,8 +128,6 @@ public class Vala.ForStatement : CodeNode, Statement { } body.accept (visitor); - - visitor.visit_for_statement (this); } public override void replace_expression (Expression! old_node, Expression! new_node) { diff --git a/vala/valaifstatement.vala b/vala/valaifstatement.vala index b12916a9c..237a00edd 100644 --- a/vala/valaifstatement.vala +++ b/vala/valaifstatement.vala @@ -1,6 +1,6 @@ /* valaifstatement.vala * - * Copyright (C) 2006-2007 Jürg Billeter + * Copyright (C) 2006-2008 Jürg Billeter * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -67,6 +67,10 @@ public class Vala.IfStatement : CodeNode, Statement { } public override void accept (CodeVisitor! visitor) { + visitor.visit_if_statement (this); + } + + public override void accept_children (CodeVisitor! visitor) { condition.accept (visitor); visitor.visit_end_full_expression (condition); @@ -75,8 +79,6 @@ public class Vala.IfStatement : CodeNode, Statement { if (false_statement != null) { false_statement.accept (visitor); } - - visitor.visit_if_statement (this); } public override void replace_expression (Expression! old_node, Expression! new_node) { diff --git a/vala/valamemorymanager.vala b/vala/valamemorymanager.vala index 9c7e79736..375027a4d 100644 --- a/vala/valamemorymanager.vala +++ b/vala/valamemorymanager.vala @@ -149,10 +149,26 @@ public class Vala.MemoryManager : CodeVisitor { visit_possibly_leaked_expression (stmt.expression); } + public override void visit_if_statement (IfStatement! stmt) { + stmt.accept_children (this); + } + public override void visit_switch_section (SwitchSection! section) { section.accept_children (this); } + public override void visit_while_statement (WhileStatement! stmt) { + stmt.accept_children (this); + } + + public override void visit_do_statement (DoStatement! stmt) { + stmt.accept_children (this); + } + + public override void visit_for_statement (ForStatement! stmt) { + stmt.accept_children (this); + } + public override void visit_foreach_statement (ForeachStatement! stmt) { stmt.accept_children (this); } diff --git a/vala/valamethod.vala b/vala/valamethod.vala index 59cb07016..d94c335c0 100644 --- a/vala/valamethod.vala +++ b/vala/valamethod.vala @@ -42,7 +42,11 @@ public class Vala.Method : Member { } public Block body { get; set; } - + + public BasicBlock entry_block { get; set; } + + public BasicBlock exit_block { get; set; } + /** * Specifies whether this method may only be called with an instance of * the contained type. diff --git a/vala/valasemanticanalyzer.vala b/vala/valasemanticanalyzer.vala index 4288c8cb1..b4b1525f9 100644 --- a/vala/valasemanticanalyzer.vala +++ b/vala/valasemanticanalyzer.vala @@ -858,6 +858,8 @@ public class Vala.SemanticAnalyzer : CodeVisitor { } public override void visit_if_statement (IfStatement! stmt) { + stmt.accept_children (this); + if (stmt.condition.error) { /* if there was an error in the condition, skip this check */ stmt.error = true; @@ -891,6 +893,24 @@ public class Vala.SemanticAnalyzer : CodeVisitor { } public override void visit_while_statement (WhileStatement! stmt) { + stmt.accept_children (this); + + if (stmt.condition.error) { + /* if there was an error in the condition, skip this check */ + stmt.error = true; + return; + } + + if (!stmt.condition.static_type.compatible (bool_type)) { + stmt.error = true; + Report.error (stmt.condition.source_reference, "Condition must be boolean"); + return; + } + } + + public override void visit_do_statement (DoStatement! stmt) { + stmt.accept_children (this); + if (stmt.condition.error) { /* if there was an error in the condition, skip this check */ stmt.error = true; @@ -905,6 +925,8 @@ public class Vala.SemanticAnalyzer : CodeVisitor { } public override void visit_for_statement (ForStatement! stmt) { + stmt.accept_children (this); + if (stmt.condition.error) { /* if there was an error in the condition, skip this check */ stmt.error = true; diff --git a/vala/valasymbolresolver.vala b/vala/valasymbolresolver.vala index 0872f3df1..7ae1adac1 100644 --- a/vala/valasymbolresolver.vala +++ b/vala/valasymbolresolver.vala @@ -322,10 +322,26 @@ public class Vala.SymbolResolver : CodeVisitor { list.accept_children (this); } + public override void visit_if_statement (IfStatement! stmt) { + stmt.accept_children (this); + } + public override void visit_switch_section (SwitchSection! section) { section.accept_children (this); } + public override void visit_while_statement (WhileStatement! stmt) { + stmt.accept_children (this); + } + + public override void visit_do_statement (DoStatement! stmt) { + stmt.accept_children (this); + } + + public override void visit_for_statement (ForStatement! stmt) { + stmt.accept_children (this); + } + public override void visit_foreach_statement (ForeachStatement! stmt) { stmt.accept_children (this); } diff --git a/vala/valawhilestatement.vala b/vala/valawhilestatement.vala index a1afdce70..77d16e0fb 100644 --- a/vala/valawhilestatement.vala +++ b/vala/valawhilestatement.vala @@ -1,6 +1,6 @@ /* valawhilestatement.vala * - * Copyright (C) 2006-2007 Jürg Billeter + * Copyright (C) 2006-2008 Jürg Billeter * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -67,13 +67,15 @@ public class Vala.WhileStatement : CodeNode, Statement { } public override void accept (CodeVisitor! visitor) { + visitor.visit_while_statement (this); + } + + public override void accept_children (CodeVisitor! visitor) { condition.accept (visitor); visitor.visit_end_full_expression (condition); body.accept (visitor); - - visitor.visit_while_statement (this); } public override void replace_expression (Expression! old_node, Expression! new_node) {