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

Patch 7.4.1715
Problem:    Double free when a partial is in a cycle with a list or dict.
            (Nikolai Pavlov)
Solution:   Do not free a nested list or dict.
Files:      src/eval.c, src/testdir/test_partial.vim


*** ../vim-7.4.1714/src/eval.c	2016-04-03 22:44:32.403625720 +0200
--- src/eval.c	2016-04-06 22:48:53.971029556 +0200
***************
*** 5929,5934 ****
--- 5929,5985 ----
      return OK;
  }
  
+     static void
+ partial_free(partial_T *pt, int recursive)
+ {
+     int i;
+ 
+     for (i = 0; i < pt->pt_argc; ++i)
+     {
+ 	typval_T *tv = &pt->pt_argv[i];
+ 
+ 	if (recursive || (tv->v_type != VAR_DICT && tv->v_type != VAR_LIST))
+ 	    clear_tv(tv);
+     }
+     vim_free(pt->pt_argv);
+     if (recursive)
+ 	dict_unref(pt->pt_dict);
+     func_unref(pt->pt_name);
+     vim_free(pt->pt_name);
+     vim_free(pt);
+ }
+ 
+ /*
+  * Unreference a closure: decrement the reference count and free it when it
+  * becomes zero.
+  */
+     void
+ partial_unref(partial_T *pt)
+ {
+     if (pt != NULL && --pt->pt_refcount <= 0)
+ 	partial_free(pt, TRUE);
+ }
+ 
+ /*
+  * Like clear_tv(), but do not free lists or dictionaries.
+  * This is when called via free_unref_items().
+  */
+     static void
+ clear_tv_no_recurse(typval_T *tv)
+ {
+     if (tv->v_type == VAR_PARTIAL)
+     {
+ 	partial_T *pt = tv->vval.v_partial;
+ 
+ 	/* We unref the partial but not the dict or any list it
+ 	 * refers to. */
+ 	if (pt != NULL && --pt->pt_refcount == 0)
+ 	    partial_free(pt, FALSE);
+     }
+     else if (tv->v_type != VAR_LIST && tv->v_type != VAR_DICT)
+ 	clear_tv(tv);
+ }
+ 
  /*
   * Allocate a variable for a List and fill it from "*arg".
   * Return OK or FAIL.
***************
*** 6070,6078 ****
      {
  	/* Remove the item before deleting it. */
  	l->lv_first = item->li_next;
! 	if (recurse || (item->li_tv.v_type != VAR_LIST
! 					   && item->li_tv.v_type != VAR_DICT))
  	    clear_tv(&item->li_tv);
  	vim_free(item);
      }
      vim_free(l);
--- 6121,6130 ----
      {
  	/* Remove the item before deleting it. */
  	l->lv_first = item->li_next;
! 	if (recurse)
  	    clear_tv(&item->li_tv);
+ 	else
+ 	    clear_tv_no_recurse(&item->li_tv);
  	vim_free(item);
      }
      vim_free(l);
***************
*** 7185,7190 ****
--- 7237,7252 ----
  		}
  	    }
  	}
+ 	if (tv->v_type == VAR_PARTIAL)
+ 	{
+ 	    partial_T	*pt = tv->vval.v_partial;
+ 	    int		i;
+ 
+ 	    if (pt != NULL)
+ 		for (i = 0; i < pt->pt_argc; ++i)
+ 		    set_ref_in_item(&pt->pt_argv[i], copyID,
+ 							ht_stack, list_stack);
+ 	}
      }
      else if (tv->v_type == VAR_LIST)
      {
***************
*** 7215,7246 ****
      return abort;
  }
  
-     static void
- partial_free(partial_T *pt, int free_dict)
- {
-     int i;
- 
-     for (i = 0; i < pt->pt_argc; ++i)
- 	clear_tv(&pt->pt_argv[i]);
-     vim_free(pt->pt_argv);
-     if (free_dict)
- 	dict_unref(pt->pt_dict);
-     func_unref(pt->pt_name);
-     vim_free(pt->pt_name);
-     vim_free(pt);
- }
- 
- /*
-  * Unreference a closure: decrement the reference count and free it when it
-  * becomes zero.
-  */
-     void
- partial_unref(partial_T *pt)
- {
-     if (pt != NULL && --pt->pt_refcount <= 0)
- 	partial_free(pt, TRUE);
- }
- 
  /*
   * Allocate an empty header for a dictionary.
   */
--- 7277,7282 ----
***************
*** 7331,7350 ****
  	     * something recursive causing trouble. */
  	    di = HI2DI(hi);
  	    hash_remove(&d->dv_hashtab, hi);
! 	    if (recurse || (di->di_tv.v_type != VAR_LIST
! 					     && di->di_tv.v_type != VAR_DICT))
! 	    {
! 		if (!recurse && di->di_tv.v_type == VAR_PARTIAL)
! 		{
! 		    partial_T *pt = di->di_tv.vval.v_partial;
! 
! 		    /* We unref the partial but not the dict it refers to. */
! 		    if (pt != NULL && --pt->pt_refcount == 0)
! 			partial_free(pt, FALSE);
! 		}
! 		else
! 		    clear_tv(&di->di_tv);
! 	    }
  	    vim_free(di);
  	    --todo;
  	}
--- 7367,7376 ----
  	     * something recursive causing trouble. */
  	    di = HI2DI(hi);
  	    hash_remove(&d->dv_hashtab, hi);
! 	    if (recurse)
! 		clear_tv(&di->di_tv);
! 	    else
! 		clear_tv_no_recurse(&di->di_tv);
  	    vim_free(di);
  	    --todo;
  	}
*** ../vim-7.4.1714/src/testdir/test_partial.vim	2016-03-24 21:58:06.940204253 +0100
--- src/testdir/test_partial.vim	2016-04-06 22:28:06.495210598 +0200
***************
*** 220,222 ****
--- 220,240 ----
      endtry
    endif
  endfunc
+ 
+ " This causes double free on exit if EXITFREE is defined.
+ func Test_cyclic_list_arg()
+   let l = []
+   let Pt = function('string', [l])
+   call add(l, Pt)
+   unlet l
+   unlet Pt
+ endfunc
+ 
+ " This causes double free on exit if EXITFREE is defined.
+ func Test_cyclic_dict_arg()
+   let d = {}
+   let Pt = function('string', [d])
+   let d.Pt = Pt
+   unlet d
+   unlet Pt
+ endfunc
*** ../vim-7.4.1714/src/version.c	2016-04-05 22:06:25.223395130 +0200
--- src/version.c	2016-04-06 22:45:13.665589830 +0200
***************
*** 750,751 ****
--- 750,753 ----
  {   /* Add new patch number below this line */
+ /**/
+     1715,
  /**/

-- 
You can tune a file system, but you can't tuna fish
							-- man tunefs

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