patch 8.2.2179: Vim9: crash when indexing a dict with a number

Problem:    Vim9: crash when indexing a dict with a number.
Solution:   Add ISN_STOREINDEX. (closes #7513)

diff --git a/src/errors.h b/src/errors.h
index 023c1d1c62c13f6d5c4affd8366e5a3471757b10..a74da2c064dad17e9b5daca1f60952f06e4c5d1c 100644 (file)
--- a/src/errors.h
+++ b/src/errors.h
@@ -323,3 +323,7 @@ EXTERN char e_missing_heredoc_end_marker_str[]
        INIT(= N_("E1145: Missing heredoc end marker: %s"));
 EXTERN char e_command_not_recognized_str[]
        INIT(= N_("E1146: Command not recognized: %s"));
+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"));

diff --git a/src/testdir/test_vim9_assign.vim b/src/testdir/test_vim9_assign.vim
index 7954aa128ed66e5540fbeaa18e13ff83890fae6c..bfc2d20de87d28f146850e6e77904a91d2cfe15d 100644 (file)
--- a/src/testdir/test_vim9_assign.vim
+++ b/src/testdir/test_vim9_assign.vim
@@ -533,6 +533,12 @@ def Test_assignment_list()
 
   # type becomes list<any>
   var somelist = rand() > 0 ? [1, 2, 3] : ['a', 'b', 'c']
+
+  var lines =<< trim END
+    var d = {dd: test_null_list()}
+    d.dd[0] = 0
+  END
+  CheckDefExecFailure(lines, 'E1147:', 2)
 enddef
 
 def Test_assignment_list_vim9script()
@@ -567,12 +573,20 @@ def Test_assignment_dict()
   assert_equal({nest: {this: 123, that: 456}, nr: 0}, anydict)
 
   var lines =<< trim END
-    vim9script
     var dd = {}
     dd.two = 2
     assert_equal({two: 2}, dd)
   END
-  CheckScriptSuccess(lines)
+  CheckDefAndScriptSuccess(lines)
+
+  lines =<< trim END
+    var d = {dd: {}}
+    d.dd[0] = 2
+    d.dd['x'] = 3
+    d.dd.y = 4
+    assert_equal({dd: {0: 2, x: 3, y: 4}}, d)
+  END
+  CheckDefAndScriptSuccess(lines)
 
   lines =<< trim END
     var dd = {one: 1}
@@ -641,6 +655,18 @@ def Test_assignment_dict()
     assert_equal({a: 43}, FillDict())
   END
   CheckScriptSuccess(lines)
+
+  lines =<< trim END
+    var d = {dd: test_null_dict()}
+    d.dd[0] = 0
+  END
+  CheckDefExecFailure(lines, 'E1103:', 2)
+
+  lines =<< trim END
+    var d = {dd: 'string'}
+    d.dd[0] = 0
+  END
+  CheckDefExecFailure(lines, 'E1148:', 2)
 enddef
 
 def Test_assignment_local()

diff --git a/src/testdir/test_vim9_disassemble.vim b/src/testdir/test_vim9_disassemble.vim
index d936bc29a408e3820c96cab5bb43738f48b43a8a..7f88cfa0d30934ee0523fd57191955ac616dd26b 100644 (file)
--- a/src/testdir/test_vim9_disassemble.vim
+++ b/src/testdir/test_vim9_disassemble.vim
@@ -276,6 +276,30 @@ def Test_disassemble_store_member()
         res)
 enddef
 
+def s:ScriptFuncStoreIndex()
+  var d = {dd: {}}
+  d.dd[0] = 0
+enddef
+
+def Test_disassemble_store_index()
+  var res = execute('disass s:ScriptFuncStoreIndex')
+  assert_match('<SNR>\d*_ScriptFuncStoreIndex\_s*' ..
+        'var d = {dd: {}}\_s*' ..
+        '\d PUSHS "dd"\_s*' ..
+        '\d NEWDICT size 0\_s*' ..
+        '\d NEWDICT size 1\_s*' ..
+        '\d STORE $0\_s*' ..
+        'd.dd\[0\] = 0\_s*' ..
+        '\d PUSHNR 0\_s*' ..
+        '\d PUSHNR 0\_s*' ..
+        '\d LOAD $0\_s*' ..
+        '\d MEMBER dd\_s*' ..
+        '\d STOREINDEX\_s*' ..
+        '\d\+ PUSHNR 0\_s*' ..
+        '\d\+ RETURN',
+        res)
+enddef
+
 def s:ListAssign()
   var x: string
   var y: string

diff --git a/src/version.c b/src/version.c
index aa6967d8587030a5f0843628e830b58cd43246b3..9e1327d0d4589cc0ac2d0eb610403cca806aa1a3 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 */
+/**/
+    2179,
 /**/
     2178,
 /**/

diff --git a/src/vim9.h b/src/vim9.h
index 26307c2707ae06259d0440c58e4be546f5fa12b4..5ced180eded5f6e17df19df5aef1dde9b7552fd0 100644 (file)
--- a/src/vim9.h
+++ b/src/vim9.h
@@ -55,8 +55,8 @@ typedef enum {
     // ISN_STOREOTHER, // pop into other script variable isn_arg.other.
 
     ISN_STORENR,    // store number into local variable isn_arg.storenr.stnr_idx
-    ISN_STORELIST,     // store into list, value/index/variable on stack
-    ISN_STOREDICT,     // store into dictionary, value/index/variable on stack
+    ISN_STOREINDEX,    // store into list or dictionary, type isn_arg.vartype,
+                       // value/index/variable on stack
 
     ISN_UNLET,         // unlet variable isn_arg.unlet.ul_name
     ISN_UNLETENV,      // unlet environment variable isn_arg.unlet.ul_name
@@ -304,6 +304,7 @@ struct isn_S {
        char_u              *string;
        varnumber_T         number;
        blob_T              *blob;
+       vartype_T           vartype;
 #ifdef FEAT_FLOAT
        float_T             fnumber;
 #endif

diff --git a/src/vim9compile.c b/src/vim9compile.c
index 133a09cc96be026a42a0151d82f1e2e3aba79594..2a305ee12ada508d7e5c9c0a022786e9a40e0a04 100644 (file)
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -5904,26 +5904,22 @@ compile_assignment(char_u *arg, exarg_T *eap, cmdidx_T cmdidx, cctx_T *cctx)
 
            if (type == &t_any)
            {
-               type_T      *idx_type = ((type_T **)stack->ga_data)[
-                                                           stack->ga_len - 1];
-               // Index on variable of unknown type: guess the type from the
-               // index type: number is dict, otherwise dict.
-               // TODO: should do the assignment at runtime
-               if (idx_type->tt_type == VAR_NUMBER)
-                   type = &t_list_any;
-               else
-                   type = &t_dict_any;
+               // Index on variable of unknown type: check at runtime.
+               dest_type = VAR_ANY;
            }
-           dest_type = type->tt_type;
-           if (dest_type == VAR_DICT
-                   && may_generate_2STRING(-1, cctx) == FAIL)
-               goto theend;
-           if (dest_type == VAR_LIST
-                   && ((type_T **)stack->ga_data)[stack->ga_len - 1]->tt_type
-                                                                != VAR_NUMBER)
+           else
            {
-               emsg(_(e_number_exp));
-               goto theend;
+               dest_type = type->tt_type;
+               if (dest_type == VAR_DICT
+                       && may_generate_2STRING(-1, cctx) == FAIL)
+                   goto theend;
+               if (dest_type == VAR_LIST
+                    && ((type_T **)stack->ga_data)[stack->ga_len - 1]->tt_type
+                                                                != VAR_NUMBER)
+               {
+                   emsg(_(e_number_exp));
+                   goto theend;
+               }
            }
 
            // Load the dict or list.  On the stack we then have:
@@ -5956,15 +5952,14 @@ compile_assignment(char_u *arg, exarg_T *eap, cmdidx_T cmdidx, cctx_T *cctx)
            else
                generate_loadvar(cctx, dest, name, lvar, type);
 
-           if (dest_type == VAR_LIST)
+           if (dest_type == VAR_LIST || dest_type == VAR_DICT
+                                                      || dest_type == VAR_ANY)
            {
-               if (generate_instr_drop(cctx, ISN_STORELIST, 3) == FAIL)
-                   goto theend;
-           }
-           else if (dest_type == VAR_DICT)
-           {
-               if (generate_instr_drop(cctx, ISN_STOREDICT, 3) == FAIL)
+               isn_T   *isn = generate_instr_drop(cctx, ISN_STOREINDEX, 3);
+
+               if (isn == NULL)
                    goto theend;
+               isn->isn_arg.vartype = dest_type;
            }
            else
            {
@@ -8194,8 +8189,7 @@ delete_instr(isn_T *isn)
        case ISN_SHUFFLE:
        case ISN_SLICE:
        case ISN_STORE:
-       case ISN_STOREDICT:
-       case ISN_STORELIST:
+       case ISN_STOREINDEX:
        case ISN_STORENR:
        case ISN_STOREOUTER:
        case ISN_STOREREG:

diff --git a/src/vim9execute.c b/src/vim9execute.c
index db15cea4cda0401fad339cb10bc952708999920b..7d56e2143ec1b69b2ae6f771aba002351ef13d56 100644 (file)
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -801,6 +801,38 @@ store_var(char_u *name, typval_T *tv)
     restore_funccal();
 }
 
+/*
+ * Convert "tv" to a string.
+ * Return FAIL if not allowed.
+ */
+    static int
+do_2string(typval_T *tv, int is_2string_any)
+{
+    if (tv->v_type != VAR_STRING)
+    {
+       char_u *str;
+
+       if (is_2string_any)
+       {
+           switch (tv->v_type)
+           {
+               case VAR_SPECIAL:
+               case VAR_BOOL:
+               case VAR_NUMBER:
+               case VAR_FLOAT:
+               case VAR_BLOB:  break;
+               default:        to_string_error(tv->v_type);
+                               return FAIL;
+           }
+       }
+       str = typval_tostring(tv);
+       clear_tv(tv);
+       tv->v_type = VAR_STRING;
+       tv->vval.v_string = str;
+    }
+    return OK;
+}
+
 /*
  * When the value of "sv" is a null list of dict, allocate it.
  */
@@ -1700,92 +1732,126 @@ call_def_function(
                tv->vval.v_number = iptr->isn_arg.storenr.stnr_val;
                break;
 
-           // store value in list variable
-           case ISN_STORELIST:
+           // store value in list or dict variable
+           case ISN_STOREINDEX:
                {
+                   vartype_T   dest_type = iptr->isn_arg.vartype;
                    typval_T    *tv_idx = STACK_TV_BOT(-2);
-                   varnumber_T lidx = tv_idx->vval.v_number;
-                   typval_T    *tv_list = STACK_TV_BOT(-1);
-                   list_T      *list = tv_list->vval.v_list;
+                   typval_T    *tv_dest = STACK_TV_BOT(-1);
+                   int         status = OK;
 
+                   tv = STACK_TV_BOT(-3);
                    SOURCING_LNUM = iptr->isn_lnum;
-                   if (lidx < 0 && list->lv_len + lidx >= 0)
-                       // negative index is relative to the end
-                       lidx = list->lv_len + lidx;
-                   if (lidx < 0 || lidx > list->lv_len)
+                   if (dest_type == VAR_ANY)
                    {
-                       semsg(_(e_listidx), lidx);
-                       goto on_error;
+                       dest_type = tv_dest->v_type;
+                       if (dest_type == VAR_DICT)
+                           status = do_2string(tv_idx, TRUE);
+                       else if (dest_type == VAR_LIST
+                                              && tv_idx->v_type != VAR_NUMBER)
+                       {
+                           emsg(_(e_number_exp));
+                           status = FAIL;
+                       }
                    }
-                   tv = STACK_TV_BOT(-3);
-                   if (lidx < list->lv_len)
+                   else if (dest_type != tv_dest->v_type)
                    {
-                       listitem_T *li = list_find(list, lidx);
-
-                       if (error_if_locked(li->li_tv.v_lock,
-                                                   e_cannot_change_list_item))
-                           goto failed;
-                       // overwrite existing list item
-                       clear_tv(&li->li_tv);
-                       li->li_tv = *tv;
+                       // just in case, should be OK
+                       semsg(_(e_expected_str_but_got_str),
+                                   vartype_name(dest_type),
+                                   vartype_name(tv_dest->v_type));
+                       status = FAIL;
                    }
-                   else
+
+                   if (status == OK && dest_type == VAR_LIST)
                    {
-                       if (error_if_locked(list->lv_lock,
-                                                        e_cannot_change_list))
-                           goto failed;
-                       // append to list, only fails when out of memory
-                       if (list_append_tv(list, tv) == FAIL)
-                           goto failed;
-                       clear_tv(tv);
-                   }
-                   clear_tv(tv_idx);
-                   clear_tv(tv_list);
-                   ectx.ec_stack.ga_len -= 3;
-               }
-               break;
+                       varnumber_T     lidx = tv_idx->vval.v_number;
+                       list_T          *list = tv_dest->vval.v_list;
 
-           // store value in dict variable
-           case ISN_STOREDICT:
-               {
-                   typval_T    *tv_key = STACK_TV_BOT(-2);
-                   char_u      *key = tv_key->vval.v_string;
-                   typval_T    *tv_dict = STACK_TV_BOT(-1);
-                   dict_T      *dict = tv_dict->vval.v_dict;
-                   dictitem_T  *di;
+                       if (list == NULL)
+                       {
+                           emsg(_(e_list_not_set));
+                           goto on_error;
+                       }
+                       if (lidx < 0 && list->lv_len + lidx >= 0)
+                           // negative index is relative to the end
+                           lidx = list->lv_len + lidx;
+                       if (lidx < 0 || lidx > list->lv_len)
+                       {
+                           semsg(_(e_listidx), lidx);
+                           goto on_error;
+                       }
+                       if (lidx < list->lv_len)
+                       {
+                           listitem_T *li = list_find(list, lidx);
 
-                   SOURCING_LNUM = iptr->isn_lnum;
-                   if (dict == NULL)
-                   {
-                       emsg(_(e_dictionary_not_set));
-                       goto on_error;
+                           if (error_if_locked(li->li_tv.v_lock,
+                                                   e_cannot_change_list_item))
+                               goto on_error;
+                           // overwrite existing list item
+                           clear_tv(&li->li_tv);
+                           li->li_tv = *tv;
+                       }
+                       else
+                       {
+                           if (error_if_locked(list->lv_lock,
+                                                        e_cannot_change_list))
+                               goto on_error;
+                           // append to list, only fails when out of memory
+                           if (list_append_tv(list, tv) == FAIL)
+                               goto failed;
+                           clear_tv(tv);
+                       }
                    }
-                   if (key == NULL)
-                       key = (char_u *)"";
-                   tv = STACK_TV_BOT(-3);
-                   di = dict_find(dict, key, -1);
-                   if (di != NULL)
+                   else if (status == OK && dest_type == VAR_DICT)
                    {
-                       if (error_if_locked(di->di_tv.v_lock,
+                       char_u          *key = tv_idx->vval.v_string;
+                       dict_T          *dict = tv_dest->vval.v_dict;
+                       dictitem_T      *di;
+
+                       SOURCING_LNUM = iptr->isn_lnum;
+                       if (dict == NULL)
+                       {
+                           emsg(_(e_dictionary_not_set));
+                           goto on_error;
+                       }
+                       if (key == NULL)
+                           key = (char_u *)"";
+                       di = dict_find(dict, key, -1);
+                       if (di != NULL)
+                       {
+                           if (error_if_locked(di->di_tv.v_lock,
                                                    e_cannot_change_dict_item))
-                           goto failed;
-                       // overwrite existing value
-                       clear_tv(&di->di_tv);
-                       di->di_tv = *tv;
+                               goto on_error;
+                           // overwrite existing value
+                           clear_tv(&di->di_tv);
+                           di->di_tv = *tv;
+                       }
+                       else
+                       {
+                           if (error_if_locked(dict->dv_lock,
+                                                        e_cannot_change_dict))
+                               goto on_error;
+                           // add to dict, only fails when out of memory
+                           if (dict_add_tv(dict, (char *)key, tv) == FAIL)
+                               goto failed;
+                           clear_tv(tv);
+                       }
                    }
                    else
                    {
-                       if (error_if_locked(dict->dv_lock,
-                                                        e_cannot_change_dict))
-                           goto failed;
-                       // add to dict, only fails when out of memory
-                       if (dict_add_tv(dict, (char *)key, tv) == FAIL)
-                           goto failed;
-                       clear_tv(tv);
+                       status = FAIL;
+                       semsg(_(e_cannot_index_str), vartype_name(dest_type));
                    }
-                   clear_tv(tv_key);
-                   clear_tv(tv_dict);
+
+                   clear_tv(tv_idx);
+                   clear_tv(tv_dest);
                    ectx.ec_stack.ga_len -= 3;
+                   if (status == FAIL)
+                   {
+                       clear_tv(tv);
+                       goto on_error;
+                   }
                }
                break;
 
@@ -2921,31 +2987,9 @@ call_def_function(
 
            case ISN_2STRING:
            case ISN_2STRING_ANY:
-               {
-                   char_u *str;
-
-                   tv = STACK_TV_BOT(iptr->isn_arg.number);
-                   if (tv->v_type != VAR_STRING)
-                   {
-                       if (iptr->isn_type == ISN_2STRING_ANY)
-                       {
-                           switch (tv->v_type)
-                           {
-                               case VAR_SPECIAL:
-                               case VAR_BOOL:
-                               case VAR_NUMBER:
-                               case VAR_FLOAT:
-                               case VAR_BLOB:  break;
-                               default:        to_string_error(tv->v_type);
-                                               goto on_error;
-                           }
-                       }
-                       str = typval_tostring(tv);
-                       clear_tv(tv);
-                       tv->v_type = VAR_STRING;
-                       tv->vval.v_string = str;
-                   }
-               }
+               if (do_2string(STACK_TV_BOT(iptr->isn_arg.number),
+                       iptr->isn_type == ISN_2STRING_ANY) == FAIL)
+                           goto on_error;
                break;
 
            case ISN_RANGE:
@@ -3462,12 +3506,20 @@ ex_disassemble(exarg_T *eap)
                                iptr->isn_arg.storenr.stnr_idx);
                break;
 
-           case ISN_STORELIST:
-               smsg("%4d STORELIST", current);
-               break;
-
-           case ISN_STOREDICT:
-               smsg("%4d STOREDICT", current);
+           case ISN_STOREINDEX:
+               switch (iptr->isn_arg.vartype)
+               {
+                   case VAR_LIST:
+                           smsg("%4d STORELIST", current);
+                           break;
+                   case VAR_DICT:
+                           smsg("%4d STOREDICT", current);
+                           break;
+                   case VAR_ANY:
+                           smsg("%4d STOREINDEX", current);
+                           break;
+                   default: break;
+               }
                break;
 
            // constants