To: vim-dev@vim.org

Subject: Patch 7.2.070

Fcc: outbox

From: Bram Moolenaar <Bram@moolenaar.net>

Mime-Version: 1.0

Content-Type: text/plain; charset=ISO-8859-1

Content-Transfer-Encoding: 8bit

------------

Patch 7.2.070

Problem:    Crash when a function returns a:000. (Matt Wozkiski)

Solution:   Don't put the function struct on the stack, allocate it.  Free it

	    only when nothing in it is used.

Files:	    src/eval.c

*** ../vim-7.2.069/src/eval.c	Tue Dec  9 10:56:50 2008

--- src/eval.c	Wed Dec 17 21:32:26 2008

***************

*** 32,37 ****

--- 32,40 ----

  #define DICT_MAXNEST 100	/* maximum nesting of lists and dicts */

+ #define DO_NOT_FREE_CNT 99999	/* refcount for dict or list that should not

+ 				   be freed. */

+ 

  /*

   * In a hashtab item "hi_key" points to "di_key" in a dictitem.

   * This avoids adding a pointer to the hashtab item.

***************

*** 789,794 ****

--- 792,799 ----

  static void func_unref __ARGS((char_u *name));

  static void func_ref __ARGS((char_u *name));

  static void call_user_func __ARGS((ufunc_T *fp, int argcount, typval_T *argvars, typval_T *rettv, linenr_T firstline, linenr_T lastline, dict_T *selfdict));

+ static int can_free_funccal __ARGS((funccall_T *fc, int copyID)) ;

+ static void free_funccal __ARGS((funccall_T *fc, int free_val));

  static void add_nr_var __ARGS((dict_T *dp, dictitem_T *v, char *name, varnumber_T nr));

  static win_T *find_win_by_nr __ARGS((typval_T *vp, tabpage_T *tp));

  static void getwinvar __ARGS((typval_T *argvars, typval_T *rettv, int off));

***************

*** 923,928 ****

--- 928,937 ----

  /* pointer to funccal for currently active function */

  funccall_T *current_funccal = NULL;

+ /* pointer to list of previously used funccal, still around because some

+  * item in it is still being used. */

+ funccall_T *previous_funccal = NULL;

+ 

  /*

   * Return TRUE when a function was ended by a ":return" command.

   */

***************

*** 6490,6496 ****

      buf_T	*buf;

      win_T	*wp;

      int		i;

!     funccall_T	*fc;

      int		did_free = FALSE;

  #ifdef FEAT_WINDOWS

      tabpage_T	*tp;

--- 6499,6505 ----

      buf_T	*buf;

      win_T	*wp;

      int		i;

!     funccall_T	*fc, **pfc;

      int		did_free = FALSE;

  #ifdef FEAT_WINDOWS

      tabpage_T	*tp;

***************

*** 6574,6579 ****

--- 6583,6602 ----

  	else

  	    ll = ll->lv_used_next;

+     /* check if any funccal can be freed now */

+     for (pfc = &previous_funccal; *pfc != NULL; )

+     {

+ 	if (can_free_funccal(*pfc, copyID))

+ 	{

+ 	    fc = *pfc;

+ 	    *pfc = fc->caller;

+ 	    free_funccal(fc, TRUE);

+ 	    did_free = TRUE;

+ 	}

+ 	else

+ 	    pfc = &(*pfc)->caller;

+     }

+ 

      return did_free;

  }

***************

*** 18962,18968 ****

      dictitem_T	*dict_var;

  {

      hash_init(&dict->dv_hashtab);

!     dict->dv_refcount = 99999;

      dict_var->di_tv.vval.v_dict = dict;

      dict_var->di_tv.v_type = VAR_DICT;

      dict_var->di_tv.v_lock = VAR_FIXED;

--- 18985,18991 ----

      dictitem_T	*dict_var;

  {

      hash_init(&dict->dv_hashtab);

!     dict->dv_refcount = DO_NOT_FREE_CNT;

      dict_var->di_tv.vval.v_dict = dict;

      dict_var->di_tv.v_type = VAR_DICT;

      dict_var->di_tv.v_lock = VAR_FIXED;

***************

*** 19299,19304 ****

--- 19322,19329 ----

   * Copy the values from typval_T "from" to typval_T "to".

   * When needed allocates string or increases reference count.

   * Does not make a copy of a list or dict but copies the reference!

+  * It is OK for "from" and "to" to point to the same item.  This is used to

+  * make a copy later.

   */

      static void

  copy_tv(from, to)

***************

*** 21111,21117 ****

      char_u	*save_sourcing_name;

      linenr_T	save_sourcing_lnum;

      scid_T	save_current_SID;

!     funccall_T	fc;

      int		save_did_emsg;

      static int	depth = 0;

      dictitem_T	*v;

--- 21136,21142 ----

      char_u	*save_sourcing_name;

      linenr_T	save_sourcing_lnum;

      scid_T	save_current_SID;

!     funccall_T	*fc;

      int		save_did_emsg;

      static int	depth = 0;

      dictitem_T	*v;

***************

*** 21137,21172 ****

      line_breakcheck();		/* check for CTRL-C hit */

!     fc.caller = current_funccal;

!     current_funccal = &fc;

!     fc.func = fp;

!     fc.rettv = rettv;

      rettv->vval.v_number = 0;

!     fc.linenr = 0;

!     fc.returned = FALSE;

!     fc.level = ex_nesting_level;

      /* Check if this function has a breakpoint. */

!     fc.breakpoint = dbg_find_breakpoint(FALSE, fp->uf_name, (linenr_T)0);

!     fc.dbg_tick = debug_tick;

      /*

!      * Note about using fc.fixvar[]: This is an array of FIXVAR_CNT variables

       * with names up to VAR_SHORT_LEN long.  This avoids having to alloc/free

       * each argument variable and saves a lot of time.

       */

      /*

       * Init l: variables.

       */

!     init_var_dict(&fc.l_vars, &fc.l_vars_var);

      if (selfdict != NULL)

      {

  	/* Set l:self to "selfdict".  Use "name" to avoid a warning from

  	 * some compiler that checks the destination size. */

! 	v = &fc.fixvar[fixvar_idx++].var;

  	name = v->di_key;

  	STRCPY(name, "self");

  	v->di_flags = DI_FLAGS_RO + DI_FLAGS_FIX;

! 	hash_add(&fc.l_vars.dv_hashtab, DI2HIKEY(v));

  	v->di_tv.v_type = VAR_DICT;

  	v->di_tv.v_lock = 0;

  	v->di_tv.vval.v_dict = selfdict;

--- 21162,21198 ----

      line_breakcheck();		/* check for CTRL-C hit */

!     fc = (funccall_T *)alloc(sizeof(funccall_T));

!     fc->caller = current_funccal;

!     current_funccal = fc;

!     fc->func = fp;

!     fc->rettv = rettv;

      rettv->vval.v_number = 0;

!     fc->linenr = 0;

!     fc->returned = FALSE;

!     fc->level = ex_nesting_level;

      /* Check if this function has a breakpoint. */

!     fc->breakpoint = dbg_find_breakpoint(FALSE, fp->uf_name, (linenr_T)0);

!     fc->dbg_tick = debug_tick;

      /*

!      * Note about using fc->fixvar[]: This is an array of FIXVAR_CNT variables

       * with names up to VAR_SHORT_LEN long.  This avoids having to alloc/free

       * each argument variable and saves a lot of time.

       */

      /*

       * Init l: variables.

       */

!     init_var_dict(&fc->l_vars, &fc->l_vars_var);

      if (selfdict != NULL)

      {

  	/* Set l:self to "selfdict".  Use "name" to avoid a warning from

  	 * some compiler that checks the destination size. */

! 	v = &fc->fixvar[fixvar_idx++].var;

  	name = v->di_key;

  	STRCPY(name, "self");

  	v->di_flags = DI_FLAGS_RO + DI_FLAGS_FIX;

! 	hash_add(&fc->l_vars.dv_hashtab, DI2HIKEY(v));

  	v->di_tv.v_type = VAR_DICT;

  	v->di_tv.v_lock = 0;

  	v->di_tv.vval.v_dict = selfdict;

***************

*** 21178,21208 ****

       * Set a:0 to "argcount".

       * Set a:000 to a list with room for the "..." arguments.

       */

!     init_var_dict(&fc.l_avars, &fc.l_avars_var);

!     add_nr_var(&fc.l_avars, &fc.fixvar[fixvar_idx++].var, "0",

  				(varnumber_T)(argcount - fp->uf_args.ga_len));

      /* Use "name" to avoid a warning from some compiler that checks the

       * destination size. */

!     v = &fc.fixvar[fixvar_idx++].var;

      name = v->di_key;

      STRCPY(name, "000");

      v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;

!     hash_add(&fc.l_avars.dv_hashtab, DI2HIKEY(v));

      v->di_tv.v_type = VAR_LIST;

      v->di_tv.v_lock = VAR_FIXED;

!     v->di_tv.vval.v_list = &fc.l_varlist;

!     vim_memset(&fc.l_varlist, 0, sizeof(list_T));

!     fc.l_varlist.lv_refcount = 99999;

!     fc.l_varlist.lv_lock = VAR_FIXED;

      /*

       * Set a:firstline to "firstline" and a:lastline to "lastline".

       * Set a:name to named arguments.

       * Set a:N to the "..." arguments.

       */

!     add_nr_var(&fc.l_avars, &fc.fixvar[fixvar_idx++].var, "firstline",

  						      (varnumber_T)firstline);

!     add_nr_var(&fc.l_avars, &fc.fixvar[fixvar_idx++].var, "lastline",

  						       (varnumber_T)lastline);

      for (i = 0; i < argcount; ++i)

      {

--- 21204,21234 ----

       * Set a:0 to "argcount".

       * Set a:000 to a list with room for the "..." arguments.

       */

!     init_var_dict(&fc->l_avars, &fc->l_avars_var);

!     add_nr_var(&fc->l_avars, &fc->fixvar[fixvar_idx++].var, "0",

  				(varnumber_T)(argcount - fp->uf_args.ga_len));

      /* Use "name" to avoid a warning from some compiler that checks the

       * destination size. */

!     v = &fc->fixvar[fixvar_idx++].var;

      name = v->di_key;

      STRCPY(name, "000");

      v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;

!     hash_add(&fc->l_avars.dv_hashtab, DI2HIKEY(v));

      v->di_tv.v_type = VAR_LIST;

      v->di_tv.v_lock = VAR_FIXED;

!     v->di_tv.vval.v_list = &fc->l_varlist;

!     vim_memset(&fc->l_varlist, 0, sizeof(list_T));

!     fc->l_varlist.lv_refcount = DO_NOT_FREE_CNT;

!     fc->l_varlist.lv_lock = VAR_FIXED;

      /*

       * Set a:firstline to "firstline" and a:lastline to "lastline".

       * Set a:name to named arguments.

       * Set a:N to the "..." arguments.

       */

!     add_nr_var(&fc->l_avars, &fc->fixvar[fixvar_idx++].var, "firstline",

  						      (varnumber_T)firstline);

!     add_nr_var(&fc->l_avars, &fc->fixvar[fixvar_idx++].var, "lastline",

  						       (varnumber_T)lastline);

      for (i = 0; i < argcount; ++i)

      {

***************

*** 21218,21224 ****

  	}

  	if (fixvar_idx < FIXVAR_CNT && STRLEN(name) <= VAR_SHORT_LEN)

  	{

! 	    v = &fc.fixvar[fixvar_idx++].var;

  	    v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;

  	}

  	else

--- 21244,21250 ----

  	}

  	if (fixvar_idx < FIXVAR_CNT && STRLEN(name) <= VAR_SHORT_LEN)

  	{

! 	    v = &fc->fixvar[fixvar_idx++].var;

  	    v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;

  	}

  	else

***************

*** 21230,21236 ****

  	    v->di_flags = DI_FLAGS_RO;

  	}

  	STRCPY(v->di_key, name);

! 	hash_add(&fc.l_avars.dv_hashtab, DI2HIKEY(v));

  	/* Note: the values are copied directly to avoid alloc/free.

  	 * "argvars" must have VAR_FIXED for v_lock. */

--- 21256,21262 ----

  	    v->di_flags = DI_FLAGS_RO;

  	}

  	STRCPY(v->di_key, name);

! 	hash_add(&fc->l_avars.dv_hashtab, DI2HIKEY(v));

  	/* Note: the values are copied directly to avoid alloc/free.

  	 * "argvars" must have VAR_FIXED for v_lock. */

***************

*** 21239,21247 ****

  	if (ai >= 0 && ai < MAX_FUNC_ARGS)

  	{

! 	    list_append(&fc.l_varlist, &fc.l_listitems[ai]);

! 	    fc.l_listitems[ai].li_tv = argvars[i];

! 	    fc.l_listitems[ai].li_tv.v_lock = VAR_FIXED;

  	}

      }

--- 21265,21273 ----

  	if (ai >= 0 && ai < MAX_FUNC_ARGS)

  	{

! 	    list_append(&fc->l_varlist, &fc->l_listitems[ai]);

! 	    fc->l_listitems[ai].li_tv = argvars[i];

! 	    fc->l_listitems[ai].li_tv.v_lock = VAR_FIXED;

  	}

      }

***************

*** 21306,21312 ****

  	if (!fp->uf_profiling && has_profiling(FALSE, fp->uf_name, NULL))

  	    func_do_profile(fp);

  	if (fp->uf_profiling

! 		       || (fc.caller != NULL && fc.caller->func->uf_profiling))

  	{

  	    ++fp->uf_tm_count;

  	    profile_start(&call_start);

--- 21332,21338 ----

  	if (!fp->uf_profiling && has_profiling(FALSE, fp->uf_name, NULL))

  	    func_do_profile(fp);

  	if (fp->uf_profiling

! 		    || (fc->caller != NULL && fc->caller->func->uf_profiling))

  	{

  	    ++fp->uf_tm_count;

  	    profile_start(&call_start);

***************

*** 21322,21328 ****

      did_emsg = FALSE;

      /* call do_cmdline() to execute the lines */

!     do_cmdline(NULL, get_func_line, (void *)&fc,

  				     DOCMD_NOWAIT|DOCMD_VERBOSE|DOCMD_REPEAT);

      --RedrawingDisabled;

--- 21348,21354 ----

      did_emsg = FALSE;

      /* call do_cmdline() to execute the lines */

!     do_cmdline(NULL, get_func_line, (void *)fc,

  				     DOCMD_NOWAIT|DOCMD_VERBOSE|DOCMD_REPEAT);

      --RedrawingDisabled;

***************

*** 21337,21352 ****

  #ifdef FEAT_PROFILE

      if (do_profiling == PROF_YES && (fp->uf_profiling

! 		    || (fc.caller != NULL && fc.caller->func->uf_profiling)))

      {

  	profile_end(&call_start);

  	profile_sub_wait(&wait_start, &call_start);

  	profile_add(&fp->uf_tm_total, &call_start);

  	profile_self(&fp->uf_tm_self, &call_start, &fp->uf_tm_children);

! 	if (fc.caller != NULL && fc.caller->func->uf_profiling)

  	{

! 	    profile_add(&fc.caller->func->uf_tm_children, &call_start);

! 	    profile_add(&fc.caller->func->uf_tml_children, &call_start);

  	}

      }

  #endif

--- 21363,21378 ----

  #ifdef FEAT_PROFILE

      if (do_profiling == PROF_YES && (fp->uf_profiling

! 		    || (fc->caller != NULL && fc->caller->func->uf_profiling)))

      {

  	profile_end(&call_start);

  	profile_sub_wait(&wait_start, &call_start);

  	profile_add(&fp->uf_tm_total, &call_start);

  	profile_self(&fp->uf_tm_self, &call_start, &fp->uf_tm_children);

! 	if (fc->caller != NULL && fc->caller->func->uf_profiling)

  	{

! 	    profile_add(&fc->caller->func->uf_tm_children, &call_start);

! 	    profile_add(&fc->caller->func->uf_tml_children, &call_start);

  	}

      }

  #endif

***************

*** 21359,21367 ****

  	if (aborting())

  	    smsg((char_u *)_("%s aborted"), sourcing_name);

! 	else if (fc.rettv->v_type == VAR_NUMBER)

  	    smsg((char_u *)_("%s returning #%ld"), sourcing_name,

! 					       (long)fc.rettv->vval.v_number);

  	else

  	{

  	    char_u	buf[MSG_BUF_LEN];

--- 21385,21393 ----

  	if (aborting())

  	    smsg((char_u *)_("%s aborted"), sourcing_name);

! 	else if (fc->rettv->v_type == VAR_NUMBER)

  	    smsg((char_u *)_("%s returning #%ld"), sourcing_name,

! 					       (long)fc->rettv->vval.v_number);

  	else

  	{

  	    char_u	buf[MSG_BUF_LEN];

***************

*** 21372,21378 ****

  	    /* The value may be very long.  Skip the middle part, so that we

  	     * have some idea how it starts and ends. smsg() would always

  	     * truncate it at the end. */

! 	    s = tv2string(fc.rettv, &tofree, numbuf2, 0);

  	    if (s != NULL)

  	    {

  		trunc_string(s, buf, MSG_BUF_CLEN);

--- 21398,21404 ----

  	    /* The value may be very long.  Skip the middle part, so that we

  	     * have some idea how it starts and ends. smsg() would always

  	     * truncate it at the end. */

! 	    s = tv2string(fc->rettv, &tofree, numbuf2, 0);

  	    if (s != NULL)

  	    {

  		trunc_string(s, buf, MSG_BUF_CLEN);

***************

*** 21408,21421 ****

      }

      did_emsg |= save_did_emsg;

!     current_funccal = fc.caller;

!     /* The a: variables typevals were not allocated, only free the allocated

!      * variables. */

!     vars_clear_ext(&fc.l_avars.dv_hashtab, FALSE);

!     vars_clear(&fc.l_vars.dv_hashtab);		/* free all l: variables */

!     --depth;

  }

  /*

--- 21434,21517 ----

      }

      did_emsg |= save_did_emsg;

!     current_funccal = fc->caller;

!     --depth;

!     /* if the a:000 list and the a: dict are not referenced we can free the

!      * funccall_T and what's in it. */

!     if (fc->l_varlist.lv_refcount == DO_NOT_FREE_CNT

! 	    && fc->l_vars.dv_refcount == DO_NOT_FREE_CNT

! 	    && fc->l_avars.dv_refcount == DO_NOT_FREE_CNT)

!     {

! 	free_funccal(fc, FALSE);

!     }

!     else

!     {

! 	hashitem_T	*hi;

! 	listitem_T	*li;

! 	int		todo;

! 	/* "fc" is still in use.  This can happen when returning "a:000" or

! 	 * assigning "l:" to a global variable.

! 	 * Link "fc" in the list for garbage collection later. */

! 	fc->caller = previous_funccal;

! 	previous_funccal = fc;

! 

! 	/* Make a copy of the a: variables, since we didn't do that above. */

! 	todo = (int)fc->l_avars.dv_hashtab.ht_used;

! 	for (hi = fc->l_avars.dv_hashtab.ht_array; todo > 0; ++hi)

! 	{

! 	    if (!HASHITEM_EMPTY(hi))

! 	    {

! 		--todo;

! 		v = HI2DI(hi);

! 		copy_tv(&v->di_tv, &v->di_tv);

! 	    }

! 	}

! 

! 	/* Make a copy of the a:000 items, since we didn't do that above. */

! 	for (li = fc->l_varlist.lv_first; li != NULL; li = li->li_next)

! 	    copy_tv(&li->li_tv, &li->li_tv);

!     }

! }

! 

! /*

!  * Return TRUE if items in "fc" do not have "copyID".  That means they are not

!  * referenced from anywyere.

!  */

!     static int

! can_free_funccal(fc, copyID)

!     funccall_T	*fc;

!     int		copyID;

! {

!     return (fc->l_varlist.lv_copyID != copyID

! 	    && fc->l_vars.dv_copyID != copyID

! 	    && fc->l_avars.dv_copyID != copyID);

! }

! 

! /*

!  * Free "fc" and what it contains.

!  */

!    static void

! free_funccal(fc, free_val)

!     funccall_T	*fc;

!     int		free_val;  /* a: vars were allocated */

! {

!     listitem_T	*li;

! 

!     /* The a: variables typevals may not have been allocated, only free the

!      * allocated variables. */

!     vars_clear_ext(&fc->l_avars.dv_hashtab, free_val);

! 

!     /* free all l: variables */

!     vars_clear(&fc->l_vars.dv_hashtab);

! 

!     /* Free the a:000 variables if they were allocated. */

!     if (free_val)

! 	for (li = fc->l_varlist.lv_first; li != NULL; li = li->li_next)

! 	    clear_tv(&li->li_tv);

! 

!     vim_free(fc);

  }

  /*

*** ../vim-7.2.069/src/version.c	Tue Dec  9 22:34:02 2008

--- src/version.c	Sun Dec 21 12:47:07 2008

***************

*** 678,679 ****

--- 678,681 ----

  {   /* Add new patch number below this line */

+ /**/

+     70,

  /**/

-- 

Close your shells, or I'll kill -9 you

Tomorrow I'll quota you

Remember the disks'll always be full

And then while I'm away

I'll write ~ everyday

And I'll send-pr all my buggings to you.

    [ CVS log "Beatles style" for FreeBSD ports/INDEX, Satoshi Asami ]

 /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net   \\\

///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\

\\\        download, build and distribute -- http://www.A-A-P.org        ///

 \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///