To: vim-dev@vim.org
Subject: Patch 7.2.188
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.2.188
Problem: Crash with specific use of function calls. (Meikel Brandmeyer)
Solution: Make sure the items referenced by a function call are not freed
twice. (based on patch from Nico Weber)
Files: src/eval.c
*** ../vim-7.2.187/src/eval.c 2009-05-16 17:29:37.000000000 +0200
--- src/eval.c 2009-05-22 20:04:22.000000000 +0200
***************
*** 129,136 ****
--- 129,139 ----
/*
* When recursively copying lists and dicts we need to remember which ones we
* have done to avoid endless recursiveness. This unique ID is used for that.
+ * The last bit is used for previous_funccal, ignored when comparing.
*/
static int current_copyID = 0;
+ #define COPYID_INC 2
+ #define COPYID_MASK (~0x1)
/*
* Array to hold the hashtab with variables local to each sourced script.
***************
*** 439,444 ****
--- 442,448 ----
static void list_remove __ARGS((list_T *l, listitem_T *item, listitem_T *item2));
static char_u *list2string __ARGS((typval_T *tv, int copyID));
static int list_join __ARGS((garray_T *gap, list_T *l, char_u *sep, int echo, int copyID));
+ static int free_unref_items __ARGS((int copyID));
static void set_ref_in_ht __ARGS((hashtab_T *ht, int copyID));
static void set_ref_in_list __ARGS((list_T *l, int copyID));
static void set_ref_in_item __ARGS((typval_T *tv, int copyID));
***************
*** 6494,6507 ****
int
garbage_collect()
{
! dict_T *dd;
! list_T *ll;
! int copyID = ++current_copyID;
buf_T *buf;
win_T *wp;
int i;
funccall_T *fc, **pfc;
! int did_free = FALSE;
#ifdef FEAT_WINDOWS
tabpage_T *tp;
#endif
--- 6498,6510 ----
int
garbage_collect()
{
! int copyID;
buf_T *buf;
win_T *wp;
int i;
funccall_T *fc, **pfc;
! int did_free;
! int did_free_funccal = FALSE;
#ifdef FEAT_WINDOWS
tabpage_T *tp;
#endif
***************
*** 6511,6520 ****
--- 6514,6538 ----
may_garbage_collect = FALSE;
garbage_collect_at_exit = FALSE;
+ /* We advance by two because we add one for items referenced through
+ * previous_funccal. */
+ current_copyID += COPYID_INC;
+ copyID = current_copyID;
+
/*
* 1. Go through all accessible variables and mark all lists and dicts
* with copyID.
*/
+
+ /* Don't free variables in the previous_funccal list unless they are only
+ * referenced through previous_funccal. This must be first, because if
+ * the item is referenced elsewhere it must not be freed. */
+ for (fc = previous_funccal; fc != NULL; fc = fc->caller)
+ {
+ set_ref_in_ht(&fc->l_vars.dv_hashtab, copyID + 1);
+ set_ref_in_ht(&fc->l_avars.dv_hashtab, copyID + 1);
+ }
+
/* script-local variables */
for (i = 1; i <= ga_scripts.ga_len; ++i)
set_ref_in_ht(&SCRIPT_VARS(i), copyID);
***************
*** 6546,6556 ****
/* v: vars */
set_ref_in_ht(&vimvarht, copyID);
/*
! * 2. Go through the list of dicts and free items without the copyID.
*/
for (dd = first_dict; dd != NULL; )
! if (dd->dv_copyID != copyID)
{
/* Free the Dictionary and ordinary items it contains, but don't
* recurse into Lists and Dictionaries, they will be in the list
--- 6564,6610 ----
/* v: vars */
set_ref_in_ht(&vimvarht, copyID);
+ /* Free lists and dictionaries that are not referenced. */
+ did_free = free_unref_items(copyID);
+
+ /* 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;
+ did_free_funccal = TRUE;
+ }
+ else
+ pfc = &(*pfc)->caller;
+ }
+ if (did_free_funccal)
+ /* When a funccal was freed some more items might be garbage
+ * collected, so run again. */
+ (void)garbage_collect();
+
+ return did_free;
+ }
+
+ /*
+ * Free lists and dictionaries that are no longer referenced.
+ */
+ static int
+ free_unref_items(copyID)
+ int copyID;
+ {
+ dict_T *dd;
+ list_T *ll;
+ int did_free = FALSE;
+
/*
! * Go through the list of dicts and free items without the copyID.
*/
for (dd = first_dict; dd != NULL; )
! if ((dd->dv_copyID & COPYID_MASK) != (copyID & COPYID_MASK))
{
/* Free the Dictionary and ordinary items it contains, but don't
* recurse into Lists and Dictionaries, they will be in the list
***************
*** 6565,6576 ****
dd = dd->dv_used_next;
/*
! * 3. Go through the list of lists and free items without the copyID.
! * But don't free a list that has a watcher (used in a for loop), these
! * are not referenced anywhere.
*/
for (ll = first_list; ll != NULL; )
! if (ll->lv_copyID != copyID && ll->lv_watch == NULL)
{
/* Free the List and ordinary items it contains, but don't recurse
* into Lists and Dictionaries, they will be in the list of dicts
--- 6619,6631 ----
dd = dd->dv_used_next;
/*
! * Go through the list of lists and free items without the copyID.
! * But don't free a list that has a watcher (used in a for loop), these
! * are not referenced anywhere.
*/
for (ll = first_list; ll != NULL; )
! if ((ll->lv_copyID & COPYID_MASK) != (copyID & COPYID_MASK)
! && ll->lv_watch == NULL)
{
/* Free the List and ordinary items it contains, but don't recurse
* into Lists and Dictionaries, they will be in the list of dicts
***************
*** 6584,6603 ****
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;
}
--- 6639,6644 ----
***************
*** 18842,18847 ****
--- 18883,18889 ----
{
hash_init(&dict->dv_hashtab);
dict->dv_refcount = DO_NOT_FREE_CNT;
+ dict->dv_copyID = 0;
dict_var->di_tv.vval.v_dict = dict;
dict_var->di_tv.v_type = VAR_DICT;
dict_var->di_tv.v_lock = VAR_FIXED;
***************
*** 21294,21301 ****
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)
--- 21336,21343 ----
current_funccal = fc->caller;
--depth;
! /* If the a:000 list and the l: and a: dicts 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)
***************
*** 21334,21340 ****
/*
* Return TRUE if items in "fc" do not have "copyID". That means they are not
! * referenced from anywhere.
*/
static int
can_free_funccal(fc, copyID)
--- 21376,21382 ----
/*
* Return TRUE if items in "fc" do not have "copyID". That means they are not
! * referenced from anywhere that is in use.
*/
static int
can_free_funccal(fc, copyID)
*** ../vim-7.2.187/src/version.c 2009-05-23 14:27:43.000000000 +0200
--- src/version.c 2009-05-24 13:20:49.000000000 +0200
***************
*** 678,679 ****
--- 678,681 ----
{ /* Add new patch number below this line */
+ /**/
+ 188,
/**/
--
ARTHUR: ... and I am your king ....
OLD WOMAN: Ooooh! I didn't know we had a king. I thought we were an
autonomous collective ...
"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/ \\\
\\\ download, build and distribute -- http://www.A-A-P.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///