# HG changeset patch # User Bram Moolenaar # Date 1608843604 -3600 # Node ID e8eb4fd44902b6279b20f7d55b95a25b2328f8a7 # Parent 2d0d83f002e8818d4bb6b81812e90c62c66424ea patch 8.2.2208: Vim9: after reloading a script variable index may be invalid Commit: https://github.com/vim/vim/commit/4aab88d919168ce2ddf4845482f4cff9efa52b5b Author: Bram Moolenaar Date: Thu Dec 24 21:56:41 2020 +0100 patch 8.2.2208: Vim9: after reloading a script variable index may be invalid 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) diff --git a/src/errors.h b/src/errors.h --- a/src/errors.h +++ b/src/errors.h @@ -327,3 +327,5 @@ EXTERN char e_list_not_set[] 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")); diff --git a/src/scriptfile.c b/src/scriptfile.c --- a/src/scriptfile.c +++ b/src/scriptfile.c @@ -1391,6 +1391,7 @@ do_source( 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) diff --git a/src/structs.h b/src/structs.h --- a/src/structs.h +++ b/src/structs.h @@ -1790,13 +1790,12 @@ typedef struct { } 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. + * 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. 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 @@ -3126,6 +3126,67 @@ def Test_white_space_after_command() 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 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 */ /**/ + 2208, +/**/ 2207, /**/ 2206, diff --git a/src/vim9.h b/src/vim9.h --- a/src/vim9.h +++ b/src/vim9.h @@ -244,8 +244,13 @@ typedef struct { // arguments to ISN_LOADSCRIPT and ISN_STORESCRIPT typedef struct { - int script_sid; // script ID - int script_idx; // index in sn_var_vals + 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,6 +350,8 @@ struct dfunc_S { 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 diff --git a/src/vim9compile.c b/src/vim9compile.c --- a/src/vim9compile.c +++ b/src/vim9compile.c @@ -1316,6 +1316,8 @@ generate_VIM9SCRIPT( 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,8 +1326,16 @@ generate_VIM9SCRIPT( 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; + + // 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,6 +7980,7 @@ nextline: 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,6 +8197,11 @@ delete_instr(isn_T *isn) 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,7 +8245,6 @@ delete_instr(isn_T *isn) case ISN_LOADGDICT: case ISN_LOADOUTER: case ISN_LOADREG: - case ISN_LOADSCRIPT: case ISN_LOADTDICT: case ISN_LOADV: case ISN_LOADWDICT: @@ -8256,7 +8271,6 @@ delete_instr(isn_T *isn) case ISN_STORENR: case ISN_STOREOUTER: case ISN_STOREREG: - case ISN_STORESCRIPT: case ISN_STOREV: case ISN_STRINDEX: case ISN_STRSLICE: diff --git a/src/vim9execute.c b/src/vim9execute.c --- a/src/vim9execute.c +++ b/src/vim9execute.c @@ -1402,12 +1402,21 @@ call_def_function( // load s: variable in Vim9 script case ISN_LOADSCRIPT: { - scriptitem_T *si = - SCRIPT_ITEM(iptr->isn_arg.script.script_sid); + 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; - sv = ((svar_T *)si->sn_var_vals.ga_data) - + iptr->isn_arg.script.script_idx; + 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,11 +1625,22 @@ call_def_function( // 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; - + 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,14 +3398,14 @@ ex_disassemble(exarg_T *eap) break; case ISN_LOADSCRIPT: { - scriptitem_T *si = - SCRIPT_ITEM(iptr->isn_arg.script.script_sid); + 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) - + iptr->isn_arg.script.script_idx; + + sref->sref_idx; smsg("%4d LOADSCRIPT %s-%d from %s", current, sv->sv_name, - iptr->isn_arg.script.script_idx, + sref->sref_idx, si->sn_name); } break; @@ -3478,14 +3498,14 @@ ex_disassemble(exarg_T *eap) break; case ISN_STORESCRIPT: { - scriptitem_T *si = - SCRIPT_ITEM(iptr->isn_arg.script.script_sid); + 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) - + iptr->isn_arg.script.script_idx; + + sref->sref_idx; smsg("%4d STORESCRIPT %s-%d in %s", current, sv->sv_name, - iptr->isn_arg.script.script_idx, + sref->sref_idx, si->sn_name); } break;