Patch 8.2.2188

[Bram Moolenaar]

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 -- Br...@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 ///