To: vim_dev@googlegroups.com Subject: Patch 8.2.2188 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.2188 Problem: Vim9: crash when calling global function from :def function. Solution: Set the outer context. Define the partial for the context on the original function. Use a refcount to keep track of which ufunc is using a dfunc. (closes #7525) Files: src/vim9compile.c, src/proto/vim9compile.pro, src/vim9execute.c, src/proto/vim9execute.pro, src/userfunc.c, src/proto/userfunc.pro, src/structs.h, src/vim9.h, src/testdir/test_vim9_func.vim *** ../vim-8.2.2187/src/vim9compile.c 2020-12-21 17:30:46.941668485 +0100 --- src/vim9compile.c 2020-12-22 16:43:30.018780085 +0100 *************** *** 7336,7341 **** --- 7336,7343 ---- dfunc->df_idx = def_functions.ga_len; ufunc->uf_dfunc_idx = dfunc->df_idx; dfunc->df_ufunc = ufunc; + dfunc->df_name = vim_strsave(ufunc->uf_name); + ++dfunc->df_refcount; ++def_functions.ga_len; return OK; } *************** *** 7928,7933 **** --- 7930,7936 ---- for (idx = 0; idx < instr->ga_len; ++idx) delete_instr(((isn_T *)instr->ga_data) + idx); ga_clear(instr); + VIM_CLEAR(dfunc->df_name); // If using the last entry in the table and it was added above, we // might as well remove it. *************** *** 8102,8110 **** if (ufunc != NULL) { ! // Clear uf_dfunc_idx so that the function is deleted. ! clear_def_function(ufunc); ! ufunc->uf_dfunc_idx = 0; func_ptr_unref(ufunc); } --- 8105,8111 ---- if (ufunc != NULL) { ! unlink_def_function(ufunc); func_ptr_unref(ufunc); } *************** *** 8206,8212 **** } /* ! * Free all instructions for "dfunc". */ static void delete_def_function_contents(dfunc_T *dfunc) --- 8207,8213 ---- } /* ! * Free all instructions for "dfunc" except df_name. */ static void delete_def_function_contents(dfunc_T *dfunc) *************** *** 8227,8257 **** /* * When a user function is deleted, clear the contents of any associated def ! * function. The position in def_functions can be re-used. */ void ! clear_def_function(ufunc_T *ufunc) { if (ufunc->uf_dfunc_idx > 0) { dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + ufunc->uf_dfunc_idx; ! delete_def_function_contents(dfunc); ufunc->uf_def_status = UF_NOT_COMPILED; } } /* ! * Used when a user function is about to be deleted: remove the pointer to it. ! * The entry in def_functions is then unused. */ void ! unlink_def_function(ufunc_T *ufunc) { ! dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + ufunc->uf_dfunc_idx; ! dfunc->df_ufunc = NULL; } #if defined(EXITFREE) || defined(PROTO) --- 8228,8266 ---- /* * When a user function is deleted, clear the contents of any associated def ! * function, unless another user function still uses it. ! * The position in def_functions can be re-used. */ void ! unlink_def_function(ufunc_T *ufunc) { if (ufunc->uf_dfunc_idx > 0) { dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + ufunc->uf_dfunc_idx; ! if (--dfunc->df_refcount <= 0) ! delete_def_function_contents(dfunc); ufunc->uf_def_status = UF_NOT_COMPILED; + ufunc->uf_dfunc_idx = 0; + if (dfunc->df_ufunc == ufunc) + dfunc->df_ufunc = NULL; } } /* ! * Used when a user function refers to an existing dfunc. */ void ! link_def_function(ufunc_T *ufunc) { ! if (ufunc->uf_dfunc_idx > 0) ! { ! dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) ! + ufunc->uf_dfunc_idx; ! ++dfunc->df_refcount; ! } } #if defined(EXITFREE) || defined(PROTO) *************** *** 8268,8273 **** --- 8277,8283 ---- dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + idx; delete_def_function_contents(dfunc); + vim_free(dfunc->df_name); } ga_clear(&def_functions); *** ../vim-8.2.2187/src/proto/vim9compile.pro 2020-11-20 18:59:14.470192932 +0100 --- src/proto/vim9compile.pro 2020-12-22 15:39:09.172811401 +0100 *************** *** 18,24 **** int compile_def_function(ufunc_T *ufunc, int set_return_type, cctx_T *outer_cctx); void set_function_type(ufunc_T *ufunc); void delete_instr(isn_T *isn); - void clear_def_function(ufunc_T *ufunc); void unlink_def_function(ufunc_T *ufunc); void free_def_functions(void); /* vim: set ft=c : */ --- 18,24 ---- int compile_def_function(ufunc_T *ufunc, int set_return_type, cctx_T *outer_cctx); void set_function_type(ufunc_T *ufunc); void delete_instr(isn_T *isn); void unlink_def_function(ufunc_T *ufunc); + void link_def_function(ufunc_T *ufunc); void free_def_functions(void); /* vim: set ft=c : */ *** ../vim-8.2.2187/src/vim9execute.c 2020-12-21 17:30:46.941668485 +0100 --- src/vim9execute.c 2020-12-22 16:40:44.771366336 +0100 *************** *** 54,60 **** /* * Execution context. */ ! typedef struct { garray_T ec_stack; // stack of typval_T values int ec_frame_idx; // index in ec_stack: context of ec_dfunc_idx --- 54,60 ---- /* * Execution context. */ ! struct ectx_S { garray_T ec_stack; // stack of typval_T values int ec_frame_idx; // index in ec_stack: context of ec_dfunc_idx *************** *** 69,75 **** int ec_iidx; // index in ec_instr: instruction to execute garray_T ec_funcrefs; // partials that might be a closure ! } ectx_T; // Get pointer to item relative to the bottom of the stack, -1 is the last one. #define STACK_TV_BOT(idx) (((typval_T *)ectx->ec_stack.ga_data) + ectx->ec_stack.ga_len + (idx)) --- 69,75 ---- int ec_iidx; // index in ec_instr: instruction to execute garray_T ec_funcrefs; // partials that might be a closure ! }; // Get pointer to item relative to the bottom of the stack, -1 is the last one. #define STACK_TV_BOT(idx) (((typval_T *)ectx->ec_stack.ga_data) + ectx->ec_stack.ga_len + (idx)) *************** *** 173,179 **** if (dfunc->df_deleted) { ! emsg_funcname(e_func_deleted, ufunc->uf_name); return FAIL; } --- 173,181 ---- if (dfunc->df_deleted) { ! // don't use ufunc->uf_name, it may have been freed ! emsg_funcname(e_func_deleted, ! dfunc->df_name == NULL ? (char_u *)"unknown" : dfunc->df_name); return FAIL; } *************** *** 260,265 **** --- 262,273 ---- } ectx->ec_stack.ga_len += STACK_FRAME_SIZE + varcount; + if (ufunc->uf_partial != NULL) + { + ectx->ec_outer_stack = ufunc->uf_partial->pt_ectx_stack; + ectx->ec_outer_frame = ufunc->uf_partial->pt_ectx_frame; + } + // Set execution state to the start of the called function. ectx->ec_dfunc_idx = cdf_idx; ectx->ec_instr = dfunc->df_instr; *************** *** 618,623 **** --- 626,632 ---- // The function has been compiled, can call it quickly. For a function // that was defined later: we can call it directly next time. + // TODO: what if the function was deleted and then defined again? if (iptr != NULL) { delete_instr(iptr); *************** *** 890,896 **** * When a function reference is used, fill a partial with the information * needed, especially when it is used as a closure. */ ! static int fill_partial_and_closure(partial_T *pt, ufunc_T *ufunc, ectx_T *ectx) { pt->pt_func = ufunc; --- 899,905 ---- * When a function reference is used, fill a partial with the information * needed, especially when it is used as a closure. */ ! int fill_partial_and_closure(partial_T *pt, ufunc_T *ufunc, ectx_T *ectx) { pt->pt_func = ufunc; *************** *** 2120,2144 **** case ISN_NEWFUNC: { newfunc_T *newfunc = &iptr->isn_arg.newfunc; - ufunc_T *new_ufunc; ! new_ufunc = copy_func( ! newfunc->nf_lambda, newfunc->nf_global); ! if (new_ufunc != NULL ! && (new_ufunc->uf_flags & FC_CLOSURE)) ! { ! partial_T *pt = ALLOC_CLEAR_ONE(partial_T); ! ! // Need to create a partial to store the context of the ! // function. ! if (pt == NULL) ! goto failed; ! if (fill_partial_and_closure(pt, new_ufunc, &ectx) == FAIL) ! goto failed; ! new_ufunc->uf_partial = pt; ! --pt->pt_refcount; // not referenced here ! } } break; --- 2129,2138 ---- case ISN_NEWFUNC: { newfunc_T *newfunc = &iptr->isn_arg.newfunc; ! if (copy_func(newfunc->nf_lambda, newfunc->nf_global, &ectx) == FAIL) ! goto failed; } break; *** ../vim-8.2.2187/src/proto/vim9execute.pro 2020-11-12 12:08:47.982254065 +0100 --- src/proto/vim9execute.pro 2020-12-22 14:16:33.134679296 +0100 *************** *** 1,6 **** --- 1,7 ---- /* vim9execute.c */ void to_string_error(vartype_T vartype); void funcstack_check_refcount(funcstack_T *funcstack); + int fill_partial_and_closure(partial_T *pt, ufunc_T *ufunc, ectx_T *ectx); int call_def_function(ufunc_T *ufunc, int argc_arg, typval_T *argv, partial_T *partial, typval_T *rettv); void ex_disassemble(exarg_T *eap); int tv2bool(typval_T *tv); *** ../vim-8.2.2187/src/userfunc.c 2020-12-20 21:10:13.902437880 +0100 --- src/userfunc.c 2020-12-22 17:22:58.998371985 +0100 *************** *** 1260,1267 **** // clear this function func_clear_items(fp); funccal_unref(fp->uf_scoped, fp, force); ! if ((fp->uf_flags & FC_COPY) == 0) ! clear_def_function(fp); } /* --- 1260,1266 ---- // clear this function func_clear_items(fp); funccal_unref(fp->uf_scoped, fp, force); ! unlink_def_function(fp); } /* *************** *** 1307,1381 **** /* * Copy already defined function "lambda" to a new function with name "global". * This is for when a compiled function defines a global function. - * Caller should take care of adding a partial for a closure. */ ! ufunc_T * ! copy_func(char_u *lambda, char_u *global) { ufunc_T *ufunc = find_func_even_dead(lambda, TRUE, NULL); ufunc_T *fp = NULL; if (ufunc == NULL) semsg(_(e_lambda_function_not_found_str), lambda); ! else { ! // TODO: handle ! to overwrite ! fp = find_func(global, TRUE, NULL); ! if (fp != NULL) ! { ! semsg(_(e_funcexts), global); ! return NULL; ! } ! ! fp = alloc_clear(offsetof(ufunc_T, uf_name) + STRLEN(global) + 1); ! if (fp == NULL) ! return NULL; ! ! fp->uf_varargs = ufunc->uf_varargs; ! fp->uf_flags = (ufunc->uf_flags & ~FC_VIM9) | FC_COPY; ! fp->uf_def_status = ufunc->uf_def_status; ! fp->uf_dfunc_idx = ufunc->uf_dfunc_idx; ! if (ga_copy_strings(&ufunc->uf_args, &fp->uf_args) == FAIL ! || ga_copy_strings(&ufunc->uf_def_args, &fp->uf_def_args) ! == FAIL ! || ga_copy_strings(&ufunc->uf_lines, &fp->uf_lines) == FAIL) goto failed; ! fp->uf_name_exp = ufunc->uf_name_exp == NULL ? NULL ! : vim_strsave(ufunc->uf_name_exp); ! if (ufunc->uf_arg_types != NULL) ! { ! fp->uf_arg_types = ALLOC_MULT(type_T *, fp->uf_args.ga_len); ! if (fp->uf_arg_types == NULL) ! goto failed; ! mch_memmove(fp->uf_arg_types, ufunc->uf_arg_types, ! sizeof(type_T *) * fp->uf_args.ga_len); ! } ! if (ufunc->uf_def_arg_idx != NULL) ! { ! fp->uf_def_arg_idx = ALLOC_MULT(int, fp->uf_def_args.ga_len + 1); ! if (fp->uf_def_arg_idx == NULL) ! goto failed; ! mch_memmove(fp->uf_def_arg_idx, ufunc->uf_def_arg_idx, ! sizeof(int) * fp->uf_def_args.ga_len + 1); ! } ! if (ufunc->uf_va_name != NULL) ! { ! fp->uf_va_name = vim_strsave(ufunc->uf_va_name); ! if (fp->uf_va_name == NULL) ! goto failed; ! } ! fp->uf_ret_type = ufunc->uf_ret_type; ! fp->uf_refcount = 1; ! STRCPY(fp->uf_name, global); ! hash_add(&func_hashtab, UF2HIKEY(fp)); } ! return fp; failed: func_clear_free(fp, TRUE); ! return NULL; } static int funcdepth = 0; --- 1306,1403 ---- /* * Copy already defined function "lambda" to a new function with name "global". * This is for when a compiled function defines a global function. */ ! int ! copy_func(char_u *lambda, char_u *global, ectx_T *ectx) { ufunc_T *ufunc = find_func_even_dead(lambda, TRUE, NULL); ufunc_T *fp = NULL; if (ufunc == NULL) + { semsg(_(e_lambda_function_not_found_str), lambda); ! return FAIL; ! } ! ! // TODO: handle ! to overwrite ! fp = find_func(global, TRUE, NULL); ! if (fp != NULL) ! { ! semsg(_(e_funcexts), global); ! return FAIL; ! } ! ! fp = alloc_clear(offsetof(ufunc_T, uf_name) + STRLEN(global) + 1); ! if (fp == NULL) ! return FAIL; ! ! fp->uf_varargs = ufunc->uf_varargs; ! fp->uf_flags = (ufunc->uf_flags & ~FC_VIM9) | FC_COPY; ! fp->uf_def_status = ufunc->uf_def_status; ! fp->uf_dfunc_idx = ufunc->uf_dfunc_idx; ! if (ga_copy_strings(&ufunc->uf_args, &fp->uf_args) == FAIL ! || ga_copy_strings(&ufunc->uf_def_args, &fp->uf_def_args) ! == FAIL ! || ga_copy_strings(&ufunc->uf_lines, &fp->uf_lines) == FAIL) ! goto failed; ! ! fp->uf_name_exp = ufunc->uf_name_exp == NULL ? NULL ! : vim_strsave(ufunc->uf_name_exp); ! if (ufunc->uf_arg_types != NULL) ! { ! fp->uf_arg_types = ALLOC_MULT(type_T *, fp->uf_args.ga_len); ! if (fp->uf_arg_types == NULL) ! goto failed; ! mch_memmove(fp->uf_arg_types, ufunc->uf_arg_types, ! sizeof(type_T *) * fp->uf_args.ga_len); ! } ! if (ufunc->uf_def_arg_idx != NULL) ! { ! fp->uf_def_arg_idx = ALLOC_MULT(int, fp->uf_def_args.ga_len + 1); ! if (fp->uf_def_arg_idx == NULL) ! goto failed; ! mch_memmove(fp->uf_def_arg_idx, ufunc->uf_def_arg_idx, ! sizeof(int) * fp->uf_def_args.ga_len + 1); ! } ! if (ufunc->uf_va_name != NULL) { ! fp->uf_va_name = vim_strsave(ufunc->uf_va_name); ! if (fp->uf_va_name == NULL) goto failed; + } + fp->uf_ret_type = ufunc->uf_ret_type; ! fp->uf_refcount = 1; ! STRCPY(fp->uf_name, global); ! hash_add(&func_hashtab, UF2HIKEY(fp)); ! // the referenced dfunc_T is now used one more time ! link_def_function(fp); ! ! // Create a partial to store the context of the function, if not done ! // already. ! if ((ufunc->uf_flags & FC_CLOSURE) && ufunc->uf_partial == NULL) ! { ! partial_T *pt = ALLOC_CLEAR_ONE(partial_T); ! ! if (pt == NULL) ! goto failed; ! if (fill_partial_and_closure(pt, ufunc, ectx) == FAIL) ! goto failed; ! ufunc->uf_partial = pt; ! --pt->pt_refcount; // not referenced here yet ! } ! if (ufunc->uf_partial != NULL) ! { ! fp->uf_partial = ufunc->uf_partial; ! ++fp->uf_partial->pt_refcount; } ! ! return OK; failed: func_clear_free(fp, TRUE); ! return FAIL; } static int funcdepth = 0; *************** *** 3515,3521 **** fp->uf_profiling = FALSE; fp->uf_prof_initialized = FALSE; #endif ! clear_def_function(fp); } } } --- 3537,3543 ---- fp->uf_profiling = FALSE; fp->uf_prof_initialized = FALSE; #endif ! unlink_def_function(fp); } } } *** ../vim-8.2.2187/src/proto/userfunc.pro 2020-12-20 21:10:13.902437880 +0100 --- src/proto/userfunc.pro 2020-12-22 14:24:53.508784632 +0100 *************** *** 13,19 **** ufunc_T *find_func(char_u *name, int is_global, cctx_T *cctx); int func_is_global(ufunc_T *ufunc); int func_name_refcount(char_u *name); ! ufunc_T *copy_func(char_u *lambda, char_u *global); int funcdepth_increment(void); void funcdepth_decrement(void); int funcdepth_get(void); --- 13,19 ---- ufunc_T *find_func(char_u *name, int is_global, cctx_T *cctx); int func_is_global(ufunc_T *ufunc); int func_name_refcount(char_u *name); ! int copy_func(char_u *lambda, char_u *global, ectx_T *ectx); int funcdepth_increment(void); void funcdepth_decrement(void); int funcdepth_get(void); *** ../vim-8.2.2187/src/structs.h 2020-12-21 19:59:04.569197722 +0100 --- src/structs.h 2020-12-22 14:16:02.638799761 +0100 *************** *** 1363,1368 **** --- 1363,1369 ---- typedef struct cbq_S cbq_T; typedef struct channel_S channel_T; typedef struct cctx_S cctx_T; + typedef struct ectx_S ectx_T; typedef enum { *** ../vim-8.2.2187/src/vim9.h 2020-12-21 17:30:46.941668485 +0100 --- src/vim9.h 2020-12-22 16:38:09.527917648 +0100 *************** *** 341,348 **** --- 341,350 ---- */ struct dfunc_S { ufunc_T *df_ufunc; // struct containing most stuff + int df_refcount; // how many ufunc_T point to this dfunc_T int df_idx; // index in def_functions int df_deleted; // if TRUE function was deleted + char_u *df_name; // name used for error messages garray_T df_def_args_isn; // default argument instructions isn_T *df_instr; // function body to be executed *** ../vim-8.2.2187/src/testdir/test_vim9_func.vim 2020-12-22 12:20:04.541293877 +0100 --- src/testdir/test_vim9_func.vim 2020-12-22 16:54:45.500378934 +0100 *************** *** 299,304 **** --- 299,305 ---- Outer() END CheckScriptFailure(lines, "E122:") + delfunc g:Inner lines =<< trim END vim9script *************** *** 1565,1570 **** --- 1566,1590 ---- bwipe! enddef + def Test_global_closure_called_directly() + var lines =<< trim END + vim9script + def Outer() + var x = 1 + def g:Inner() + var y = x + x += 1 + assert_equal(1, y) + enddef + g:Inner() + assert_equal(2, x) + enddef + Outer() + END + CheckScriptSuccess(lines) + delfunc g:Inner + enddef + def Test_failure_in_called_function() # this was using the frame index as the return value var lines =<< trim END *** ../vim-8.2.2187/src/version.c 2020-12-22 12:50:07.368223959 +0100 --- src/version.c 2020-12-22 13:58:14.464067615 +0100 *************** *** 752,753 **** --- 752,755 ---- { /* Add new patch number below this line */ + /**/ + 2188, /**/ -- You have heard the saying that if you put a thousand monkeys in a room with a thousand typewriters and waited long enough, eventually you would have a room full of dead monkeys. (Scott Adams - The Dilbert principle) /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///