]> git.ipfire.org Git - thirdparty/vala.git/commitdiff
codegen: Properly handle and catch inner-error of finally-block
authorRico Tzschichholz <ricotz@ubuntu.com>
Wed, 23 Jan 2019 21:39:42 +0000 (22:39 +0100)
committerRico Tzschichholz <ricotz@ubuntu.com>
Tue, 26 Feb 2019 16:53:41 +0000 (17:53 +0100)
If all inner-errors are caught there is no jump-out of the current finally-
block therefore the control-flow analyzer is happy. To make this work the
surrounding inner-error must not be used or changed here.

Fixes https://gitlab.gnome.org/GNOME/vala/issues/742

codegen/valaccodebasemodule.vala
codegen/valaccodemethodcallmodule.vala
codegen/valaccodemethodmodule.vala
codegen/valagdbusclientmodule.vala
codegen/valagdbusservermodule.vala
codegen/valagerrormodule.vala
codegen/valagobjectmodule.vala
codegen/valagtypemodule.vala
tests/Makefile.am
tests/asynchronous/catch-in-finally.vala [new file with mode: 0644]
tests/errors/catch-in-finally.vala [new file with mode: 0644]

index 519fd8589c1d9e916a4f6fdf51a80092a363bb06..447dee408e13a8cfafa48328ae52deb8807f5817 100644 (file)
@@ -36,6 +36,7 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                public ArrayList<CCodeFunction> ccode_stack = new ArrayList<CCodeFunction> ();
                public ArrayList<TargetValue> temp_ref_values = new ArrayList<TargetValue> ();
                public int next_temp_var_id;
+               public int current_inner_error_id;
                public bool current_method_inner_error;
                public bool current_method_return;
                public int next_coroutine_state = 1;
@@ -81,6 +82,11 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                set { emit_context.current_catch = value; }
        }
 
+       public int current_inner_error_id {
+               get { return emit_context.current_inner_error_id; }
+               set { emit_context.current_inner_error_id = value; }
+       }
+
        public TypeSymbol? current_type_symbol {
                get {
                        var sym = current_symbol;
@@ -273,6 +279,9 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
        int next_block_id = 0;
        Map<Block,int> block_map = new HashMap<Block,int> ();
 
+       /* count of emitted inner_error variables in methods */
+       Map<weak Method,int> method_inner_error_var_count = new HashMap<weak Method,int> ();
+
        public DataType void_type = new VoidType ();
        public DataType bool_type;
        public DataType char_type;
@@ -1884,7 +1893,7 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                        }
 
                        if (current_method_inner_error) {
-                               ccode.add_declaration ("GError *", new CCodeVariableDeclarator.zero ("_inner_error_", new CCodeConstant ("NULL")));
+                               ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero ("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
                        }
 
                        cfile.add_function (function);
@@ -1962,6 +1971,24 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                        ccode.open_block ();
                }
 
+               if (b.parent_node is TryStatement && b == ((TryStatement) b.parent_node).finally_body) {
+                       // finally-block with nested error handling
+                       if (((TryStatement) b.parent_node).body.tree_can_fail) {
+                               if (is_in_coroutine ()) {
+                                       // _inner_error0_ gets added to closure_struct in CCodeMethodModule.visit_method()
+                                       if (current_inner_error_id > 0
+                                           && (!method_inner_error_var_count.contains (current_method)
+                                           || method_inner_error_var_count.get (current_method) < current_inner_error_id)) {
+                                               // no initialization necessary, closure struct is zeroed
+                                               closure_struct.add_field ("GError*", "_inner_error%d_".printf (current_inner_error_id));
+                                               method_inner_error_var_count.set (current_method, current_inner_error_id);
+                                       }
+                               } else {
+                                       ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero ("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
+                               }
+                       }
+               }
+
                if (b.captured) {
                        var parent_block = next_closure_block (b.parent_symbol);
 
@@ -2337,6 +2364,14 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                }
        }
 
+       public CCodeExpression get_inner_error_cexpression () {
+               if (is_in_coroutine ()) {
+                       return new CCodeMemberAccess.pointer (new CCodeIdentifier ("_data_"), "_inner_error%d_".printf (current_inner_error_id));
+               } else {
+                       return new CCodeIdentifier ("_inner_error%d_".printf (current_inner_error_id));
+               }
+       }
+
        public string get_local_cname (LocalVariable local) {
                var cname = get_variable_cname (local.name);
                if (cname[0].isdigit ()) {
@@ -4900,7 +4935,7 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                                // method can fail
                                current_method_inner_error = true;
                                // add &inner_error before the ellipsis arguments
-                               out_arg_map.set (get_param_pos (-1), new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_variable_cexpression ("_inner_error_")));
+                               out_arg_map.set (get_param_pos (-1), new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_inner_error_cexpression ()));
                        }
 
                        if (ellipsis) {
index a3f6030cabbdc62f7af6f24d20a7bf0cf76c8498..31cb0a7768ca716123edd05f418b5558c855a508 100644 (file)
@@ -644,7 +644,7 @@ public class Vala.CCodeMethodCallModule : CCodeAssignmentModule {
                        // method can fail
                        current_method_inner_error = true;
                        // add &inner_error before the ellipsis arguments
-                       out_arg_map.set (get_param_pos (-1), new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_variable_cexpression ("_inner_error_")));
+                       out_arg_map.set (get_param_pos (-1), new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_inner_error_cexpression ()));
                } else if (m != null && m.has_error_type_parameter () && async_call != ccall) {
                        // inferred error argument from base method
                        out_arg_map.set (get_param_pos (-1), new CCodeConstant ("NULL"));
index 0c8dbe25abaaea332c235ebecd1ec9f790e1ea91..e51ba00cff23dd9ade314c9c7cd17c31ed4fc349 100644 (file)
@@ -747,11 +747,10 @@ public abstract class Vala.CCodeMethodModule : CCodeStructModule {
                                         * as error may be set to NULL but we're always interested in inner errors
                                         */
                                        if (m.coroutine) {
-                                               closure_struct.add_field ("GError *", "_inner_error_");
-
                                                // no initialization necessary, closure struct is zeroed
+                                               closure_struct.add_field ("GError*", "_inner_error%d_".printf (current_inner_error_id));
                                        } else {
-                                               ccode.add_declaration ("GError *", new CCodeVariableDeclarator.zero ("_inner_error_", new CCodeConstant ("NULL")));
+                                               ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero ("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
                                        }
                                }
 
index 32ecb66667bf4da6f1cc4bd8df3a4e4393d9b2cd..4e4668d2e915ba228c3a729dc876eca564090a76 100644 (file)
@@ -321,7 +321,7 @@ public class Vala.GDBusClientModule : GDBusModule {
                                var ccall = new CCodeFunctionCall (new CCodeIdentifier ("g_async_initable_new_finish"));
                                ccall.add_argument (new CCodeCastExpression (source_ref, "GAsyncInitable *"));
                                ccall.add_argument (get_cvalue (res));
-                               ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_variable_cexpression ("_inner_error_")));
+                               ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_inner_error_cexpression ()));
 
                                var temp_var = get_temp_variable (expr.value_type, expr.value_type.value_owned);
                                var temp_ref = get_variable_cexpression (temp_var.name);
@@ -376,7 +376,7 @@ public class Vala.GDBusClientModule : GDBusModule {
                                ccall.add_argument (get_delegate_target (callback));
                        }
                } else {
-                       ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_variable_cexpression ("_inner_error_")));
+                       ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_inner_error_cexpression ()));
                }
                ccall.add_argument (new CCodeConstant ("\"g-flags\""));
                ccall.add_argument (get_cvalue (flags));
@@ -418,7 +418,7 @@ public class Vala.GDBusClientModule : GDBusModule {
                                ccall.add_argument (new CCodeCastExpression (new CCodeMemberAccess.pointer (new CCodeIdentifier ("_data_"), "_source_object_"), "GAsyncInitable *"));
                                // pass GAsyncResult stored in closure to finish function
                                ccall.add_argument (new CCodeMemberAccess.pointer (new CCodeIdentifier ("_data_"), "_res_"));
-                               ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_variable_cexpression ("_inner_error_")));
+                               ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_inner_error_cexpression ()));
                        } else {
                                // begin
                                ccode.add_expression (ccall);
index e1984e24f796ee27431be86765e8c41eef77f93c..b539daee4aae5cdcdc710524182d307facb7c79a 100644 (file)
@@ -1016,7 +1016,7 @@ public class Vala.GDBusServerModule : GDBusClientModule {
                cregister.add_argument (get_cvalue (obj_arg));
                cregister.add_argument (get_cvalue (ma.inner));
                cregister.add_argument (get_cvalue (path_arg));
-               cregister.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_variable_cexpression ("_inner_error_")));
+               cregister.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_inner_error_cexpression ()));
 
                if (expr.parent_node is ExpressionStatement) {
                        ccode.add_expression (cregister);
index ea8367fd361a924f6f006287f5bd8d610af3f7c6..eda2493cee0d1cd18d0246ea86625c4aa6f61cce 100644 (file)
@@ -87,7 +87,7 @@ public class Vala.GErrorModule : CCodeDelegateModule {
        public override void visit_throw_statement (ThrowStatement stmt) {
                // method will fail
                current_method_inner_error = true;
-               ccode.add_assignment (get_variable_cexpression ("_inner_error_"), get_cvalue (stmt.error_expression));
+               ccode.add_assignment (get_inner_error_cexpression (), get_cvalue (stmt.error_expression));
 
                add_simple_check (stmt, true);
        }
@@ -168,13 +168,11 @@ public class Vala.GErrorModule : CCodeDelegateModule {
        public override void add_simple_check (CodeNode node, bool always_fails = false) {
                current_method_inner_error = true;
 
-               var inner_error = get_variable_cexpression ("_inner_error_");
-
                if (always_fails) {
                        // inner_error is always set, avoid unnecessary if statement
                        // eliminates C warnings
                } else {
-                       var ccond = new CCodeBinaryExpression (CCodeBinaryOperator.INEQUALITY, inner_error, new CCodeConstant ("NULL"));
+                       var ccond = new CCodeBinaryExpression (CCodeBinaryOperator.INEQUALITY, get_inner_error_cexpression (), new CCodeConstant ("NULL"));
                        var unlikely = new CCodeFunctionCall (new CCodeIdentifier ("G_UNLIKELY"));
                        unlikely.add_argument (ccond);
                        ccode.open_if (unlikely);
@@ -222,7 +220,7 @@ public class Vala.GErrorModule : CCodeDelegateModule {
                                                if (catch_type.error_code != null) {
                                                        /* catch clause specifies a specific error code */
                                                        var error_match = new CCodeFunctionCall (new CCodeIdentifier ("g_error_matches"));
-                                                       error_match.add_argument (inner_error);
+                                                       error_match.add_argument (get_inner_error_cexpression ());
                                                        error_match.add_argument (new CCodeIdentifier (get_ccode_upper_case_name (catch_type.data_type)));
                                                        error_match.add_argument (new CCodeIdentifier (get_ccode_name (catch_type.error_code)));
 
@@ -230,7 +228,7 @@ public class Vala.GErrorModule : CCodeDelegateModule {
                                                } else {
                                                        /* catch clause specifies a full error domain */
                                                        var ccond = new CCodeBinaryExpression (CCodeBinaryOperator.EQUALITY,
-                                                                       new CCodeMemberAccess.pointer (inner_error, "domain"), new CCodeIdentifier
+                                                                       new CCodeMemberAccess.pointer (get_inner_error_cexpression (), "domain"), new CCodeIdentifier
                                                                        (get_ccode_upper_case_name (clause.error_type.data_type)));
 
                                                        ccode.open_if (ccond);
@@ -256,7 +254,7 @@ public class Vala.GErrorModule : CCodeDelegateModule {
                                // as jump out of finally block is not supported
                        } else {
                                // should never happen with correct bindings
-                               uncaught_error_statement (inner_error, true);
+                               uncaught_error_statement (get_inner_error_cexpression (), true);
                        }
                } else if (current_method != null && current_method.get_error_types ().size > 0) {
                        // current method can fail, propagate error
@@ -271,7 +269,7 @@ public class Vala.GErrorModule : CCodeDelegateModule {
 
                                // Check the allowed error domains to propagate
                                var domain_check = new CCodeBinaryExpression (CCodeBinaryOperator.EQUALITY, new CCodeMemberAccess.pointer
-                                       (inner_error, "domain"), new CCodeIdentifier (get_ccode_upper_case_name (error_type.data_type)));
+                                       (get_inner_error_cexpression (), "domain"), new CCodeIdentifier (get_ccode_upper_case_name (error_type.data_type)));
                                if (ccond == null) {
                                        ccond = domain_check;
                                } else {
@@ -281,16 +279,16 @@ public class Vala.GErrorModule : CCodeDelegateModule {
 
                        if (ccond != null) {
                                ccode.open_if (ccond);
-                               return_with_exception (inner_error);
+                               return_with_exception (get_inner_error_cexpression ());
 
                                ccode.add_else ();
-                               uncaught_error_statement (inner_error);
+                               uncaught_error_statement (get_inner_error_cexpression ());
                                ccode.close ();
                        } else {
-                               return_with_exception (inner_error);
+                               return_with_exception (get_inner_error_cexpression ());
                        }
                } else {
-                       uncaught_error_statement (inner_error);
+                       uncaught_error_statement (get_inner_error_cexpression ());
                }
 
                if (!always_fails) {
@@ -330,7 +328,11 @@ public class Vala.GErrorModule : CCodeDelegateModule {
 
                ccode.add_label ("__finally%d".printf (this_try_id));
                if (stmt.finally_body != null) {
+                       // use a dedicated inner_error variable, if there
+                       // is some error handling happening in finally-block
+                       current_inner_error_id++;
                        stmt.finally_body.emit (this);
+                       current_inner_error_id--;
                }
 
                // check for errors not handled by this try statement
@@ -352,14 +354,14 @@ public class Vala.GErrorModule : CCodeDelegateModule {
 
                if (clause.error_variable != null) {
                        visit_local_variable (clause.error_variable);
-                       ccode.add_assignment (get_variable_cexpression (get_local_cname (clause.error_variable)), get_variable_cexpression ("_inner_error_"));
+                       ccode.add_assignment (get_variable_cexpression (get_local_cname (clause.error_variable)), get_inner_error_cexpression ());
                } else {
                        // error object is not used within catch statement, clear it
                        var cclear = new CCodeFunctionCall (new CCodeIdentifier ("g_clear_error"));
-                       cclear.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_variable_cexpression ("_inner_error_")));
+                       cclear.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_inner_error_cexpression ()));
                        ccode.add_expression (cclear);
                }
-               ccode.add_assignment (get_variable_cexpression ("_inner_error_"), new CCodeConstant ("NULL"));
+               ccode.add_assignment (get_inner_error_cexpression (), new CCodeConstant ("NULL"));
 
                clause.body.emit (this);
 
index ad808a138fff4c850f09acb0963bc2e3b4ad3bc1..a432019edf5e1656574c64cbc71a73ce560a32d4 100644 (file)
@@ -476,7 +476,7 @@ public class Vala.GObjectModule : GTypeModule {
                                /* always separate error parameter and inner_error local variable
                                 * as error may be set to NULL but we're always interested in inner errors
                                 */
-                               ccode.add_declaration ("GError *", new CCodeVariableDeclarator.zero ("_inner_error_", new CCodeConstant ("NULL")));
+                               ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero ("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
                        }
 
                        ccode.add_return (new CCodeIdentifier ("obj"));
@@ -502,7 +502,7 @@ public class Vala.GObjectModule : GTypeModule {
                                /* always separate error parameter and inner_error local variable
                                 * as error may be set to NULL but we're always interested in inner errors
                                 */
-                               ccode.add_declaration ("GError *", new CCodeVariableDeclarator.zero ("_inner_error_", new CCodeConstant ("NULL")));
+                               ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero ("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
                        }
 
                        pop_context ();
@@ -524,7 +524,7 @@ public class Vala.GObjectModule : GTypeModule {
                                /* always separate error parameter and inner_error local variable
                                 * as error may be set to NULL but we're always interested in inner errors
                                 */
-                               ccode.add_declaration ("GError *", new CCodeVariableDeclarator.zero ("_inner_error_", new CCodeConstant ("NULL")));
+                               ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero ("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
                        }
 
                        pop_context ();
index a654c0197de820d6381b29793112c994a0b9e715..f74b5fabf8ff73a7e592f7db2fbd189f1b1fc344 100644 (file)
@@ -1802,7 +1802,7 @@ public class Vala.GTypeModule : GErrorModule {
                        cl.destructor.body.emit (this);
 
                        if (current_method_inner_error) {
-                               ccode.add_declaration ("GError *", new CCodeVariableDeclarator.zero ("_inner_error_", new CCodeConstant ("NULL")));
+                               ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero ("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
                        }
 
                        if (current_method_return) {
index 144a095da65c0499e487c52d5db5518f9b866947..fe5aa20ae62bdbe784e90c40d25acf68ad2dc4e8 100644 (file)
@@ -348,6 +348,7 @@ TESTS = \
        objects/bug795225-4.test \
        objects/bug795521.vala \
        errors/catch-error-code.vala \
+       errors/catch-in-finally.vala \
        errors/errors.vala \
        errors/errordomain.vala \
        errors/invalid-type-check.test \
@@ -395,6 +396,7 @@ TESTS = \
        asynchronous/bug792660.vala \
        asynchronous/bug792942.vala \
        asynchronous/bug793158.vala \
+       asynchronous/catch-in-finally.vala \
        asynchronous/closures.vala \
        asynchronous/generator.vala \
        asynchronous/result-pos.vala \
diff --git a/tests/asynchronous/catch-in-finally.vala b/tests/asynchronous/catch-in-finally.vala
new file mode 100644 (file)
index 0000000..9f0f952
--- /dev/null
@@ -0,0 +1,72 @@
+errordomain FooError {
+       FAIL
+}
+
+async void fail () throws FooError {
+       throw new FooError.FAIL ("fail");
+}
+
+async void may_fail () throws FooError {
+}
+
+async void foo () throws FooError {
+       try {
+               yield fail ();
+       } finally {
+               try {
+                       yield may_fail ();
+               } catch (FooError e) {
+                       assert_not_reached ();
+               }
+       }
+       assert_not_reached ();
+}
+
+async void bar () throws FooError {
+       try {
+               yield may_fail ();
+       } finally {
+               try {
+                       yield fail ();
+               } catch (FooError e) {
+               }
+       }
+
+       try {
+               yield fail ();
+       } finally {
+               try {
+                       yield may_fail ();
+               } catch (FooError e) {
+                       assert_not_reached ();
+               }
+       }
+       assert_not_reached ();
+}
+
+async void run () {
+       foo.begin ((o,r) => {
+               try {
+                       foo.end (r);
+               } catch (FooError e) {
+                       assert (e.message == "fail");
+               }
+       });
+
+       bar.begin ((o,r) => {
+               try {
+                       bar.end (r);
+               } catch (FooError e) {
+                       assert (e.message == "fail");
+                       loop.quit ();
+               }
+       });
+}
+
+MainLoop loop;
+
+void main () {
+       loop = new MainLoop ();
+       run.begin ();
+       loop.run ();
+}
diff --git a/tests/errors/catch-in-finally.vala b/tests/errors/catch-in-finally.vala
new file mode 100644 (file)
index 0000000..8845e4b
--- /dev/null
@@ -0,0 +1,59 @@
+errordomain FooError {
+       FAIL
+}
+
+void fail () throws FooError {
+       throw new FooError.FAIL ("fail");
+}
+
+void may_fail () throws FooError {
+}
+
+void foo () throws FooError {
+       try {
+               fail ();
+       } finally {
+               try {
+                       may_fail ();
+               } catch (FooError e) {
+                       assert_not_reached ();
+               }
+       }
+       assert_not_reached ();
+}
+
+void bar () throws FooError {
+       try {
+               may_fail ();
+       } finally {
+               try {
+                       fail ();
+               } catch (FooError e) {
+               }
+       }
+
+       try {
+               fail ();
+       } finally {
+               try {
+                       may_fail ();
+               } catch (FooError e) {
+                       assert_not_reached ();
+               }
+       }
+       assert_not_reached ();
+}
+
+void main () {
+       try {
+               foo ();
+       } catch (FooError e) {
+               assert (e.message == "fail");
+       }
+
+       try {
+               bar ();
+       } catch (FooError e) {
+               assert (e.message == "fail");
+       }
+}