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.

Author: brammool

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"));

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)

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);

src/proto/vim9type.pro

@@ -18,6 +18,7 @@ int check_type(type_T *expected, type_T *actual, int give_msg, int argidx);
 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);

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

src/version.c

@@ -750,6 +750,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2224,
 /**/
     2223,
 /**/

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 {

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;
 }
 

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;
+		    scriptref_T	    *sref = iptr->isn_arg.script.scriptref;
+		    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);
+		    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);

src/vim9script.c

@@ -591,36 +591,37 @@ vim9_declare_scriptvar(exarg_T *eap, char_u *arg)
 /*
  * 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(
-				       sizeof(sallvar_T) + STRLEN(di->di_key));
+	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, typval_T *tv, type_T *type)
 	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;
     }
+    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;
 }
 
 /*