# HG changeset patch # User Bram Moolenaar # Date 1608569104 -3600 # Node ID 00f7cd9b60332a7275552779889e06dc67af7dfb # Parent 684de8fd484ac5ed9f53102fea35da67cbe9c4b7 patch 8.2.2179: Vim9: crash when indexing a dict with a number Commit: https://github.com/vim/vim/commit/4f5e39775616795ac7d1c01bf15a1bd316feb387 Author: Bram Moolenaar 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) diff --git a/src/errors.h b/src/errors.h --- 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")); diff --git a/src/testdir/test_vim9_assign.vim b/src/testdir/test_vim9_assign.vim --- 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 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 --- 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('\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 --- 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, diff --git a/src/vim9.h b/src/vim9.h --- 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 --- 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: diff --git a/src/vim9execute.c b/src/vim9execute.c --- 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