| 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 |
| |
| |
| |
| |
| |
| *** 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) |
| |
| |
| |
| *** 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 /// |