# HG changeset patch # User Bram Moolenaar # Date 1613940303 -3600 # Node ID 3daeb2060f253aea150236c9ced18663d25dae4c # Parent fa875fec0ed568436fd03809d2c4a7d7f5179f84 patch 8.2.2539: Vim9: return from finally block causes a hang Commit: https://github.com/vim/vim/commit/7e82c5f338efe5661951675565f27f6512901a6e Author: Bram Moolenaar Date: Sun Feb 21 21:32:45 2021 +0100 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 https://github.com/vim/vim/issues/7885) diff --git a/src/testdir/test_vim9_disassemble.vim b/src/testdir/test_vim9_disassemble.vim --- 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('\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 --- 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 --- a/src/version.c +++ b/src/version.c @@ -751,6 +751,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 2539, +/**/ 2538, /**/ 2537, diff --git a/src/vim9.h b/src/vim9.h --- 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 --- 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 // 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 *cct // 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 *cct 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 *cct 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 --- 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;