Patch 8.2.3809

[Bram Moolenaar]

Patch 8.2.3809
Problem: Vim9: crash when garbage collecting a nested partial. (Virginia
Senioria)
Solution: Set references in all the funcstacks. (closes #9348)
Files: src/vim9execute.c, src/proto/vim9execute.pro, src/structs.h,
src/eval.c, src/testdir/test_vim9_func.vim


*** ../vim-8.2.3808/src/vim9execute.c 2021-12-14 14:29:12.053734514 +0000
--- src/vim9execute.c 2021-12-14 17:58:36.178986528 +0000
***************
*** 468,473 ****
--- 468,498 ----
// Get pointer to item in the stack.
#define STACK_TV(idx) (((typval_T *)ectx->ec_stack.ga_data) + idx)

+ // Double linked list of funcstack_T in use.
+ static funcstack_T *first_funcstack = NULL;
+
+ static void
+ add_funcstack_to_list(funcstack_T *funcstack)
+ {
+ // Link in list of funcstacks.
+ if (first_funcstack != NULL)
+ first_funcstack->fs_prev = funcstack;
+ funcstack->fs_next = first_funcstack;
+ funcstack->fs_prev = NULL;
+ first_funcstack = funcstack;
+ }
+
+ static void
+ remove_funcstack_from_list(funcstack_T *funcstack)
+ {
+ if (funcstack->fs_prev == NULL)
+ first_funcstack = funcstack->fs_next;
+ else
+ funcstack->fs_prev->fs_next = funcstack->fs_next;
+ if (funcstack->fs_next != NULL)
+ funcstack->fs_next->fs_prev = funcstack->fs_prev;
+ }
+
/*
* Used when returning from a function: Check if any closure is still
* referenced. If so then move the arguments and variables to a separate piece
***************
*** 540,545 ****
--- 565,571 ----
// Move them to the called function.
if (funcstack == NULL)
return FAIL;
+
funcstack->fs_var_offset = argcount + STACK_FRAME_SIZE;
funcstack->fs_ga.ga_len = funcstack->fs_var_offset + dfunc->df_varcount;
stack = ALLOC_CLEAR_MULT(typval_T, funcstack->fs_ga.ga_len);
***************
*** 549,554 ****
--- 575,581 ----
vim_free(funcstack);
return FAIL;
}
+ add_funcstack_to_list(funcstack);

// Move or copy the arguments.
for (idx = 0; idx < argcount; ++idx)
***************
*** 571,577 ****
// local variable, has a reference count for the variable, thus
// will never go down to zero. When all these refcounts are one
// then the funcstack is unused. We need to count how many we have
! // so we need when to check.
if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial != NULL)
{
int i;
--- 598,604 ----
// local variable, has a reference count for the variable, thus
// will never go down to zero. When all these refcounts are one
// then the funcstack is unused. We need to count how many we have
! // so we know when to check.
if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial != NULL)
{
int i;
***************
*** 643,653 ****
--- 670,703 ----
for (i = 0; i < gap->ga_len; ++i)
clear_tv(stack + i);
vim_free(stack);
+ remove_funcstack_from_list(funcstack);
vim_free(funcstack);
}
}

/*
+ * For garbage collecting: set references in all variables referenced by
+ * all funcstacks.
+ */
+ int
+ set_ref_in_funcstacks(int copyID)
+ {
+ funcstack_T *funcstack;
+
+ for (funcstack = first_funcstack; funcstack != NULL;
+ funcstack = funcstack->fs_next)
+ {
+ typval_T *stack = funcstack->fs_ga.ga_data;
+ int i;
+
+ for (i = 0; i < funcstack->fs_ga.ga_len; ++i)
+ if (set_ref_in_item(stack + i, copyID, NULL, NULL))
+ return TRUE; // abort
+ }
+ return FALSE;
+ }
+
+ /*
* Return from the current function.
*/
static int
*** ../vim-8.2.3808/src/proto/vim9execute.pro 2021-09-02 17:49:02.744932328 +0100
--- src/proto/vim9execute.pro 2021-12-14 17:47:09.295704754 +0000
***************
*** 1,6 ****
--- 1,7 ----
/* vim9execute.c */
void to_string_error(vartype_T vartype);
void funcstack_check_refcount(funcstack_T *funcstack);
+ int set_ref_in_funcstacks(int copyID);
char_u *char_from_string(char_u *str, varnumber_T index);
char_u *string_slice(char_u *str, varnumber_T first, varnumber_T last, int exclusive);
int fill_partial_and_closure(partial_T *pt, ufunc_T *ufunc, ectx_T *ectx);
*** ../vim-8.2.3808/src/structs.h 2021-12-13 14:26:40.992627753 +0000
--- src/structs.h 2021-12-14 17:46:09.915820923 +0000
***************
*** 2009,2016 ****
* Structure to hold the context of a compiled function, used by closures
* defined in that function.
*/
! typedef struct funcstack_S
{
garray_T fs_ga; // contains the stack, with:
// - arguments
// - frame
--- 2009,2021 ----
* Structure to hold the context of a compiled function, used by closures
* defined in that function.
*/
! typedef struct funcstack_S funcstack_T;
!
! struct funcstack_S
{
+ funcstack_T *fs_next; // linked list at "first_funcstack"
+ funcstack_T *fs_prev;
+
garray_T fs_ga; // contains the stack, with:
// - arguments
// - frame
***************
*** 2021,2027 ****
int fs_refcount; // nr of closures referencing this funcstack
int fs_min_refcount; // nr of closures on this funcstack
int fs_copyID; // for garray_T collection
! } funcstack_T;

typedef struct outer_S outer_T;
struct outer_S {
--- 2026,2032 ----
int fs_refcount; // nr of closures referencing this funcstack
int fs_min_refcount; // nr of closures on this funcstack
int fs_copyID; // for garray_T collection
! };

typedef struct outer_S outer_T;
struct outer_S {
*** ../vim-8.2.3808/src/eval.c 2021-12-14 12:06:12.533925687 +0000
--- src/eval.c 2021-12-14 17:44:20.900034455 +0000
***************
*** 4510,4515 ****
--- 4510,4518 ----
// function call arguments, if v:testing is set.
abort = abort || set_ref_in_func_args(copyID);

+ // funcstacks keep variables for closures
+ abort = abort || set_ref_in_funcstacks(copyID);
+
// v: vars
abort = abort || garbage_collect_vimvars(copyID);

***************
*** 4869,4883 ****
for (i = 0; i < pt->pt_argc; ++i)
abort = abort || set_ref_in_item(&pt->pt_argv[i], copyID,
ht_stack, list_stack);
! if (pt->pt_funcstack != NULL)
! {
! typval_T *stack = pt->pt_funcstack->fs_ga.ga_data;
!
! for (i = 0; i < pt->pt_funcstack->fs_ga.ga_len; ++i)
! abort = abort || set_ref_in_item(stack + i, copyID,
! ht_stack, list_stack);
! }
!
}
}
#ifdef FEAT_JOB_CHANNEL
--- 4872,4878 ----
for (i = 0; i < pt->pt_argc; ++i)
abort = abort || set_ref_in_item(&pt->pt_argv[i], copyID,
ht_stack, list_stack);
! // pt_funcstack is handled in set_ref_in_funcstacks()
}
}
#ifdef FEAT_JOB_CHANNEL
*** ../vim-8.2.3808/src/testdir/test_vim9_func.vim 2021-12-13 18:14:11.760881287 +0000
--- src/testdir/test_vim9_func.vim 2021-12-14 18:11:57.841770846 +0000
***************
*** 1903,1908 ****
--- 1903,1943 ----
delete('XToDelFunc')
enddef

+ func Test_free_dict_while_in_funcstack()
+ " relies on the sleep command
+ CheckUnix
+ call Run_Test_free_dict_while_in_funcstack()
+ endfunc
+
+ def Run_Test_free_dict_while_in_funcstack()
+
+ # this was freeing the TermRun() default argument dictionary while it was
+ # still referenced in a funcstack_T
+ var lines =<< trim END
+ vim9script
+
+ &updatetime = 400
+ def TermRun(_ = {})
+ def Post()
+ enddef
+ def Exec()
+ term_start('sleep 1', {
+ term_finish: 'close',
+ exit_cb: (_, _) => Post(),
+ })
+ enddef
+ Exec()
+ enddef
+ nnoremap <F4> <Cmd>call <SID>TermRun()<CR>
+ timer_start(100, (_) => feedkeys("\<F4>"))
+ timer_start(1000, (_) => feedkeys("\<F4>"))
+ sleep 1500m
+ END
+ CheckScriptSuccess(lines)
+ nunmap <F4>
+ set updatetime&
+ enddef
+
def Test_redef_failure()
writefile(['def Func0(): string', 'return "Func0"', 'enddef'], 'Xdef')
so Xdef
*** ../vim-8.2.3808/src/version.c 2021-12-14 14:29:12.057734501 +0000
--- src/version.c 2021-12-14 18:12:54.005677305 +0000
***************
*** 751,752 ****
--- 751,754 ----
{ /* Add new patch number below this line */
+ /**/
+ 3809,
/**/

--
hundred-and-one symptoms of being an internet addict:
32. You don't know what sex three of your closest friends are, because they
have neutral nicknames and you never bothered to ask.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///