patch 8.2.2539: Vim9: return from finally block causes a hang

Problem:    Vim9: return from finally block causes a hang.
Solution:   Store both the finally and endtry indexes. (closes #7885)

diff --git a/src/testdir/test_vim9_disassemble.vim b/src/testdir/test_vim9_disassemble.vim
index 4d7bbf63e9f98f4144e93b27405cdaadec7ef53e..9d05bbfd221f7f1f64d74bfb99b9c5bdae67cb7e 100644 (file)
--- a/src/testdir/test_vim9_disassemble.vim
+++ b/src/testdir/test_vim9_disassemble.vim
@@ -422,7 +422,7 @@ def Test_disassemble_try()
   var res = execute('disass s:ScriptFuncTry')
   assert_match('<SNR>\d*_ScriptFuncTry\_s*' ..
         'try\_s*' ..
-        '\d TRY catch -> \d\+, finally -> \d\+\_s*' ..
+        '\d TRY catch -> \d\+, finally -> \d\+, endtry -> \d\+\_s*' ..
         'echo "yes"\_s*' ..
         '\d PUSHS "yes"\_s*' ..
         '\d ECHO 1\_s*' ..
@@ -437,6 +437,7 @@ def Test_disassemble_try()
         '\d\+ PUSHS "no"\_s*' ..
         '\d\+ ECHO 1\_s*' ..
         'finally\_s*' ..
+        '\d\+ FINALLY\_s*' ..
         'throw "end"\_s*' ..
         '\d\+ PUSHS "end"\_s*' ..
         '\d\+ THROW\_s*' ..
@@ -1137,12 +1138,12 @@ def Test_disassemble_for_loop_continue()
         '4 FOR $0 -> 22\_s*' ..
         '5 STORE $1\_s*' ..
         'try\_s*' ..
-        '6 TRY catch -> 17, end -> 20\_s*' ..
+        '6 TRY catch -> 17, endtry -> 20\_s*' ..
         'echo "ok"\_s*' ..
         '7 PUSHS "ok"\_s*' ..
         '8 ECHO 1\_s*' ..
         'try\_s*' ..
-        '9 TRY catch -> 13, end -> 15\_s*' ..
+        '9 TRY catch -> 13, endtry -> 15\_s*' ..
         'echo "deeper"\_s*' ..
         '10 PUSHS "deeper"\_s*' ..
         '11 ECHO 1\_s*' ..

diff --git a/src/testdir/test_vim9_script.vim b/src/testdir/test_vim9_script.vim
index eb46d35656683997adf4af68c3e63b07700f6e18..0fb09fd817a7e6f5838d0b495b25e7cbfb7b39a2 100644 (file)
--- a/src/testdir/test_vim9_script.vim
+++ b/src/testdir/test_vim9_script.vim
@@ -577,6 +577,16 @@ def Test_try_catch_throw()
     counter += 1
   endfor
   assert_equal(4, counter)
+
+  # return in finally after empty catch
+  def ReturnInFinally(): number
+    try
+    finally
+      return 4
+    endtry
+    return 2
+  enddef
+  assert_equal(4, ReturnInFinally())
 enddef
 
 def Test_cnext_works_in_catch()

diff --git a/src/version.c b/src/version.c
index a64037d4ca747d46f7f2d2391a1adbbb39487ba1..b096340cfa5bea7d251394912f666d59cca18895 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -750,6 +750,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2539,
 /**/
     2538,
 /**/

diff --git a/src/vim9.h b/src/vim9.h
index ca8e64d175dbeb8ce6e046679e864af75a0b1541..66a21b77a7991c86aea0fcd70372bea45914a882 100644 (file)
--- a/src/vim9.h
+++ b/src/vim9.h
@@ -100,6 +100,7 @@ typedef enum {
     ISN_THROW,     // pop value of stack, store in v:exception
     ISN_PUSHEXC,    // push v:exception
     ISN_CATCH,     // drop v:exception
+    ISN_FINALLY,    // start of :finally block
     ISN_ENDTRY,            // take entry off from ec_trystack
     ISN_TRYCONT,    // handle :continue inside a :try statement
 
@@ -208,10 +209,16 @@ typedef struct {
     int            for_end;        // position to jump to after done
 } forloop_T;
 
-// arguments to ISN_TRY
+// indirect arguments to ISN_TRY
 typedef struct {
     int            try_catch;      // position to jump to on throw
     int            try_finally;    // :finally or :endtry position to jump to
+    int            try_endtry;     // :endtry position to jump to
+} tryref_T;
+
+// arguments to ISN_TRY
+typedef struct {
+    tryref_T *try_ref;
 } try_T;
 
 // arguments to ISN_TRYCONT

diff --git a/src/vim9compile.c b/src/vim9compile.c
index 648f4e031cfde52d481b60e5361fb132e35a73a2..c9591ad7b7434fa79b27436424e105f23471b0ac 100644 (file)
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -7518,10 +7518,17 @@ compile_try(char_u *arg, cctx_T *cctx)
 
     if (cctx->ctx_skip != SKIP_YES)
     {
-       // "catch" is set when the first ":catch" is found.
-       // "finally" is set when ":finally" or ":endtry" is found
+       isn_T   *isn;
+
+       // "try_catch" is set when the first ":catch" is found or when no catch
+       // is found and ":finally" is found.
+       // "try_finally" is set when ":finally" is found
+       // "try_endtry" is set when ":endtry" is found
        try_scope->se_u.se_try.ts_try_label = instr->ga_len;
-       if (generate_instr(cctx, ISN_TRY) == NULL)
+       if ((isn = generate_instr(cctx, ISN_TRY)) == NULL)
+           return NULL;
+       isn->isn_arg.try.try_ref = ALLOC_CLEAR_ONE(tryref_T);
+       if (isn->isn_arg.try.try_ref == NULL)
            return NULL;
     }
 
@@ -7577,8 +7584,8 @@ compile_catch(char_u *arg, cctx_T *cctx UNUSED)
 
        // End :try or :catch scope: set value in ISN_TRY instruction
        isn = ((isn_T *)instr->ga_data) + scope->se_u.se_try.ts_try_label;
-       if (isn->isn_arg.try.try_catch == 0)
-           isn->isn_arg.try.try_catch = instr->ga_len;
+       if (isn->isn_arg.try.try_ref->try_catch == 0)
+           isn->isn_arg.try.try_ref->try_catch = instr->ga_len;
        if (scope->se_u.se_try.ts_catch_label != 0)
        {
            // Previous catch without match jumps here
@@ -7670,7 +7677,7 @@ compile_finally(char_u *arg, cctx_T *cctx)
 
     // End :catch or :finally scope: set value in ISN_TRY instruction
     isn = ((isn_T *)instr->ga_data) + scope->se_u.se_try.ts_try_label;
-    if (isn->isn_arg.try.try_finally != 0)
+    if (isn->isn_arg.try.try_ref->try_finally != 0)
     {
        emsg(_(e_finally_dup));
        return NULL;
@@ -7688,7 +7695,10 @@ compile_finally(char_u *arg, cctx_T *cctx)
     compile_fill_jump_to_end(&scope->se_u.se_try.ts_end_label,
                                                             this_instr, cctx);
 
-    isn->isn_arg.try.try_finally = this_instr;
+    // If there is no :catch then an exception jumps to :finally.
+    if (isn->isn_arg.try.try_ref->try_catch == 0)
+       isn->isn_arg.try.try_ref->try_catch = this_instr;
+    isn->isn_arg.try.try_ref->try_finally = this_instr;
     if (scope->se_u.se_try.ts_catch_label != 0)
     {
        // Previous catch without match jumps here
@@ -7696,6 +7706,8 @@ compile_finally(char_u *arg, cctx_T *cctx)
        isn->isn_arg.jump.jump_where = this_instr;
        scope->se_u.se_try.ts_catch_label = 0;
     }
+    if (generate_instr(cctx, ISN_FINALLY) == NULL)
+       return NULL;
 
     // TODO: set index in ts_finally_label jumps
 
@@ -7731,8 +7743,8 @@ compile_endtry(char_u *arg, cctx_T *cctx)
     try_isn = ((isn_T *)instr->ga_data) + scope->se_u.se_try.ts_try_label;
     if (cctx->ctx_skip != SKIP_YES)
     {
-       if (try_isn->isn_arg.try.try_catch == 0
-                                     && try_isn->isn_arg.try.try_finally == 0)
+       if (try_isn->isn_arg.try.try_ref->try_catch == 0
+                                     && try_isn->isn_arg.try.try_ref->try_finally == 0)
        {
            emsg(_(e_missing_catch_or_finally));
            return NULL;
@@ -7751,12 +7763,6 @@ compile_endtry(char_u *arg, cctx_T *cctx)
        compile_fill_jump_to_end(&scope->se_u.se_try.ts_end_label,
                                                          instr->ga_len, cctx);
 
-       // End :catch or :finally scope: set value in ISN_TRY instruction
-       if (try_isn->isn_arg.try.try_catch == 0)
-           try_isn->isn_arg.try.try_catch = instr->ga_len;
-       if (try_isn->isn_arg.try.try_finally == 0)
-           try_isn->isn_arg.try.try_finally = instr->ga_len;
-
        if (scope->se_u.se_try.ts_catch_label != 0)
        {
            // Last catch without match jumps here
@@ -7770,11 +7776,9 @@ compile_endtry(char_u *arg, cctx_T *cctx)
 
     if (cctx->ctx_skip != SKIP_YES)
     {
-       if (try_isn->isn_arg.try.try_finally == 0)
-           // No :finally encountered, use the try_finaly field to point to
-           // ENDTRY, so that TRYCONT can jump there.
-           try_isn->isn_arg.try.try_finally = instr->ga_len;
-
+       // End :catch or :finally scope: set instruction index in ISN_TRY
+       // instruction
+       try_isn->isn_arg.try.try_ref->try_endtry = instr->ga_len;
        if (cctx->ctx_skip != SKIP_YES
                                   && generate_instr(cctx, ISN_ENDTRY) == NULL)
            return NULL;
@@ -8867,6 +8871,10 @@ delete_instr(isn_T *isn)
            vim_free(isn->isn_arg.script.scriptref);
            break;
 
+       case ISN_TRY:
+           vim_free(isn->isn_arg.try.try_ref);
+           break;
+
        case ISN_2BOOL:
        case ISN_2STRING:
        case ISN_2STRING_ANY:
@@ -8899,6 +8907,7 @@ delete_instr(isn_T *isn)
        case ISN_ENDTRY:
        case ISN_EXECCONCAT:
        case ISN_EXECUTE:
+       case ISN_FINALLY:
        case ISN_FOR:
        case ISN_GETITEM:
        case ISN_JUMP:
@@ -8943,7 +8952,6 @@ delete_instr(isn_T *isn)
        case ISN_STRINDEX:
        case ISN_STRSLICE:
        case ISN_THROW:
-       case ISN_TRY:
        case ISN_TRYCONT:
        case ISN_UNLETINDEX:
        case ISN_UNLETRANGE:

diff --git a/src/vim9execute.c b/src/vim9execute.c
index 6734f4f7daa1227b49f25e66173b7fb1ae68f9c2..d2cc62f9ae267f83c6924f4e18abb4f1f218274b 100644 (file)
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -26,8 +26,9 @@
 typedef struct {
     int            tcd_frame_idx;      // ec_frame_idx at ISN_TRY
     int            tcd_stack_len;      // size of ectx.ec_stack at ISN_TRY
-    int            tcd_catch_idx;      // instruction of the first catch
-    int            tcd_finally_idx;    // instruction of the finally block or :endtry
+    int            tcd_catch_idx;      // instruction of the first :catch or :finally
+    int            tcd_finally_idx;    // instruction of the :finally block or zero
+    int            tcd_endtry_idx;     // instruction of the :endtry
     int            tcd_caught;         // catch block entered
     int            tcd_cont;           // :continue encountered, jump here
     int            tcd_return;         // when TRUE return from end of :finally
@@ -2517,10 +2518,9 @@ call_def_function(
                                                        + trystack->ga_len - 1;
                    if (trycmd != NULL
                                  && trycmd->tcd_frame_idx == ectx.ec_frame_idx
-                                 && ectx.ec_instr[trycmd->tcd_finally_idx]
-                                                      .isn_type != ISN_ENDTRY)
+                                 && trycmd->tcd_finally_idx != 0)
                    {
-                       // jump to ":finally"
+                       // jump to ":finally" once
                        ectx.ec_iidx = trycmd->tcd_finally_idx;
                        trycmd->tcd_return = TRUE;
                    }
@@ -2665,8 +2665,9 @@ call_def_function(
                    CLEAR_POINTER(trycmd);
                    trycmd->tcd_frame_idx = ectx.ec_frame_idx;
                    trycmd->tcd_stack_len = ectx.ec_stack.ga_len;
-                   trycmd->tcd_catch_idx = iptr->isn_arg.try.try_catch;
-                   trycmd->tcd_finally_idx = iptr->isn_arg.try.try_finally;
+                   trycmd->tcd_catch_idx = iptr->isn_arg.try.try_ref->try_catch;
+                   trycmd->tcd_finally_idx = iptr->isn_arg.try.try_ref->try_finally;
+                   trycmd->tcd_endtry_idx = iptr->isn_arg.try.try_ref->try_endtry;
                }
                break;
 
@@ -2731,13 +2732,26 @@ call_def_function(
                        trycmd = ((trycmd_T *)trystack->ga_data)
                                                        + trystack->ga_len - i;
                        trycmd->tcd_cont = iidx;
-                       iidx = trycmd->tcd_finally_idx;
+                       iidx = trycmd->tcd_finally_idx == 0
+                           ? trycmd->tcd_endtry_idx : trycmd->tcd_finally_idx;
                    }
                    // jump to :finally or :endtry of current try statement
                    ectx.ec_iidx = iidx;
                }
                break;
 
+           case ISN_FINALLY:
+               {
+                   garray_T    *trystack = &ectx.ec_trystack;
+                   trycmd_T    *trycmd = ((trycmd_T *)trystack->ga_data)
+                                                       + trystack->ga_len - 1;
+
+                   // Reset the index to avoid a return statement jumps here
+                   // again.
+                   trycmd->tcd_finally_idx = 0;
+                   break;
+               }
+
            // end of ":try" block
            case ISN_ENDTRY:
                {
@@ -4348,11 +4362,17 @@ ex_disassemble(exarg_T *eap)
                {
                    try_T *try = &iptr->isn_arg.try;
 
-                   smsg("%4d TRY catch -> %d, %s -> %d", current,
-                                try->try_catch,
-                                instr[try->try_finally].isn_type == ISN_ENDTRY
-                                                          ? "end" : "finally",
-                                try->try_finally);
+                   if (try->try_ref->try_finally == 0)
+                       smsg("%4d TRY catch -> %d, endtry -> %d",
+                               current,
+                               try->try_ref->try_catch,
+                               try->try_ref->try_endtry);
+                   else
+                       smsg("%4d TRY catch -> %d, finally -> %d, endtry -> %d",
+                               current,
+                               try->try_ref->try_catch,
+                               try->try_ref->try_finally,
+                               try->try_ref->try_endtry);
                }
                break;
            case ISN_CATCH:
@@ -4369,6 +4389,9 @@ ex_disassemble(exarg_T *eap)
                                      trycont->tct_where);
                }
                break;
+           case ISN_FINALLY:
+               smsg("%4d FINALLY", current);
+               break;
            case ISN_ENDTRY:
                smsg("%4d ENDTRY", current);
                break;