To: vim_dev@googlegroups.com
Subject: Patch 8.2.2967
Fcc: outbox
From: Bram Moolenaar <Bram@moolenaar.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
------------

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 -- Bram@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    ///