Patch 8.2.0419

[Bram Moolenaar]

Patch 8.2.0419
Problem: Various memory leaks in Vim9 script code.
Solution: Fix the leaks. (Ozaki Kiichi, closes #5814)
Files: src/proto/vim9compile.pro, src/scriptfile.c, src/structs.h,
src/testdir/test_vim9_script.vim, src/vim9.h, src/vim9compile.c,
src/vim9execute.c, src/vim9script.c


*** ../vim-8.2.0418/src/proto/vim9compile.pro 2020-03-09 19:25:21.208186448 +0100
--- src/proto/vim9compile.pro 2020-03-20 18:26:23.847779599 +0100
***************
*** 9,14 ****
--- 9,15 ----
char_u *to_name_const_end(char_u *arg);
int assignment_len(char_u *p, int *heredoc);
void compile_def_function(ufunc_T *ufunc, int set_return_type);
+ void delete_instr(isn_T *isn);
void delete_def_function(ufunc_T *ufunc);
void free_def_functions(void);
/* vim: set ft=c : */
*** ../vim-8.2.0418/src/scriptfile.c 2020-03-18 15:23:10.983695087 +0100
--- src/scriptfile.c 2020-03-20 18:26:23.847779599 +0100
***************
*** 1526,1531 ****
--- 1526,1532 ----
vim_free(si->sn_vars);

vim_free(si->sn_name);
+ free_imports(i);
free_string_option(si->sn_save_cpo);
# ifdef FEAT_PROFILE
ga_clear(&si->sn_prl_ga);
*** ../vim-8.2.0418/src/structs.h 2020-02-26 16:15:31.076386941 +0100
--- src/structs.h 2020-03-20 18:26:23.847779599 +0100
***************
*** 1308,1313 ****
--- 1308,1314 ----
int cb_free_name; // cb_name was allocated
} callback_T;

+ typedef struct isn_S isn_T; // instruction
typedef struct dfunc_S dfunc_T; // :def function

typedef struct jobvar_S job_T;
*** ../vim-8.2.0418/src/testdir/test_vim9_script.vim 2020-03-09 19:25:21.208186448 +0100
--- src/testdir/test_vim9_script.vim 2020-03-20 18:26:23.847779599 +0100
***************
*** 942,947 ****
--- 942,957 ----
assert_equal('1_3_', result)
enddef

+ def Test_interrupt_loop()
+ let x = 0
+ while 1
+ x += 1
+ if x == 100
+ feedkeys("\<C-C>", 'L')
+ endif
+ endwhile
+ enddef
+
def Test_substitute_cmd()
new
setline(1, 'something')
***************
*** 964,967 ****
--- 974,997 ----
delete('Xvim9lines')
enddef

+ def Test_redef_failure()
+ call writefile(['def Func0(): string', 'return "Func0"', 'enddef'], 'Xdef')
+ so Xdef
+ call writefile(['def Func1(): string', 'return "Func1"', 'enddef'], 'Xdef')
+ so Xdef
+ call writefile(['def! Func0(): string', 'enddef'], 'Xdef')
+ call assert_fails('so Xdef', 'E1027:')
+ call writefile(['def Func2(): string', 'return "Func2"', 'enddef'], 'Xdef')
+ so Xdef
+ call delete('Xdef')
+
+ call assert_equal(0, Func0())
+ call assert_equal('Func1', Func1())
+ call assert_equal('Func2', Func2())
+
+ delfunc! Func0
+ delfunc! Func1
+ delfunc! Func2
+ enddef
+
" vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker
*** ../vim-8.2.0418/src/vim9.h 2020-03-04 22:20:23.362900855 +0100
--- src/vim9.h 2020-03-20 18:26:23.847779599 +0100
***************
*** 203,209 ****
/*
* Instruction
*/
! typedef struct {
isntype_T isn_type;
int isn_lnum;
union {
--- 203,209 ----
/*
* Instruction
*/
! struct isn_S {
isntype_T isn_type;
int isn_lnum;
union {
***************
*** 231,237 ****
loadstore_T loadstore;
script_T script;
} isn_arg;
! } isn_T;

/*
* Info about a function defined with :def. Used in "def_functions".
--- 231,237 ----
loadstore_T loadstore;
script_T script;
} isn_arg;
! };

/*
* Info about a function defined with :def. Used in "def_functions".
*** ../vim-8.2.0418/src/vim9compile.c 2020-03-09 19:25:21.208186448 +0100
--- src/vim9compile.c 2020-03-20 18:32:31.294616471 +0100
***************
*** 129,134 ****
--- 129,135 ----
static int compile_expr1(char_u **arg, cctx_T *cctx);
static int compile_expr2(char_u **arg, cctx_T *cctx);
static int compile_expr3(char_u **arg, cctx_T *cctx);
+ static void delete_def_function_contents(dfunc_T *dfunc);

/*
* Lookup variable "name" in the local scope and return the index.
***************
*** 1306,1311 ****
--- 1307,1342 ----
}

/*
+ * Remove local variables above "new_top".
+ */
+ static void
+ unwind_locals(cctx_T *cctx, int new_top)
+ {
+ if (cctx->ctx_locals.ga_len > new_top)
+ {
+ int idx;
+ lvar_T *lvar;
+
+ for (idx = new_top; idx < cctx->ctx_locals.ga_len; ++idx)
+ {
+ lvar = ((lvar_T *)cctx->ctx_locals.ga_data) + idx;
+ vim_free(lvar->lv_name);
+ }
+ }
+ cctx->ctx_locals.ga_len = new_top;
+ }
+
+ /*
+ * Free all local variables.
+ */
+ static void
+ free_local(cctx_T *cctx)
+ {
+ unwind_locals(cctx, 0);
+ ga_clear(&cctx->ctx_locals);
+ }
+
+ /*
* Skip over a type definition and return a pointer to just after it.
*/
char_u *
***************
*** 1672,1677 ****
--- 1703,1725 ----
}

/*
+ * Free all imported variables.
+ */
+ static void
+ free_imported(cctx_T *cctx)
+ {
+ int idx;
+
+ for (idx = 0; idx < cctx->ctx_imports.ga_len; ++idx)
+ {
+ imported_T *import = ((imported_T *)cctx->ctx_imports.ga_data) + idx;
+
+ vim_free(import->imp_name);
+ }
+ ga_clear(&cctx->ctx_imports);
+ }
+
+ /*
* Generate an instruction to load script-local variable "name".
*/
static int
***************
*** 2127,2133 ****
--- 2175,2184 ----
// Get the funcref in "rettv".
if (get_lambda_tv(arg, &rettv, TRUE) == FAIL)
return FAIL;
+
ufunc = rettv.vval.v_partial->pt_func;
+ ++ufunc->uf_refcount;
+ clear_tv(&rettv);

// The function will have one line: "return {expr}".
// Compile it into instructions.
***************
*** 2169,2178 ****
return FAIL;
}

- // The function will have one line: "return {expr}".
- // Compile it into instructions.
ufunc = rettv.vval.v_partial->pt_func;
++ufunc->uf_refcount;
compile_def_function(ufunc, TRUE);

// compile the arguments
--- 2220,2231 ----
return FAIL;
}

ufunc = rettv.vval.v_partial->pt_func;
++ufunc->uf_refcount;
+ clear_tv(&rettv);
+
+ // The function will have one line: "return {expr}".
+ // Compile it into instructions.
compile_def_function(ufunc, TRUE);

// compile the arguments
***************
*** 2181,2187 ****
// call the compiled function
ret = generate_CALL(cctx, ufunc, argcount);

- clear_tv(&rettv);
return ret;
}

--- 2234,2239 ----
***************
*** 3398,3404 ****
{
int cc;
long numval;
- char_u *stringval = NULL;

dest = dest_option;
if (cmdidx == CMD_const)
--- 3450,3455 ----
***************
*** 3420,3426 ****
}
cc = *p;
*p = NUL;
! opt_type = get_option_value(arg + 1, &numval, &stringval, opt_flags);
*p = cc;
if (opt_type == -3)
{
--- 3471,3477 ----
}
cc = *p;
*p = NUL;
! opt_type = get_option_value(arg + 1, &numval, NULL, opt_flags);
*p = cc;
if (opt_type == -3)
{
***************
*** 4217,4223 ****
emsg(_(e_elseif_without_if));
return NULL;
}
! cctx->ctx_locals.ga_len = scope->se_local_count;

if (cctx->ctx_skip == MAYBE)
{
--- 4268,4274 ----
emsg(_(e_elseif_without_if));
return NULL;
}
! unwind_locals(cctx, scope->se_local_count);

if (cctx->ctx_skip == MAYBE)
{
***************
*** 4265,4271 ****
emsg(_(e_else_without_if));
return NULL;
}
! cctx->ctx_locals.ga_len = scope->se_local_count;

// jump from previous block to the end, unless the else block is empty
if (cctx->ctx_skip == MAYBE)
--- 4316,4322 ----
emsg(_(e_else_without_if));
return NULL;
}
! unwind_locals(cctx, scope->se_local_count);

// jump from previous block to the end, unless the else block is empty
if (cctx->ctx_skip == MAYBE)
***************
*** 4307,4313 ****
}
ifscope = &scope->se_u.se_if;
cctx->ctx_scope = scope->se_outer;
! cctx->ctx_locals.ga_len = scope->se_local_count;

if (scope->se_u.se_if.is_if_label >= 0)
{
--- 4358,4364 ----
}
ifscope = &scope->se_u.se_if;
cctx->ctx_scope = scope->se_outer;
! unwind_locals(cctx, scope->se_local_count);

if (scope->se_u.se_if.is_if_label >= 0)
{
***************
*** 4435,4441 ****
}
forscope = &scope->se_u.se_for;
cctx->ctx_scope = scope->se_outer;
! cctx->ctx_locals.ga_len = scope->se_local_count;

// At end of ":for" scope jump back to the FOR instruction.
generate_JUMP(cctx, JUMP_ALWAYS, forscope->fs_top_label);
--- 4486,4492 ----
}
forscope = &scope->se_u.se_for;
cctx->ctx_scope = scope->se_outer;
! unwind_locals(cctx, scope->se_local_count);

// At end of ":for" scope jump back to the FOR instruction.
generate_JUMP(cctx, JUMP_ALWAYS, forscope->fs_top_label);
***************
*** 4506,4512 ****
return NULL;
}
cctx->ctx_scope = scope->se_outer;
! cctx->ctx_locals.ga_len = scope->se_local_count;

// At end of ":for" scope jump back to the FOR instruction.
generate_JUMP(cctx, JUMP_ALWAYS, scope->se_u.se_while.ws_top_label);
--- 4557,4563 ----
return NULL;
}
cctx->ctx_scope = scope->se_outer;
! unwind_locals(cctx, scope->se_local_count);

// At end of ":for" scope jump back to the FOR instruction.
generate_JUMP(cctx, JUMP_ALWAYS, scope->se_u.se_while.ws_top_label);
***************
*** 4599,4605 ****
scope_T *scope = cctx->ctx_scope;

cctx->ctx_scope = scope->se_outer;
! cctx->ctx_locals.ga_len = scope->se_local_count;
vim_free(scope);
}

--- 4650,4656 ----
scope_T *scope = cctx->ctx_scope;

cctx->ctx_scope = scope->se_outer;
! unwind_locals(cctx, scope->se_local_count);
vim_free(scope);
}

***************
*** 4942,4950 ****

if (ufunc->uf_dfunc_idx >= 0)
{
! // redefining a function that was compiled before
dfunc = ((dfunc_T *)def_functions.ga_data) + ufunc->uf_dfunc_idx;
! dfunc->df_deleted = FALSE;
}
else
{
--- 4993,5003 ----

if (ufunc->uf_dfunc_idx >= 0)
{
! // Redefining a function that was compiled before.
dfunc = ((dfunc_T *)def_functions.ga_data) + ufunc->uf_dfunc_idx;
!
! // Free old instructions.
! delete_def_function_contents(dfunc);
}
else
{
***************
*** 5305,5310 ****
--- 5358,5364 ----
generate_instr(&cctx, ISN_RETURN);
}

+ dfunc->df_deleted = FALSE;
dfunc->df_instr = instr->ga_data;
dfunc->df_instr_count = instr->ga_len;
dfunc->df_varcount = cctx.ctx_max_local;
***************
*** 5314,5340 ****
erret:
if (ret == FAIL)
{
ga_clear(instr);
ufunc->uf_dfunc_idx = -1;
! --def_functions.ga_len;
if (errormsg != NULL)
emsg(errormsg);
else if (called_emsg == called_emsg_before)
emsg(_("E1028: compile_def_function failed"));
-
- // don't execute this function body
- ufunc->uf_lines.ga_len = 0;
}

current_sctx = save_current_sctx;
ga_clear(&cctx.ctx_type_stack);
- ga_clear(&cctx.ctx_locals);
}

/*
* Delete an instruction, free what it contains.
*/
! static void
delete_instr(isn_T *isn)
{
switch (isn->isn_type)
--- 5368,5402 ----
erret:
if (ret == FAIL)
{
+ int idx;
+
+ for (idx = 0; idx < instr->ga_len; ++idx)
+ delete_instr(((isn_T *)instr->ga_data) + idx);
ga_clear(instr);
+
ufunc->uf_dfunc_idx = -1;
! if (!dfunc->df_deleted)
! --def_functions.ga_len;
!
! // Don't execute this function body.
! ga_clear_strings(&ufunc->uf_lines);
!
if (errormsg != NULL)
emsg(errormsg);
else if (called_emsg == called_emsg_before)
emsg(_("E1028: compile_def_function failed"));
}

current_sctx = save_current_sctx;
+ free_imported(&cctx);
+ free_local(&cctx);
ga_clear(&cctx.ctx_type_stack);
}

/*
* Delete an instruction, free what it contains.
*/
! void
delete_instr(isn_T *isn)
{
switch (isn->isn_type)
***************
*** 5443,5474 ****
}

/*
* When a user function is deleted, delete any associated def function.
*/
void
delete_def_function(ufunc_T *ufunc)
{
- int idx;
-
if (ufunc->uf_dfunc_idx >= 0)
{
dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data)
+ ufunc->uf_dfunc_idx;
- ga_clear(&dfunc->df_def_args_isn);

! for (idx = 0; idx < dfunc->df_instr_count; ++idx)
! delete_instr(dfunc->df_instr + idx);
! VIM_CLEAR(dfunc->df_instr);
!
! dfunc->df_deleted = TRUE;
}
}

#if defined(EXITFREE) || defined(PROTO)
void
free_def_functions(void)
{
! vim_free(def_functions.ga_data);
}
#endif

--- 5505,5561 ----
}

/*
+ * Free all instructions for "dfunc".
+ */
+ static void
+ delete_def_function_contents(dfunc_T *dfunc)
+ {
+ int idx;
+
+ ga_clear(&dfunc->df_def_args_isn);
+
+ if (dfunc->df_instr != NULL)
+ {
+ for (idx = 0; idx < dfunc->df_instr_count; ++idx)
+ delete_instr(dfunc->df_instr + idx);
+ VIM_CLEAR(dfunc->df_instr);
+ }
+
+ dfunc->df_deleted = TRUE;
+ }
+
+ /*
* When a user function is deleted, delete any associated def function.
*/
void
delete_def_function(ufunc_T *ufunc)
{
if (ufunc->uf_dfunc_idx >= 0)
{
dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data)
+ ufunc->uf_dfunc_idx;

! delete_def_function_contents(dfunc);
}
}

#if defined(EXITFREE) || defined(PROTO)
+ /*
+ * Free all functions defined with ":def".
+ */
void
free_def_functions(void)
{
! int idx;
!
! for (idx = 0; idx < def_functions.ga_len; ++idx)
! {
! dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + idx;
!
! delete_def_function_contents(dfunc);
! }
!
! ga_clear(&def_functions);
}
#endif

*** ../vim-8.2.0418/src/vim9execute.c 2020-03-12 19:15:36.212964855 +0100
--- src/vim9execute.c 2020-03-20 18:26:23.851779585 +0100
***************
*** 283,288 ****
--- 283,289 ----
// that was defined later: we can call it directly next time.
if (iptr != NULL)
{
+ delete_instr(iptr);
iptr->isn_type = ISN_DCALL;
iptr->isn_arg.dfunc.cdf_idx = ufunc->uf_dfunc_idx;
iptr->isn_arg.dfunc.cdf_argcount = argcount;
***************
*** 480,490 ****
for (;;)
{
isn_T *iptr;
! trycmd_T *trycmd = NULL;

if (did_throw && !ectx.ec_in_catch)
{
garray_T *trystack = &ectx.ec_trystack;

// An exception jumps to the first catch, finally, or returns from
// the current function.
--- 481,501 ----
for (;;)
{
isn_T *iptr;
!
! veryfast_breakcheck();
! if (got_int)
! {
! // Turn CTRL-C into an exception.
! got_int = FALSE;
! if (throw_exception("Vim:Interrupt", ET_INTERRUPT, NULL) != FAIL)
! goto failed;
! did_throw = TRUE;
! }

if (did_throw && !ectx.ec_in_catch)
{
garray_T *trystack = &ectx.ec_trystack;
+ trycmd_T *trycmd = NULL;

// An exception jumps to the first catch, finally, or returns from
// the current function.
***************
*** 782,789 ****
// store $ENV
case ISN_STOREENV:
--ectx.ec_stack.ga_len;
! vim_setenv_ext(iptr->isn_arg.string,
! tv_get_string(STACK_TV_BOT(0)));
break;

// store @r
--- 793,801 ----
// store $ENV
case ISN_STOREENV:
--ectx.ec_stack.ga_len;
! tv = STACK_TV_BOT(0);
! vim_setenv_ext(iptr->isn_arg.string, tv_get_string(tv));
! clear_tv(tv);
break;

// store @r
***************
*** 1038,1043 ****
--- 1050,1056 ----
case ISN_RETURN:
{
garray_T *trystack = &ectx.ec_trystack;
+ trycmd_T *trycmd = NULL;

if (trystack->ga_len > 0)
trycmd = ((trycmd_T *)trystack->ga_data)
***************
*** 1146,1151 ****
--- 1159,1166 ----
// start of ":try" block
case ISN_TRY:
{
+ trycmd_T *trycmd = NULL;
+
if (ga_grow(&ectx.ec_trystack, 1) == FAIL)
goto failed;
trycmd = ((trycmd_T *)ectx.ec_trystack.ga_data)
***************
*** 1180,1186 ****

if (trystack->ga_len > 0)
{
! trycmd = ((trycmd_T *)trystack->ga_data)
+ trystack->ga_len - 1;
trycmd->tcd_caught = TRUE;
}
--- 1195,1201 ----

if (trystack->ga_len > 0)
{
! trycmd_T *trycmd = ((trycmd_T *)trystack->ga_data)
+ trystack->ga_len - 1;
trycmd->tcd_caught = TRUE;
}
***************
*** 1196,1201 ****
--- 1211,1218 ----

if (trystack->ga_len > 0)
{
+ trycmd_T *trycmd = NULL;
+
--trystack->ga_len;
--trylevel;
trycmd = ((trycmd_T *)trystack->ga_data)
***************
*** 1677,1682 ****
--- 1694,1700 ----
for (idx = 0; idx < ectx.ec_stack.ga_len; ++idx)
clear_tv(STACK_TV(idx));
vim_free(ectx.ec_stack.ga_data);
+ vim_free(ectx.ec_trystack.ga_data);
return ret;
}

*** ../vim-8.2.0418/src/vim9script.c 2020-03-09 19:25:21.208186448 +0100
--- src/vim9script.c 2020-03-20 18:26:23.851779585 +0100
***************
*** 119,129 ****

for (idx = 0; idx < si->sn_imports.ga_len; ++idx)
{
! imported_T *imp = ((imported_T *)si->sn_imports.ga_data + idx);

vim_free(imp->imp_name);
}
ga_clear(&si->sn_imports);
}

/*
--- 119,131 ----

for (idx = 0; idx < si->sn_imports.ga_len; ++idx)
{
! imported_T *imp = ((imported_T *)si->sn_imports.ga_data) + idx;

vim_free(imp->imp_name);
}
ga_clear(&si->sn_imports);
+ ga_clear(&si->sn_var_vals);
+ ga_clear(&si->sn_type_list);
}

/*
*** ../vim-8.2.0418/src/version.c 2020-03-20 18:20:47.084975607 +0100
--- src/version.c 2020-03-20 18:24:29.700185662 +0100
***************
*** 740,741 ****
--- 740,743 ----
{ /* Add new patch number below this line */
+ /**/
+ 419,
/**/

--
You were lucky to have a LAKE! There were a hundred and sixty of
us living in a small shoebox in the middle of the road.

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

[Tom M]

!         if (got_int) 
!         { 
!             // Turn CTRL-C into an exception. 
!             got_int = FALSE; 
!             if (throw_exception("Vim:Interrupt", ET_INTERRUPT, NULL) != FAIL) 
!                 goto failed; 
!             did_throw = TRUE; 
!         } 

This was probably meant to check if "throw_exception() == FAIL" instead of "!= FAIL", wasn't it?

Tom

[Bram Moolenaar]

Good catch. Exception handling is still to be done for Vim9 script, but
this part should work now.

--
LAUNCELOT: At last! A call! A cry of distress ...
(he draws his sword, and turns to CONCORDE)
Concorde! Brave, Concorde ... you shall not have died in vain!
CONCORDE: I'm not quite dead, sir ...
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD