To: vim_dev@googlegroups.com Subject: Patch 8.2.2224 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.2224 Problem: Vim9: crash if script reloaded with different variable type. Solution: Check the type when accessing the variable. Files: src/vim9execute.c, src/vim9compile.c, src/vim9.h, src/vim9type.c, src/proto/vim9type.pro, src/errors.h, src/evalvars.c, src/vim9script.c, src/proto/vim9script.pro, src/testdir/test_vim9_script.vim *** ../vim-8.2.2223/src/vim9execute.c 2020-12-25 20:24:48.172245018 +0100 --- src/vim9execute.c 2020-12-26 19:58:40.692216975 +0100 *************** *** 863,868 **** --- 863,893 ---- } } + 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,1425 **** 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; --- 1431,1441 ---- case ISN_LOADSCRIPT: { scriptref_T *sref = iptr->isn_arg.script.scriptref; svar_T *sv; ! sv = get_script_svar(sref, &ectx); ! if (sv == NULL) goto failed; allocate_if_null(sv->sv_tv); if (GA_GROW(&ectx.ec_stack, 1) == FAIL) goto failed; *************** *** 1628,1649 **** // 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); --- 1644,1655 ---- // store script-local variable in Vim9 script case ISN_STORESCRIPT: { ! scriptref_T *sref = iptr->isn_arg.script.scriptref; ! svar_T *sv; ! sv = get_script_svar(sref, &ectx); ! if (sv == NULL) goto failed; --ectx.ec_stack.ga_len; clear_tv(sv->sv_tv); *sv->sv_tv = *STACK_TV_BOT(0); *** ../vim-8.2.2223/src/vim9compile.c 2020-12-26 17:43:03.284516767 +0100 --- src/vim9compile.c 2020-12-26 18:46:40.743791704 +0100 *************** *** 1327,1332 **** --- 1327,1333 ---- sref->sref_sid = sid; sref->sref_idx = idx; sref->sref_seq = si->sn_script_seq; + sref->sref_type = type; return OK; } *** ../vim-8.2.2223/src/vim9.h 2020-12-24 21:56:37.643479575 +0100 --- src/vim9.h 2020-12-26 18:46:07.340042848 +0100 *************** *** 247,252 **** --- 247,253 ---- 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 { *** ../vim-8.2.2223/src/vim9type.c 2020-12-25 15:24:19.902942195 +0100 --- src/vim9type.c 2020-12-26 18:50:57.002085776 +0100 *************** *** 853,859 **** /* * Check if "type1" and "type2" are exactly the same. */ ! static int equal_type(type_T *type1, type_T *type2) { int i; --- 853,859 ---- /* * Check if "type1" and "type2" are exactly the same. */ ! int equal_type(type_T *type1, type_T *type2) { int i; *** ../vim-8.2.2223/src/proto/vim9type.pro 2020-12-25 12:37:59.487073428 +0100 --- src/proto/vim9type.pro 2020-12-26 18:51:21.097941716 +0100 *************** *** 18,23 **** --- 18,24 ---- 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); *** ../vim-8.2.2223/src/errors.h 2020-12-25 19:25:41.742706213 +0100 --- src/errors.h 2020-12-26 18:44:16.004941576 +0100 *************** *** 329,331 **** --- 329,333 ---- 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")); *** ../vim-8.2.2223/src/evalvars.c 2020-12-22 22:07:25.556665849 +0100 --- src/evalvars.c 2020-12-26 19:24:27.014145846 +0100 *************** *** 784,790 **** { if (vim9script) { ! // Vim9 declaration ":let var: type" arg = vim9_declare_scriptvar(eap, arg); } else --- 784,790 ---- { if (vim9script) { ! // Vim9 declaration ":var name: type" arg = vim9_declare_scriptvar(eap, arg); } else *************** *** 3133,3141 **** --- 3133,3148 ---- 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,3222 **** // 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); } if (copy || tv->v_type == VAR_NUMBER || tv->v_type == VAR_FLOAT) --- 3223,3229 ---- // A Vim9 script-local variable is also added to sn_all_vars and // sn_var_vals. if (is_script_local && vim9script) ! update_vim9_script_var(TRUE, di, tv, type); } if (copy || tv->v_type == VAR_NUMBER || tv->v_type == VAR_FLOAT) *** ../vim-8.2.2223/src/vim9script.c 2020-12-26 15:39:24.619550795 +0100 --- src/vim9script.c 2020-12-26 19:49:01.545432856 +0100 *************** *** 591,626 **** /* * 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 "type" is NULL use "tv" for the type. */ void ! add_vim9_script_var(dictitem_T *di, typval_T *tv, type_T *type) { ! scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid); ! // 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) { ! 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( ! 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; --- 591,627 ---- /* * 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 ! update_vim9_script_var(int create, dictitem_T *di, typval_T *tv, type_T *type) { ! scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid); ! hashitem_T *hi; ! svar_T *sv; ! if (create) { ! 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; 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,648 **** else // new variable name hash_add(&si->sn_all_vars.dv_hashtab, newsav->sav_key); - - // let ex_export() know the export worked. - is_export = FALSE; } } /* --- 640,660 ---- 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; } /* *** ../vim-8.2.2223/src/proto/vim9script.pro 2020-10-20 14:25:03.926883018 +0200 --- src/proto/vim9script.pro 2020-12-26 19:25:23.653995469 +0100 *************** *** 8,14 **** 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 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); --- 8,14 ---- 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 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); *** ../vim-8.2.2223/src/testdir/test_vim9_script.vim 2020-12-26 17:43:03.284516767 +0100 --- src/testdir/test_vim9_script.vim 2020-12-26 20:07:19.198798265 +0100 *************** *** 1247,1252 **** --- 1247,1278 ---- 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 *** ../vim-8.2.2223/src/version.c 2020-12-26 17:43:03.284516767 +0100 --- src/version.c 2020-12-26 18:42:21.469983844 +0100 *************** *** 752,753 **** --- 752,755 ---- { /* Add new patch number below this line */ + /**/ + 2224, /**/ -- This planet has -- or rather had -- a problem, which was this: most of the people living on it were unhappy for pretty much of the time. Many solutions were suggested for this problem, but most of these were largely concerned with the movements of small green pieces of paper, which is odd because on the whole it wasn't the small green pieces of paper that were unhappy. -- 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 ///