To: vim_dev@googlegroups.com Subject: Patch 8.2.2208 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.2208 Problem: Vim9: after reloading a script variable index may be invalid. Solution: When the sequence number doesn't match give an error for using a script-local variable from a compiled function. (closes #7547) Files: src/vim9.h, src/structs.h, src/errors.h, src/vim9compile.c, src/vim9execute.c, src/scriptfile.c, src/testdir/test_vim9_script.vim *** ../vim-8.2.2207/src/vim9.h 2020-12-22 17:35:50.043978116 +0100 --- src/vim9.h 2020-12-24 21:22:54.689838136 +0100 *************** *** 244,251 **** // arguments to ISN_LOADSCRIPT and ISN_STORESCRIPT typedef struct { ! int script_sid; // script ID ! int script_idx; // index in sn_var_vals } script_T; // arguments to ISN_UNLET --- 244,256 ---- // arguments to ISN_LOADSCRIPT and ISN_STORESCRIPT typedef struct { ! int sref_sid; // script ID ! int sref_idx; // index in sn_var_vals ! int sref_seq; // sn_script_seq when compiled ! } scriptref_T; ! ! typedef struct { ! scriptref_T *scriptref; } script_T; // arguments to ISN_UNLET *************** *** 345,350 **** --- 350,357 ---- int df_idx; // index in def_functions int df_deleted; // if TRUE function was deleted char_u *df_name; // name used for error messages + int df_script_seq; // Value of sctx_T sc_seq when the function + // was compiled. garray_T df_def_args_isn; // default argument instructions isn_T *df_instr; // function body to be executed *** ../vim-8.2.2207/src/structs.h 2020-12-22 17:35:50.043978116 +0100 --- src/structs.h 2020-12-24 19:48:13.649694262 +0100 *************** *** 1790,1802 **** } imported_T; /* ! * Growarray to store info about already sourced scripts. ! * For Unix also store the dev/ino, so that we don't have to stat() each ! * script when going through the list. */ typedef struct { char_u *sn_name; // "sn_vars" stores the s: variables currently valid. When leaving a block // variables local to that block are removed. --- 1790,1801 ---- } imported_T; /* ! * Info about an already sourced scripts. */ typedef struct { char_u *sn_name; + int sn_script_seq; // latest sctx_T sc_seq value // "sn_vars" stores the s: variables currently valid. When leaving a block // variables local to that block are removed. *** ../vim-8.2.2207/src/errors.h 2020-12-21 17:30:46.941668485 +0100 --- src/errors.h 2020-12-24 20:35:11.085382660 +0100 *************** *** 327,329 **** --- 327,331 ---- INIT(= N_("E1147: List not set")); EXTERN char e_cannot_index_str[] INIT(= N_("E1148: Cannot index a %s")); + EXTERN char e_script_variable_invalid_after_reload_in_function_str[] + INIT(= N_("E1149: Script variable is invalid after reload in function %s")); *** ../vim-8.2.2207/src/vim9compile.c 2020-12-24 15:13:35.850860411 +0100 --- src/vim9compile.c 2020-12-24 21:20:21.190527412 +0100 *************** *** 1316,1321 **** --- 1316,1323 ---- type_T *type) { isn_T *isn; + scriptref_T *sref; + scriptitem_T *si = SCRIPT_ITEM(sid); RETURN_OK_IF_SKIP(cctx); if (isn_type == ISN_LOADSCRIPT) *************** *** 1324,1331 **** isn = generate_instr_drop(cctx, isn_type, 1); if (isn == NULL) return FAIL; ! isn->isn_arg.script.script_sid = sid; ! isn->isn_arg.script.script_idx = idx; return OK; } --- 1326,1341 ---- isn = generate_instr_drop(cctx, isn_type, 1); if (isn == NULL) return FAIL; ! ! // This requires three arguments, which doesn't fit in an instruction, thus ! // we need to allocate a struct for this. ! sref = ALLOC_ONE(scriptref_T); ! if (sref == NULL) ! return FAIL; ! isn->isn_arg.script.scriptref = sref; ! sref->sref_sid = sid; ! sref->sref_idx = idx; ! sref->sref_seq = si->sn_script_seq; return OK; } *************** *** 7970,7975 **** --- 7980,7986 ---- dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + ufunc->uf_dfunc_idx; dfunc->df_deleted = FALSE; + dfunc->df_script_seq = current_sctx.sc_seq; dfunc->df_instr = instr->ga_data; dfunc->df_instr_count = instr->ga_len; dfunc->df_varcount = cctx.ctx_locals_count; *************** *** 8186,8191 **** --- 8197,8207 ---- vim_free(isn->isn_arg.cmdmod.cf_cmdmod); break; + case ISN_LOADSCRIPT: + case ISN_STORESCRIPT: + vim_free(isn->isn_arg.script.scriptref); + break; + case ISN_2BOOL: case ISN_2STRING: case ISN_2STRING_ANY: *************** *** 8229,8235 **** case ISN_LOADGDICT: case ISN_LOADOUTER: case ISN_LOADREG: - case ISN_LOADSCRIPT: case ISN_LOADTDICT: case ISN_LOADV: case ISN_LOADWDICT: --- 8245,8250 ---- *************** *** 8256,8262 **** case ISN_STORENR: case ISN_STOREOUTER: case ISN_STOREREG: - case ISN_STORESCRIPT: case ISN_STOREV: case ISN_STRINDEX: case ISN_STRSLICE: --- 8271,8276 ---- *** ../vim-8.2.2207/src/vim9execute.c 2020-12-22 20:35:37.469034276 +0100 --- src/vim9execute.c 2020-12-24 21:26:28.664932472 +0100 *************** *** 1402,1413 **** // load s: variable in Vim9 script case ISN_LOADSCRIPT: { ! scriptitem_T *si = ! SCRIPT_ITEM(iptr->isn_arg.script.script_sid); svar_T *sv; ! sv = ((svar_T *)si->sn_var_vals.ga_data) ! + iptr->isn_arg.script.script_idx; allocate_if_null(sv->sv_tv); if (GA_GROW(&ectx.ec_stack, 1) == FAIL) goto failed; --- 1402,1422 ---- // load s: variable in Vim9 script case ISN_LOADSCRIPT: { ! scriptref_T *sref = iptr->isn_arg.script.scriptref; ! dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) ! + ectx.ec_dfunc_idx; ! scriptitem_T *si = SCRIPT_ITEM(sref->sref_sid); svar_T *sv; ! if (sref->sref_seq != si->sn_script_seq) ! { ! // The script was reloaded after the function was ! // compiled, the script_idx may not be valid. ! semsg(_(e_script_variable_invalid_after_reload_in_function_str), ! dfunc->df_ufunc->uf_name_exp); ! goto failed; ! } ! sv = ((svar_T *)si->sn_var_vals.ga_data) + sref->sref_idx; allocate_if_null(sv->sv_tv); if (GA_GROW(&ectx.ec_stack, 1) == FAIL) goto failed; *************** *** 1616,1626 **** // store script-local variable in Vim9 script case ISN_STORESCRIPT: { ! scriptitem_T *si = SCRIPT_ITEM( ! iptr->isn_arg.script.script_sid); ! svar_T *sv = ((svar_T *)si->sn_var_vals.ga_data) ! + iptr->isn_arg.script.script_idx; --ectx.ec_stack.ga_len; clear_tv(sv->sv_tv); *sv->sv_tv = *STACK_TV_BOT(0); --- 1625,1646 ---- // store script-local variable in Vim9 script case ISN_STORESCRIPT: { ! scriptref_T *sref = iptr->isn_arg.script.scriptref; ! dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) ! + ectx.ec_dfunc_idx; ! scriptitem_T *si = SCRIPT_ITEM(sref->sref_sid); ! svar_T *sv; + if (sref->sref_seq != si->sn_script_seq) + { + // The script was reloaded after the function was + // compiled, the script_idx may not be valid. + SOURCING_LNUM = iptr->isn_lnum; + semsg(_(e_script_variable_invalid_after_reload_in_function_str), + dfunc->df_ufunc->uf_name_exp); + goto failed; + } + sv = ((svar_T *)si->sn_var_vals.ga_data) + sref->sref_idx; --ectx.ec_stack.ga_len; clear_tv(sv->sv_tv); *sv->sv_tv = *STACK_TV_BOT(0); *************** *** 3378,3391 **** break; case ISN_LOADSCRIPT: { ! scriptitem_T *si = ! SCRIPT_ITEM(iptr->isn_arg.script.script_sid); svar_T *sv = ((svar_T *)si->sn_var_vals.ga_data) ! + iptr->isn_arg.script.script_idx; smsg("%4d LOADSCRIPT %s-%d from %s", current, sv->sv_name, ! iptr->isn_arg.script.script_idx, si->sn_name); } break; --- 3398,3411 ---- break; case ISN_LOADSCRIPT: { ! scriptref_T *sref = iptr->isn_arg.script.scriptref; ! scriptitem_T *si = SCRIPT_ITEM(sref->sref_sid); svar_T *sv = ((svar_T *)si->sn_var_vals.ga_data) ! + sref->sref_idx; smsg("%4d LOADSCRIPT %s-%d from %s", current, sv->sv_name, ! sref->sref_idx, si->sn_name); } break; *************** *** 3478,3491 **** break; case ISN_STORESCRIPT: { ! scriptitem_T *si = ! SCRIPT_ITEM(iptr->isn_arg.script.script_sid); svar_T *sv = ((svar_T *)si->sn_var_vals.ga_data) ! + iptr->isn_arg.script.script_idx; smsg("%4d STORESCRIPT %s-%d in %s", current, sv->sv_name, ! iptr->isn_arg.script.script_idx, si->sn_name); } break; --- 3498,3511 ---- break; case ISN_STORESCRIPT: { ! scriptref_T *sref = iptr->isn_arg.script.scriptref; ! scriptitem_T *si = SCRIPT_ITEM(sref->sref_sid); svar_T *sv = ((svar_T *)si->sn_var_vals.ga_data) ! + sref->sref_idx; smsg("%4d STORESCRIPT %s-%d in %s", current, sv->sv_name, ! sref->sref_idx, si->sn_name); } break; *** ../vim-8.2.2207/src/scriptfile.c 2020-12-10 20:21:20.274198438 +0100 --- src/scriptfile.c 2020-12-24 19:51:46.237066252 +0100 *************** *** 1391,1396 **** --- 1391,1397 ---- if (ret_sid != NULL) *ret_sid = current_sctx.sc_sid; } + si->sn_script_seq = current_sctx.sc_seq; # ifdef FEAT_PROFILE if (do_profiling == PROF_YES) *** ../vim-8.2.2207/src/testdir/test_vim9_script.vim 2020-12-21 18:11:20.837680928 +0100 --- src/testdir/test_vim9_script.vim 2020-12-24 21:51:23.640213386 +0100 *************** *** 3126,3131 **** --- 3126,3192 ---- CheckDefAndScriptFailure(lines, 'E1144:', 1) enddef + def Test_script_var_gone_when_sourced_twice() + var lines =<< trim END + vim9script + if exists('g:guard') + finish + endif + g:guard = 1 + var name = 'thename' + def g:GetName(): string + return name + enddef + def g:SetName(arg: string) + name = arg + enddef + END + writefile(lines, 'XscriptTwice.vim') + so XscriptTwice.vim + assert_equal('thename', g:GetName()) + g:SetName('newname') + assert_equal('newname', g:GetName()) + so XscriptTwice.vim + assert_fails('call g:GetName()', 'E1149:') + assert_fails('call g:SetName("x")', 'E1149:') + + delfunc g:GetName + delfunc g:SetName + delete('XscriptTwice.vim') + unlet g:guard + enddef + + def Test_import_gone_when_sourced_twice() + var exportlines =<< trim END + vim9script + if exists('g:guard') + finish + endif + g:guard = 1 + export var name = 'someName' + END + writefile(exportlines, 'XexportScript.vim') + + var lines =<< trim END + vim9script + import name from './XexportScript.vim' + def g:GetName(): string + return name + enddef + END + writefile(lines, 'XscriptImport.vim') + so XscriptImport.vim + assert_equal('someName', g:GetName()) + + so XexportScript.vim + assert_fails('call g:GetName()', 'E1149:') + + delfunc g:GetName + delete('XexportScript.vim') + delete('XscriptImport.vim') + unlet g:guard + enddef + " Keep this last, it messes up highlighting. def Test_substitute_cmd() new *** ../vim-8.2.2207/src/version.c 2020-12-24 18:38:37.181858419 +0100 --- src/version.c 2020-12-24 19:48:38.257621559 +0100 *************** *** 752,753 **** --- 752,755 ---- { /* Add new patch number below this line */ + /**/ + 2208, /**/ -- For a moment, nothing happened. Then, after a second or so, nothing continued to happen. -- Douglas Adams, "The Hitchhiker's Guide to the Galaxy" /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///