To: vim-dev@vim.org Subject: Patch 7.2.188 Fcc: outbox From: Bram Moolenaar 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 ///