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)

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

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)

src/structs.h

@@ -1790,13 +1790,12 @@ typedef struct {
 } imported_T;
 
 /*
- * Growarray to store info about already sourced scripts.
+ * Info about an 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;
+    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.

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

src/version.c

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

src/vim9.h

@@ -244,8 +244,13 @@ typedef struct {
 
 // arguments to ISN_LOADSCRIPT and ISN_STORESCRIPT
 typedef struct {
-    int		script_sid;	// script ID
+    int		sref_sid;	// script ID
-    int		script_idx;	// index in sn_var_vals
+    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

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:

src/vim9execute.c

@@ -1402,12 +1402,21 @@ call_def_function(
 	    // load s: variable in Vim9 script
 	    case ISN_LOADSCRIPT:
 		{
-		    scriptitem_T *si =
+		    scriptref_T	*sref = iptr->isn_arg.script.scriptref;
-				  SCRIPT_ITEM(iptr->isn_arg.script.script_sid);
+		    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)
+		    if (sref->sref_seq != si->sn_script_seq)
-					     + iptr->isn_arg.script.script_idx;
+		    {
+			// 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(
+		    scriptref_T	*sref = iptr->isn_arg.script.scriptref;
-					      iptr->isn_arg.script.script_sid);
+		    dfunc_T	*dfunc = ((dfunc_T *)def_functions.ga_data)
-		    svar_T	 *sv = ((svar_T *)si->sn_var_vals.ga_data)
+							  + ectx.ec_dfunc_idx;
-					     + iptr->isn_arg.script.script_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 =
+		    scriptref_T	*sref = iptr->isn_arg.script.scriptref;
-				  SCRIPT_ITEM(iptr->isn_arg.script.script_sid);
+		    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 =
+		    scriptref_T	*sref = iptr->isn_arg.script.scriptref;
-				  SCRIPT_ITEM(iptr->isn_arg.script.script_sid);
+		    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;