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
index a74da2c064dad17e9b5daca1f60952f06e4c5d1c..4f404b533595d7185ed568364bf90b670c76bfb7 100644 (file)
--- 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
index aebd98dc771b54dd02d7ed0f651fd1baead4f5a8..327500bd07f59b40a533662eaa6eeae08e62a45e 100644 (file)
--- 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
index 14f382765ca6e696270e2b65186b31ed0c8c5f58..ea994f8b7027e36573e055d550ec5ae634bb458a 100644 (file)
--- 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
index 2f6655287df173a11968beccfc68520e5411f643..607c0cb61e6e35f784777878ee81f57b150e530a 100644 (file)
--- 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
index a5394f931e8eba18a097024f8e0ba37a66712725..50acca18caf78c4452d2e9d5bf7e99ad9cf52381 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -750,6 +750,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2208,
 /**/
     2207,
 /**/

diff --git a/src/vim9.h b/src/vim9.h
index 9048d23ca0c3704c63247fd15f21d7f296d034e3..2752aa80149e247af0f5be06c9461d72b2d9e0d2 100644 (file)
--- 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
index 18bf68b0c020d32096d99b62eb02cda6e39e3b0a..3305bbbd6911561687a722ac8e220189c9f3c8a4 100644 (file)
--- 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
index 90b4e0ba6b4f14477509431201a38c9a906fcaf1..41505925680a5441ba3829f4542583841799c27b 100644 (file)
--- 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;