# HG changeset patch # User Bram Moolenaar # Date 1609010104 -3600 # Node ID f181fe2150ab74304235051135847a3d287c7f83 # Parent 5c7e91d7fc6d40ea1c3e9b9e07a380ae42bced72 patch 8.2.2224: Vim9: crash if script reloaded with different variable type Commit: https://github.com/vim/vim/commit/07a65d26e7d76ad22d6ef23b50c0faa25e435e02 Author: Bram Moolenaar Date: Sat Dec 26 20:09:15 2020 +0100 patch 8.2.2224: Vim9: crash if script reloaded with different variable type Problem: Vim9: crash if script reloaded with different variable type. Solution: Check the type when accessing the variable. diff --git a/src/errors.h b/src/errors.h --- a/src/errors.h +++ b/src/errors.h @@ -329,3 +329,5 @@ 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")); +EXTERN char e_script_variable_type_changed[] + INIT(= N_("E1150: Script variable type changed")); diff --git a/src/evalvars.c b/src/evalvars.c --- a/src/evalvars.c +++ b/src/evalvars.c @@ -784,7 +784,7 @@ ex_let(exarg_T *eap) { if (vim9script) { - // Vim9 declaration ":let var: type" + // Vim9 declaration ":var name: type" arg = vim9_declare_scriptvar(eap, arg); } else @@ -3133,9 +3133,16 @@ set_var_const( goto failed; } else + { // can only redefine once di->di_flags &= ~DI_FLAGS_RELOAD; + // A Vim9 script-local variable is also present in sn_all_vars and + // sn_var_vals. + if (is_script_local && vim9script) + update_vim9_script_var(FALSE, di, tv, type); + } + // existing variable, need to clear the value // Handle setting internal di: variables separately where needed to @@ -3216,7 +3223,7 @@ set_var_const( // A Vim9 script-local variable is also added to sn_all_vars and // sn_var_vals. if (is_script_local && vim9script) - add_vim9_script_var(di, tv, type); + update_vim9_script_var(TRUE, di, tv, type); } if (copy || tv->v_type == VAR_NUMBER || tv->v_type == VAR_FLOAT) diff --git a/src/proto/vim9script.pro b/src/proto/vim9script.pro --- a/src/proto/vim9script.pro +++ b/src/proto/vim9script.pro @@ -8,7 +8,7 @@ void ex_import(exarg_T *eap); int find_exported(int sid, char_u *name, ufunc_T **ufunc, type_T **type, cctx_T *cctx); char_u *handle_import(char_u *arg_start, garray_T *gap, int import_sid, evalarg_T *evalarg, void *cctx); char_u *vim9_declare_scriptvar(exarg_T *eap, char_u *arg); -void add_vim9_script_var(dictitem_T *di, typval_T *tv, type_T *type); +void update_vim9_script_var(int create, dictitem_T *di, typval_T *tv, type_T *type); void hide_script_var(scriptitem_T *si, int idx, int func_defined); void free_all_script_vars(scriptitem_T *si); svar_T *find_typval_in_script(typval_T *dest); diff --git a/src/proto/vim9type.pro b/src/proto/vim9type.pro --- a/src/proto/vim9type.pro +++ b/src/proto/vim9type.pro @@ -18,6 +18,7 @@ int check_type(type_T *expected, type_T int check_arg_type(type_T *expected, type_T *actual, int argidx); char_u *skip_type(char_u *start, int optional); type_T *parse_type(char_u **arg, garray_T *type_gap, int give_error); +int equal_type(type_T *type1, type_T *type2); void common_type(type_T *type1, type_T *type2, type_T **dest, garray_T *type_gap); type_T *get_member_type_from_stack(type_T **stack_top, int count, int skip, garray_T *type_gap); char *vartype_name(vartype_T type); 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 @@ -1247,6 +1247,32 @@ def Test_vim9script_reload_import() delete('Ximport.vim') enddef +" if a script is reloaded with a script-local variable that changed its type, a +" compiled function using that variable must fail. +def Test_script_reload_change_type() + var lines =<< trim END + vim9script noclear + var str = 'string' + def g:GetStr(): string + return str .. 'xxx' + enddef + END + writefile(lines, 'Xreload.vim') + source Xreload.vim + echo g:GetStr() + + lines =<< trim END + vim9script noclear + var str = 1234 + END + writefile(lines, 'Xreload.vim') + source Xreload.vim + assert_fails('echo g:GetStr()', 'E1150:') + + delfunc g:GetStr + delete('Xreload.vim') +enddef + def s:RetSome(): string return 'some' enddef 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 */ /**/ + 2224, +/**/ 2223, /**/ 2222, diff --git a/src/vim9.h b/src/vim9.h --- a/src/vim9.h +++ b/src/vim9.h @@ -247,6 +247,7 @@ typedef struct { int sref_sid; // script ID int sref_idx; // index in sn_var_vals int sref_seq; // sn_script_seq when compiled + type_T *sref_type; // type of the variable when compiled } scriptref_T; typedef struct { diff --git a/src/vim9compile.c b/src/vim9compile.c --- a/src/vim9compile.c +++ b/src/vim9compile.c @@ -1327,6 +1327,7 @@ generate_VIM9SCRIPT( sref->sref_sid = sid; sref->sref_idx = idx; sref->sref_seq = si->sn_script_seq; + sref->sref_type = type; return OK; } diff --git a/src/vim9execute.c b/src/vim9execute.c --- a/src/vim9execute.c +++ b/src/vim9execute.c @@ -863,6 +863,31 @@ allocate_if_null(typval_T *tv) } } + static svar_T * +get_script_svar(scriptref_T *sref, ectx_T *ectx) +{ + scriptitem_T *si = SCRIPT_ITEM(sref->sref_sid); + dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + + ectx->ec_dfunc_idx; + 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); + return NULL; + } + sv = ((svar_T *)si->sn_var_vals.ga_data) + sref->sref_idx; + if (!equal_type(sv->sv_type, sref->sref_type)) + { + emsg(_(e_script_variable_type_changed)); + return NULL; + } + return sv; +} + /* * Execute a function by "name". * This can be a builtin function, user function or a funcref. @@ -1406,20 +1431,11 @@ call_def_function( 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); + sv = get_script_svar(sref, &ectx); + if (sv == NULL) 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; @@ -1628,22 +1644,12 @@ call_def_function( // 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); + scriptref_T *sref = iptr->isn_arg.script.scriptref; + svar_T *sv; + + sv = get_script_svar(sref, &ectx); + if (sv == NULL) 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); diff --git a/src/vim9script.c b/src/vim9script.c --- a/src/vim9script.c +++ b/src/vim9script.c @@ -591,36 +591,37 @@ vim9_declare_scriptvar(exarg_T *eap, cha /* * Vim9 part of adding a script variable: add it to sn_all_vars (lookup by name * with a hashtable) and sn_var_vals (lookup by index). + * When "create" is TRUE this is a new variable, otherwise find and update an + * existing variable. * When "type" is NULL use "tv" for the type. */ void -add_vim9_script_var(dictitem_T *di, typval_T *tv, type_T *type) +update_vim9_script_var(int create, dictitem_T *di, typval_T *tv, type_T *type) { - scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid); + scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid); + hashitem_T *hi; + svar_T *sv; - // Store a pointer to the typval_T, so that it can be found by - // index instead of using a hastab lookup. - if (ga_grow(&si->sn_var_vals, 1) == OK) + if (create) { - svar_T *sv = ((svar_T *)si->sn_var_vals.ga_data) - + si->sn_var_vals.ga_len; - hashitem_T *hi; - sallvar_T *newsav = (sallvar_T *)alloc_clear( + sallvar_T *newsav; + + // Store a pointer to the typval_T, so that it can be found by index + // instead of using a hastab lookup. + if (ga_grow(&si->sn_var_vals, 1) == FAIL) + return; + + sv = ((svar_T *)si->sn_var_vals.ga_data) + si->sn_var_vals.ga_len; + newsav = (sallvar_T *)alloc_clear( sizeof(sallvar_T) + STRLEN(di->di_key)); - if (newsav == NULL) return; sv->sv_tv = &di->di_tv; - if (type == NULL) - sv->sv_type = typval2type(tv, &si->sn_type_list); - else - sv->sv_type = type; sv->sv_const = (di->di_flags & DI_FLAGS_LOCK) ? ASSIGN_CONST : 0; sv->sv_export = is_export; newsav->sav_var_vals_idx = si->sn_var_vals.ga_len; ++si->sn_var_vals.ga_len; - STRCPY(&newsav->sav_key, di->di_key); sv->sv_name = newsav->sav_key; newsav->sav_di = di; @@ -639,10 +640,21 @@ add_vim9_script_var(dictitem_T *di, typv else // new variable name hash_add(&si->sn_all_vars.dv_hashtab, newsav->sav_key); + } + else + { + sv = find_typval_in_script(&di->di_tv); + } + if (sv != NULL) + { + if (type == NULL) + sv->sv_type = typval2type(tv, &si->sn_type_list); + else + sv->sv_type = type; + } - // let ex_export() know the export worked. - is_export = FALSE; - } + // let ex_export() know the export worked. + is_export = FALSE; } /* diff --git a/src/vim9type.c b/src/vim9type.c --- a/src/vim9type.c +++ b/src/vim9type.c @@ -853,7 +853,7 @@ parse_type(char_u **arg, garray_T *type_ /* * Check if "type1" and "type2" are exactly the same. */ - static int + int equal_type(type_T *type1, type_T *type2) { int i;