patch 8.2.2188: Vim9: crash when calling global function from :def function
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)
diff --git a/src/proto/userfunc.pro b/src/proto/userfunc.pro
index c79c101..da4a880 100644
--- a/src/proto/userfunc.pro
+++ b/src/proto/userfunc.pro
@@ -13,7 +13,7 @@
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 copy_func(char_u *lambda, char_u *global, ectx_T *ectx);
int funcdepth_increment(void);
void funcdepth_decrement(void);
int funcdepth_get(void);
diff --git a/src/proto/vim9compile.pro b/src/proto/vim9compile.pro
index a19088b..3f8ae13 100644
--- a/src/proto/vim9compile.pro
+++ b/src/proto/vim9compile.pro
@@ -18,7 +18,7 @@
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 link_def_function(ufunc_T *ufunc);
void free_def_functions(void);
/* vim: set ft=c : */
diff --git a/src/proto/vim9execute.pro b/src/proto/vim9execute.pro
index 882456d..8e948c5 100644
--- a/src/proto/vim9execute.pro
+++ b/src/proto/vim9execute.pro
@@ -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);
diff --git a/src/structs.h b/src/structs.h
index 64d903b..14f3827 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -1363,6 +1363,7 @@
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
{
diff --git a/src/testdir/test_vim9_func.vim b/src/testdir/test_vim9_func.vim
index fcfe41f..015fe3d 100644
--- a/src/testdir/test_vim9_func.vim
+++ b/src/testdir/test_vim9_func.vim
@@ -299,6 +299,7 @@
Outer()
END
CheckScriptFailure(lines, "E122:")
+ delfunc g:Inner
lines =<< trim END
vim9script
@@ -1565,6 +1566,25 @@
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
diff --git a/src/userfunc.c b/src/userfunc.c
index f7ad9f3..ff19afd 100644
--- a/src/userfunc.c
+++ b/src/userfunc.c
@@ -1260,8 +1260,7 @@
// 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);
+ unlink_def_function(fp);
}
/*
@@ -1307,75 +1306,98 @@
/*
* 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)
+ 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);
- 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));
+ semsg(_(e_lambda_function_not_found_str), lambda);
+ return FAIL;
}
- return fp;
+
+ // 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 NULL;
+ return FAIL;
}
static int funcdepth = 0;
@@ -3515,7 +3537,7 @@
fp->uf_profiling = FALSE;
fp->uf_prof_initialized = FALSE;
#endif
- clear_def_function(fp);
+ unlink_def_function(fp);
}
}
}
diff --git a/src/version.c b/src/version.c
index 8aaa0b2..d6eaf8d 100644
--- a/src/version.c
+++ b/src/version.c
@@ -751,6 +751,8 @@
static int included_patches[] =
{ /* Add new patch number below this line */
/**/
+ 2188,
+/**/
2187,
/**/
2186,
diff --git a/src/vim9.h b/src/vim9.h
index 5ced180..9048d23 100644
--- a/src/vim9.h
+++ b/src/vim9.h
@@ -341,8 +341,10 @@
*/
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
diff --git a/src/vim9compile.c b/src/vim9compile.c
index 2a305ee..8e07968 100644
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -7336,6 +7336,8 @@
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,6 +7930,7 @@
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,9 +8105,7 @@
if (ufunc != NULL)
{
- // Clear uf_dfunc_idx so that the function is deleted.
- clear_def_function(ufunc);
- ufunc->uf_dfunc_idx = 0;
+ unlink_def_function(ufunc);
func_ptr_unref(ufunc);
}
@@ -8206,7 +8207,7 @@
}
/*
- * Free all instructions for "dfunc".
+ * Free all instructions for "dfunc" except df_name.
*/
static void
delete_def_function_contents(dfunc_T *dfunc)
@@ -8227,31 +8228,39 @@
/*
* When a user function is deleted, clear the contents of any associated def
- * function. The position in def_functions can be re-used.
+ * function, unless another user function still uses it.
+ * The position in def_functions can be re-used.
*/
void
-clear_def_function(ufunc_T *ufunc)
+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;
- delete_def_function_contents(dfunc);
+ 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 is about to be deleted: remove the pointer to it.
- * The entry in def_functions is then unused.
+ * Used when a user function refers to an existing dfunc.
*/
void
-unlink_def_function(ufunc_T *ufunc)
+link_def_function(ufunc_T *ufunc)
{
- dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + ufunc->uf_dfunc_idx;
+ if (ufunc->uf_dfunc_idx > 0)
+ {
+ dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data)
+ + ufunc->uf_dfunc_idx;
- dfunc->df_ufunc = NULL;
+ ++dfunc->df_refcount;
+ }
}
#if defined(EXITFREE) || defined(PROTO)
@@ -8268,6 +8277,7 @@
dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + idx;
delete_def_function_contents(dfunc);
+ vim_free(dfunc->df_name);
}
ga_clear(&def_functions);
diff --git a/src/vim9execute.c b/src/vim9execute.c
index 7d56e21..9636cbf 100644
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -54,7 +54,7 @@
/*
* Execution context.
*/
-typedef struct {
+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,7 +69,7 @@
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))
@@ -173,7 +173,9 @@
if (dfunc->df_deleted)
{
- emsg_funcname(e_func_deleted, ufunc->uf_name);
+ // 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,6 +262,12 @@
}
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,6 +626,7 @@
// 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,7 +899,7 @@
* When a function reference is used, fill a partial with the information
* needed, especially when it is used as a closure.
*/
- static int
+ int
fill_partial_and_closure(partial_T *pt, ufunc_T *ufunc, ectx_T *ectx)
{
pt->pt_func = ufunc;
@@ -2120,25 +2129,10 @@
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,
+ if (copy_func(newfunc->nf_lambda, newfunc->nf_global,
&ectx) == FAIL)
- goto failed;
- new_ufunc->uf_partial = pt;
- --pt->pt_refcount; // not referenced here
- }
+ goto failed;
}
break;