Mercurial > vim

changeset 23266:00f7cd9b6033 v8.2.2179

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
patch 8.2.2179: Vim9: crash when indexing a dict with a number Commit: https://github.com/vim/vim/commit/4f5e39775616795ac7d1c01bf15a1bd316feb387 Author: Bram Moolenaar <Bram@vim.org> Date: Mon Dec 21 17:30:50 2020 +0100 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 https://github.com/vim/vim/issues/7513)
author Bram Moolenaar <Bram@vim.org>
date Mon, 21 Dec 2020 17:45:04 +0100
parents 684de8fd484a
children 26bd0b99cce1
files src/errors.h src/testdir/test_vim9_assign.vim src/testdir/test_vim9_disassemble.vim src/version.c src/vim9.h src/vim9compile.c src/vim9execute.c
diffstat 7 files changed, 236 insertions(+), 133 deletions(-) [+]
line wrap: on
line diff
--- a/src/errors.h
+++ b/src/errors.h
@@ -323,3 +323,7 @@ EXTERN char e_missing_heredoc_end_marker
 	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"));
--- 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()
--- 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
--- a/src/version.c
+++ b/src/version.c
@@ -751,6 +751,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    2179,
+/**/
     2178,
 /**/
     2177,
--- 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
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -5904,26 +5904,22 @@ compile_assignment(char_u *arg, exarg_T 
 
 	    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
+	    else
+	    {
+		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;
+		{
+		    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 
 	    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)
+		isn_T	*isn = generate_instr_drop(cctx, ISN_STOREINDEX, 3);
+
+		if (isn == NULL)
 		    goto theend;
-	    }
-	    else if (dest_type == VAR_DICT)
-	    {
-		if (generate_instr_drop(cctx, ISN_STOREDICT, 3) == FAIL)
-		    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:
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -802,6 +802,38 @@ store_var(char_u *name, typval_T *tv)
 }
 
 /*
+ * 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.
  */
     static void
@@ -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)
+		    {
+			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;
+			}
+		    }
+		    else if (dest_type != tv_dest->v_type)
+		    {
+			// 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;
+		    }
+
+		    if (status == OK && dest_type == VAR_LIST)
 		    {
-			semsg(_(e_listidx), lidx);
-			goto on_error;
+			varnumber_T	lidx = tv_idx->vval.v_number;
+			list_T		*list = tv_dest->vval.v_list;
+
+			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);
+
+			    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);
+			}
 		    }
-		    tv = STACK_TV_BOT(-3);
-		    if (lidx < list->lv_len)
+		    else if (status == OK && dest_type == VAR_DICT)
 		    {
-			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;
+			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 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(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);
+			status = FAIL;
+			semsg(_(e_cannot_index_str), vartype_name(dest_type));
 		    }
+
 		    clear_tv(tv_idx);
-		    clear_tv(tv_list);
+		    clear_tv(tv_dest);
 		    ectx.ec_stack.ga_len -= 3;
-		}
-		break;
-
-	    // 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;
-
-		    SOURCING_LNUM = iptr->isn_lnum;
-		    if (dict == NULL)
+		    if (status == FAIL)
 		    {
-			emsg(_(e_dictionary_not_set));
+			clear_tv(tv);
 			goto on_error;
 		    }
-		    if (key == NULL)
-			key = (char_u *)"";
-		    tv = STACK_TV_BOT(-3);
-		    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;
-		    }
-		    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);
-		    }
-		    clear_tv(tv_key);
-		    clear_tv(tv_dict);
-		    ectx.ec_stack.ga_len -= 3;
 		}
 		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