Patch 8.2.2967

[Bram Moolenaar]

Patch 8.2.2967
Problem: Vim9: crash when using two levels of partials.
Solution: Add outer_ref_T and use it in the execution context.
Files: src/structs.h, src/vim9execute.c, src/testdir/test_vim9_func.vim


*** ../vim-8.2.2966/src/structs.h 2021-05-26 22:32:06.254066656 +0200
--- src/structs.h 2021-06-09 17:48:01.565325109 +0200
***************
*** 1995,2001 ****
garray_T *out_stack; // stack from outer scope
int out_frame_idx; // index of stack frame in out_stack
outer_T *out_up; // outer scope of outer scope or NULL
! int out_up_is_copy; // don't free out_up
};

struct partial_S
--- 1995,2001 ----
garray_T *out_stack; // stack from outer scope
int out_frame_idx; // index of stack frame in out_stack
outer_T *out_up; // outer scope of outer scope or NULL
! partial_T *out_up_partial; // partial owning out_up or NULL
};

struct partial_S
*** ../vim-8.2.2966/src/vim9execute.c 2021-06-06 17:34:10.324945725 +0200
--- src/vim9execute.c 2021-06-09 19:23:50.661691083 +0200
***************
*** 43,48 ****
--- 43,56 ----
int floc_restore_cmdmod_stacklen;
} funclocal_T;

+ // Structure to hold a reference to an outer_T, with information of whether it
+ // was allocated.
+ typedef struct {
+ outer_T *or_outer;
+ partial_T *or_partial; // decrement "or_partial->pt_refcount" later
+ int or_outer_allocated; // free "or_outer" later
+ } outer_ref_T;
+
// A stack is used to store:
// - arguments passed to a :def function
// - info about the calling function, to use when returning
***************
*** 70,76 ****
int ec_frame_idx; // index in ec_stack: context of ec_dfunc_idx
int ec_initial_frame_idx; // frame index when called

! outer_T *ec_outer; // outer scope used for closures, allocated
funclocal_T ec_funclocal;

garray_T ec_trystack; // stack of trycmd_T values
--- 78,84 ----
int ec_frame_idx; // index in ec_stack: context of ec_dfunc_idx
int ec_initial_frame_idx; // frame index when called

! outer_ref_T *ec_outer_ref; // outer scope used for closures, allocated
funclocal_T ec_funclocal;

garray_T ec_trystack; // stack of trycmd_T values
***************
*** 143,149 ****
* Call compiled function "cdf_idx" from compiled code.
* This adds a stack frame and sets the instruction pointer to the start of the
* called function.
! * If "pt" is not null use "pt->pt_outer" for ec_outer.
*
* Stack has:
* - current arguments (already there)
--- 151,157 ----
* Call compiled function "cdf_idx" from compiled code.
* This adds a stack frame and sets the instruction pointer to the start of the
* called function.
! * If "pt" is not null use "pt->pt_outer" for ec_outer_ref->or_outer.
*
* Stack has:
* - current arguments (already there)
***************
*** 280,286 ****
STACK_TV_BOT(STACK_FRAME_FUNC_OFF)->vval.v_number = ectx->ec_dfunc_idx;
STACK_TV_BOT(STACK_FRAME_IIDX_OFF)->vval.v_number = ectx->ec_iidx;
STACK_TV_BOT(STACK_FRAME_INSTR_OFF)->vval.v_string = (void *)ectx->ec_instr;
! STACK_TV_BOT(STACK_FRAME_OUTER_OFF)->vval.v_string = (void *)ectx->ec_outer;
STACK_TV_BOT(STACK_FRAME_FUNCLOCAL_OFF)->vval.v_string = (void *)floc;
STACK_TV_BOT(STACK_FRAME_IDX_OFF)->vval.v_number = ectx->ec_frame_idx;
ectx->ec_frame_idx = ectx->ec_stack.ga_len;
--- 288,295 ----
STACK_TV_BOT(STACK_FRAME_FUNC_OFF)->vval.v_number = ectx->ec_dfunc_idx;
STACK_TV_BOT(STACK_FRAME_IIDX_OFF)->vval.v_number = ectx->ec_iidx;
STACK_TV_BOT(STACK_FRAME_INSTR_OFF)->vval.v_string = (void *)ectx->ec_instr;
! STACK_TV_BOT(STACK_FRAME_OUTER_OFF)->vval.v_string =
! (void *)ectx->ec_outer_ref;
STACK_TV_BOT(STACK_FRAME_FUNCLOCAL_OFF)->vval.v_string = (void *)floc;
STACK_TV_BOT(STACK_FRAME_IDX_OFF)->vval.v_number = ectx->ec_frame_idx;
ectx->ec_frame_idx = ectx->ec_stack.ga_len;
***************
*** 300,329 ****
if (pt != NULL || ufunc->uf_partial != NULL
|| (ufunc->uf_flags & FC_CLOSURE))
{
! outer_T *outer = ALLOC_CLEAR_ONE(outer_T);

! if (outer == NULL)
return FAIL;
if (pt != NULL)
{
! *outer = pt->pt_outer;
! outer->out_up_is_copy = TRUE;
}
else if (ufunc->uf_partial != NULL)
{
! *outer = ufunc->uf_partial->pt_outer;
! outer->out_up_is_copy = TRUE;
}
else
{
! outer->out_stack = &ectx->ec_stack;
! outer->out_frame_idx = ectx->ec_frame_idx;
! outer->out_up = ectx->ec_outer;
}
! ectx->ec_outer = outer;
}
else
! ectx->ec_outer = NULL;

++ufunc->uf_calls;

--- 309,348 ----
if (pt != NULL || ufunc->uf_partial != NULL
|| (ufunc->uf_flags & FC_CLOSURE))
{
! outer_ref_T *ref = ALLOC_CLEAR_ONE(outer_ref_T);

! if (ref == NULL)
return FAIL;
if (pt != NULL)
{
! ref->or_outer = &pt->pt_outer;
! ++pt->pt_refcount;
! ref->or_partial = pt;
}
else if (ufunc->uf_partial != NULL)
{
! ref->or_outer = &ufunc->uf_partial->pt_outer;
! ++ufunc->uf_partial->pt_refcount;
! ref->or_partial = ufunc->uf_partial;
}
else
{
! ref->or_outer = ALLOC_CLEAR_ONE(outer_T);
! if (ref->or_outer == NULL)
! {
! vim_free(ref);
! return FAIL;
! }
! ref->or_outer_allocated = TRUE;
! ref->or_outer->out_stack = &ectx->ec_stack;
! ref->or_outer->out_frame_idx = ectx->ec_frame_idx;
! if (ectx->ec_outer_ref != NULL)
! ref->or_outer->out_up = ectx->ec_outer_ref->or_outer;
}
! ectx->ec_outer_ref = ref;
}
else
! ectx->ec_outer_ref = NULL;

++ufunc->uf_calls;

***************
*** 476,482 ****
pt->pt_funcstack = funcstack;
pt->pt_outer.out_stack = &funcstack->fs_ga;
pt->pt_outer.out_frame_idx = ectx->ec_frame_idx - top;
- pt->pt_outer.out_up = ectx->ec_outer;
}
}
}
--- 495,500 ----
***************
*** 587,593 ****
if (ret_idx == ectx->ec_frame_idx + STACK_FRAME_IDX_OFF)
ret_idx = 0;

! vim_free(ectx->ec_outer);

// Restore the previous frame.
ectx->ec_dfunc_idx = prev_dfunc_idx;
--- 605,617 ----
if (ret_idx == ectx->ec_frame_idx + STACK_FRAME_IDX_OFF)
ret_idx = 0;

! if (ectx->ec_outer_ref != NULL)
! {
! if (ectx->ec_outer_ref->or_outer_allocated)
! vim_free(ectx->ec_outer_ref->or_outer);
! partial_unref(ectx->ec_outer_ref->or_partial);
! vim_free(ectx->ec_outer_ref);
! }

// Restore the previous frame.
ectx->ec_dfunc_idx = prev_dfunc_idx;
***************
*** 595,601 ****
+ STACK_FRAME_IIDX_OFF)->vval.v_number;
ectx->ec_instr = (void *)STACK_TV(ectx->ec_frame_idx
+ STACK_FRAME_INSTR_OFF)->vval.v_string;
! ectx->ec_outer = (void *)STACK_TV(ectx->ec_frame_idx
+ STACK_FRAME_OUTER_OFF)->vval.v_string;
floc = (void *)STACK_TV(ectx->ec_frame_idx
+ STACK_FRAME_FUNCLOCAL_OFF)->vval.v_string;
--- 619,625 ----
+ STACK_FRAME_IIDX_OFF)->vval.v_number;
ectx->ec_instr = (void *)STACK_TV(ectx->ec_frame_idx
+ STACK_FRAME_INSTR_OFF)->vval.v_string;
! ectx->ec_outer_ref = (void *)STACK_TV(ectx->ec_frame_idx
+ STACK_FRAME_OUTER_OFF)->vval.v_string;
floc = (void *)STACK_TV(ectx->ec_frame_idx
+ STACK_FRAME_FUNCLOCAL_OFF)->vval.v_string;
***************
*** 696,702 ****
* If the function is compiled this will add a stack frame and set the
* instruction pointer at the start of the function.
* Otherwise the function is called here.
! * If "pt" is not null use "pt->pt_outer" for ec_outer.
* "iptr" can be used to replace the instruction with a more efficient one.
*/
static int
--- 720,726 ----
* If the function is compiled this will add a stack frame and set the
* instruction pointer at the start of the function.
* Otherwise the function is called here.
! * If "pt" is not null use "pt->pt_outer" for ec_outer_ref->or_outer.
* "iptr" can be used to replace the instruction with a more efficient one.
*/
static int
***************
*** 1295,1318 ****
dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data)
+ ectx->ec_dfunc_idx;

! // The closure needs to find arguments and local
! // variables in the current stack.
pt->pt_outer.out_stack = &ectx->ec_stack;
pt->pt_outer.out_frame_idx = ectx->ec_frame_idx;
! pt->pt_outer.out_up = ectx->ec_outer;
! pt->pt_outer.out_up_is_copy = TRUE;

! // If this function returns and the closure is still
! // being used, we need to make a copy of the context
! // (arguments and local variables). Store a reference
! // to the partial so we can handle that.
if (ga_grow(&ectx->ec_funcrefs, 1) == FAIL)
{
vim_free(pt);
return FAIL;
}
! // Extra variable keeps the count of closures created
! // in the current function call.
++(((typval_T *)ectx->ec_stack.ga_data) + ectx->ec_frame_idx
+ STACK_FRAME_SIZE + dfunc->df_varcount)->vval.v_number;

--- 1319,1349 ----
dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data)
+ ectx->ec_dfunc_idx;

! // The closure may need to find arguments and local variables in the
! // current stack.
pt->pt_outer.out_stack = &ectx->ec_stack;
pt->pt_outer.out_frame_idx = ectx->ec_frame_idx;
! if (ectx->ec_outer_ref != NULL)
! {
! // The current context already has a context, link to that one.
! pt->pt_outer.out_up = ectx->ec_outer_ref->or_outer;
! if (ectx->ec_outer_ref->or_partial != NULL)
! {
! pt->pt_outer.out_up_partial = ectx->ec_outer_ref->or_partial;
! ++pt->pt_outer.out_up_partial->pt_refcount;
! }
! }

! // If this function returns and the closure is still being used, we
! // need to make a copy of the context (arguments and local variables).
! // Store a reference to the partial so we can handle that.
if (ga_grow(&ectx->ec_funcrefs, 1) == FAIL)
{
vim_free(pt);
return FAIL;
}
! // Extra variable keeps the count of closures created in the current
! // function call.
++(((typval_T *)ectx->ec_stack.ga_data) + ectx->ec_frame_idx
+ STACK_FRAME_SIZE + dfunc->df_varcount)->vval.v_number;

***************
*** 2355,2361 ****
case ISN_STOREOUTER:
{
int depth = iptr->isn_arg.outer.outer_depth;
! outer_T *outer = ectx->ec_outer;

while (depth > 1 && outer != NULL)
{
--- 2386,2393 ----
case ISN_STOREOUTER:
{
int depth = iptr->isn_arg.outer.outer_depth;
! outer_T *outer = ectx->ec_outer_ref == NULL ? NULL
! : ectx->ec_outer_ref->or_outer;

while (depth > 1 && outer != NULL)
{
***************
*** 2774,2780 ****
}
break;

! // push a function reference to a compiled function
case ISN_FUNCREF:
{
partial_T *pt = ALLOC_CLEAR_ONE(partial_T);
--- 2806,2812 ----
}
break;

! // push a partial, a reference to a compiled function
case ISN_FUNCREF:
{
partial_T *pt = ALLOC_CLEAR_ONE(partial_T);
***************
*** 2791,2797 ****
if (fill_partial_and_closure(pt, pt_dfunc->df_ufunc,
ectx) == FAIL)
goto theend;
-
tv = STACK_TV_BOT(0);
++ectx->ec_stack.ga_len;
tv->vval.v_partial = pt;
--- 2823,2828 ----
***************
*** 4384,4405 ****
// by copy_func().
if (partial != NULL || base_ufunc->uf_partial != NULL)
{
! ectx.ec_outer = ALLOC_CLEAR_ONE(outer_T);
! if (ectx.ec_outer == NULL)
goto failed_early;
if (partial != NULL)
{
if (partial->pt_outer.out_stack == NULL && current_ectx != NULL)
{
! if (current_ectx->ec_outer != NULL)
! *ectx.ec_outer = *current_ectx->ec_outer;
}
else
! *ectx.ec_outer = partial->pt_outer;
}
else
! *ectx.ec_outer = base_ufunc->uf_partial->pt_outer;
! ectx.ec_outer->out_up_is_copy = TRUE;
}
}

--- 4415,4445 ----
// by copy_func().
if (partial != NULL || base_ufunc->uf_partial != NULL)
{
! ectx.ec_outer_ref = ALLOC_CLEAR_ONE(outer_ref_T);
! if (ectx.ec_outer_ref == NULL)
goto failed_early;
if (partial != NULL)
{
if (partial->pt_outer.out_stack == NULL && current_ectx != NULL)
{
! if (current_ectx->ec_outer_ref != NULL
! && current_ectx->ec_outer_ref->or_outer != NULL)
! ectx.ec_outer_ref->or_outer =
! current_ectx->ec_outer_ref->or_outer;
}
else
! {
! ectx.ec_outer_ref->or_outer = &partial->pt_outer;
! ++partial->pt_refcount;
! ectx.ec_outer_ref->or_partial = partial;
! }
}
else
! {
! ectx.ec_outer_ref->or_outer = &base_ufunc->uf_partial->pt_outer;
! ++base_ufunc->uf_partial->pt_refcount;
! ectx.ec_outer_ref->or_partial = base_ufunc->uf_partial;
! }
}
}

***************
*** 4516,4529 ****

vim_free(ectx.ec_stack.ga_data);
vim_free(ectx.ec_trystack.ga_data);
!
! while (ectx.ec_outer != NULL)
{
! outer_T *up = ectx.ec_outer->out_up_is_copy
! ? NULL : ectx.ec_outer->out_up;
!
! vim_free(ectx.ec_outer);
! ectx.ec_outer = up;
}

// Not sure if this is necessary.
--- 4556,4567 ----

vim_free(ectx.ec_stack.ga_data);
vim_free(ectx.ec_trystack.ga_data);
! if (ectx.ec_outer_ref != NULL)
{
! if (ectx.ec_outer_ref->or_outer_allocated)
! vim_free(ectx.ec_outer_ref->or_outer);
! partial_unref(ectx.ec_outer_ref->or_partial);
! vim_free(ectx.ec_outer_ref);
}

// Not sure if this is necessary.
*** ../vim-8.2.2966/src/testdir/test_vim9_func.vim 2021-06-08 22:01:48.754571742 +0200
--- src/testdir/test_vim9_func.vim 2021-06-09 19:26:22.685387099 +0200
***************
*** 2128,2133 ****
--- 2128,2146 ----
CheckScriptSuccess(lines)
enddef

+ def Test_double_nested_lambda()
+ var lines =<< trim END
+ vim9script
+ def F(head: string): func(string): func(string): string
+ return (sep: string): func(string): string => ((tail: string): string => {
+ return head .. sep .. tail
+ })
+ enddef
+ assert_equal('hello-there', F('hello')('-')('there'))
+ END
+ CheckScriptSuccess(lines)
+ enddef
+
def Test_nested_inline_lambda()
# TODO: use the "text" argument
var lines =<< trim END
*** ../vim-8.2.2966/src/version.c 2021-06-09 12:33:36.908213351 +0200
--- src/version.c 2021-06-09 16:39:18.905392009 +0200
***************
*** 752,753 ****
--- 752,755 ----
{ /* Add new patch number below this line */
+ /**/
+ 2967,
/**/

--
CART DRIVER: Bring out your dead!
There are legs stick out of windows and doors. Two MEN are fighting in the
mud - covered from head to foot in it. Another MAN is on his hands in
knees shovelling mud into his mouth. We just catch sight of a MAN falling
into a well.
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///