Patch 8.2.5146

[Bram Moolenaar]

Patch 8.2.5146
Problem: Memory leak when substitute expression nests.
Solution: Use an array of expression results.
Files: src/alloc.c, src/regexp.c, src/proto/regexp.pro,
src/errors.h, src/ex_cmds.c, src/testdir/test_substitute.vim


*** ../vim-8.2.5145/src/alloc.c 2022-05-10 13:24:17.640706898 +0100
--- src/alloc.c 2022-06-21 22:04:43.569152241 +0100
***************
*** 586,591 ****
--- 586,594 ----
# ifdef FEAT_QUICKFIX
check_quickfix_busy();
# endif
+ # ifdef FEAT_EVAL
+ free_resub_eval_result();
+ # endif
}
#endif

*** ../vim-8.2.5145/src/regexp.c 2022-06-20 13:38:29.166026119 +0100
--- src/regexp.c 2022-06-21 22:08:29.896040071 +0100
***************
*** 1922,1927 ****
--- 1922,1944 ----
return result;
}

+ #if defined(FEAT_EVAL) || defined(PROTO)
+ // When nesting more than a couple levels it's probably a mistake.
+ # define MAX_REGSUB_NESTING 4
+ static char_u *eval_result[MAX_REGSUB_NESTING] = {NULL, NULL, NULL, NULL};
+
+ # if defined(EXITFREE) || defined(PROTO)
+ void
+ free_resub_eval_result(void)
+ {
+ int i;
+
+ for (i = 0; i < MAX_REGSUB_NESTING; ++i)
+ VIM_CLEAR(eval_result[i]);
+ }
+ # endif
+ #endif
+
static int
vim_regsub_both(
char_u *source,
***************
*** 1941,1947 ****
linenr_T clnum = 0; // init for GCC
int len = 0; // init for GCC
#ifdef FEAT_EVAL
! static char_u *eval_result = NULL;
#endif
int copy = flags & REGSUB_COPY;

--- 1958,1965 ----
linenr_T clnum = 0; // init for GCC
int len = 0; // init for GCC
#ifdef FEAT_EVAL
! static int nesting = 0;
! int nested;
#endif
int copy = flags & REGSUB_COPY;

***************
*** 1953,1958 ****
--- 1971,1984 ----
}
if (prog_magic_wrong())
return 0;
+ #ifdef FEAT_EVAL
+ if (nesting == MAX_REGSUB_NESTING)
+ {
+ emsg(_(e_substitute_nesting_too_deep));
+ return 0;
+ }
+ nested = nesting;
+ #endif
src = source;
dst = dest;

***************
*** 1969,1979 ****
// "flags & REGSUB_COPY" != 0.
if (copy)
{
! if (eval_result != NULL)
{
! STRCPY(dest, eval_result);
! dst += STRLEN(eval_result);
! VIM_CLEAR(eval_result);
}
}
else
--- 1995,2005 ----
// "flags & REGSUB_COPY" != 0.
if (copy)
{
! if (eval_result[nested] != NULL)
{
! STRCPY(dest, eval_result[nested]);
! dst += STRLEN(eval_result[nested]);
! VIM_CLEAR(eval_result[nested]);
}
}
else
***************
*** 1981,1987 ****
int prev_can_f_submatch = can_f_submatch;
regsubmatch_T rsm_save;

! VIM_CLEAR(eval_result);

// The expression may contain substitute(), which calls us
// recursively. Make sure submatch() gets the text from the first
--- 2007,2013 ----
int prev_can_f_submatch = can_f_submatch;
regsubmatch_T rsm_save;

! VIM_CLEAR(eval_result[nested]);

// The expression may contain substitute(), which calls us
// recursively. Make sure submatch() gets the text from the first
***************
*** 1995,2000 ****
--- 2021,2031 ----
rsm.sm_maxline = rex.reg_maxline;
rsm.sm_line_lbr = rex.reg_line_lbr;

+ // Although unlikely, it is possible that the expression invokes a
+ // substitute command (it might fail, but still). Therefore keep
+ // an array if eval results.
+ ++nesting;
+
if (expr != NULL)
{
typval_T argv[2];
***************
*** 2034,2059 ****

if (rettv.v_type == VAR_UNKNOWN)
// something failed, no need to report another error
! eval_result = NULL;
else
{
! eval_result = tv_get_string_buf_chk(&rettv, buf);
! if (eval_result != NULL)
! eval_result = vim_strsave(eval_result);
}
clear_tv(&rettv);
}
else if (substitute_instr != NULL)
// Execute instructions from ISN_SUBSTITUTE.
! eval_result = exe_substitute_instr();
else
! eval_result = eval_to_string(source + 2, TRUE);

! if (eval_result != NULL)
{
int had_backslash = FALSE;

! for (s = eval_result; *s != NUL; MB_PTR_ADV(s))
{
// Change NL to CR, so that it becomes a line break,
// unless called from vim_regexec_nl().
--- 2065,2091 ----

if (rettv.v_type == VAR_UNKNOWN)
// something failed, no need to report another error
! eval_result[nested] = NULL;
else
{
! eval_result[nested] = tv_get_string_buf_chk(&rettv, buf);
! if (eval_result[nested] != NULL)
! eval_result[nested] = vim_strsave(eval_result[nested]);
}
clear_tv(&rettv);
}
else if (substitute_instr != NULL)
// Execute instructions from ISN_SUBSTITUTE.
! eval_result[nested] = exe_substitute_instr();
else
! eval_result[nested] = eval_to_string(source + 2, TRUE);
! --nesting;

! if (eval_result[nested] != NULL)
{
int had_backslash = FALSE;

! for (s = eval_result[nested]; *s != NUL; MB_PTR_ADV(s))
{
// Change NL to CR, so that it becomes a line break,
// unless called from vim_regexec_nl().
***************
*** 2077,2091 ****
if (had_backslash && (flags & REGSUB_BACKSLASH))
{
// Backslashes will be consumed, need to double them.
! s = vim_strsave_escaped(eval_result, (char_u *)"\\");
if (s != NULL)
{
! vim_free(eval_result);
! eval_result = s;
}
}

! dst += STRLEN(eval_result);
}

can_f_submatch = prev_can_f_submatch;
--- 2109,2123 ----
if (had_backslash && (flags & REGSUB_BACKSLASH))
{
// Backslashes will be consumed, need to double them.
! s = vim_strsave_escaped(eval_result[nested], (char_u *)"\\");
if (s != NULL)
{
! vim_free(eval_result[nested]);
! eval_result[nested] = s;
}
}

! dst += STRLEN(eval_result[nested]);
}

can_f_submatch = prev_can_f_submatch;
*** ../vim-8.2.5145/src/proto/regexp.pro 2022-06-05 16:55:50.698774344 +0100
--- src/proto/regexp.pro 2022-06-21 20:46:50.380250130 +0100
***************
*** 10,15 ****
--- 10,16 ----
char_u *regtilde(char_u *source, int magic);
int vim_regsub(regmatch_T *rmp, char_u *source, typval_T *expr, char_u *dest, int destlen, int flags);
int vim_regsub_multi(regmmatch_T *rmp, linenr_T lnum, char_u *source, char_u *dest, int destlen, int flags);
+ void free_resub_eval_result(void);
char_u *reg_submatch(int no);
list_T *reg_submatch_list(int no);
int vim_regcomp_had_eol(void);
*** ../vim-8.2.5145/src/errors.h 2022-06-05 16:55:50.694774344 +0100
--- src/errors.h 2022-06-21 21:15:26.136555198 +0100
***************
*** 3300,3302 ****
--- 3300,3306 ----
EXTERN char e_could_not_check_for_pending_sigalrm_str[]
INIT(= N_("E1289: Could not check for pending SIGALRM: %s"));
#endif
+ #ifdef FEAT_EVAL
+ EXTERN char e_substitute_nesting_too_deep[]
+ INIT(= N_("E1290: substitute nesting too deep"));
+ #endif
*** ../vim-8.2.5145/src/ex_cmds.c 2022-06-05 16:55:50.694774344 +0100
--- src/ex_cmds.c 2022-06-21 22:03:42.837500613 +0100
***************
*** 3701,3706 ****
--- 3701,3707 ----
int start_nsubs;
#ifdef FEAT_EVAL
int save_ma = 0;
+ int save_sandbox = 0;
#endif

cmd = eap->arg;
***************
*** 4403,4408 ****
--- 4404,4410 ----
*/
#ifdef FEAT_EVAL
save_ma = curbuf->b_p_ma;
+ save_sandbox = sandbox;
if (subflags.do_count)
{
// prevent accidentally changing the buffer by a function
***************
*** 4416,4422 ****
// Disallow changing text or switching window in an expression.
++textlock;
#endif
! // get length of substitution part
sublen = vim_regsub_multi(&regmatch,
sub_firstlnum - regmatch.startpos[0].lnum,
sub, sub_firstline, 0,
--- 4418,4425 ----
// Disallow changing text or switching window in an expression.
++textlock;
#endif
! // Get length of substitution part, including the NUL.
! // When it fails sublen is zero.
sublen = vim_regsub_multi(&regmatch,
sub_firstlnum - regmatch.startpos[0].lnum,
sub, sub_firstline, 0,
***************
*** 4429,4439 ****
// the replacement.
// Don't keep flags set by a recursive call.
subflags = subflags_save;
! if (aborting() || subflags.do_count)
{
curbuf->b_p_ma = save_ma;
! if (sandbox > 0)
! sandbox--;
goto skip;
}
#endif
--- 4432,4441 ----
// the replacement.
// Don't keep flags set by a recursive call.
subflags = subflags_save;
! if (sublen == 0 || aborting() || subflags.do_count)
{
curbuf->b_p_ma = save_ma;
! sandbox = save_sandbox;
goto skip;
}
#endif
*** ../vim-8.2.5145/src/testdir/test_substitute.vim 2022-06-18 19:48:10.813508250 +0100
--- src/testdir/test_substitute.vim 2022-06-21 22:13:30.586876515 +0100
***************
*** 995,1001 ****
~
s/
endfunc
! silent! s/\%')/\=Repl()

delfunc Repl
bwipe!
--- 995,1001 ----
~
s/
endfunc
! silent! s/\%')/\=Repl()

delfunc Repl
bwipe!
***************
*** 1359,1362 ****
--- 1359,1372 ----
bw!
endfunc

+ " This should be done last to reveal a memory leak when vim_regsub_both() is
+ " called to evaluate an expression but it is not used in a second call.
+ func Test_z_substitute_expr_leak()
+ func SubExpr()
+ ~n
+ endfunc
+ silent! s/\%')/\=SubExpr()
+ delfunc SubExpr
+ endfunc
+
" vim: shiftwidth=2 sts=2 expandtab
*** ../vim-8.2.5145/src/version.c 2022-06-21 18:34:37.581776214 +0100
--- src/version.c 2022-06-21 22:14:09.066745338 +0100
***************
*** 736,737 ****
--- 736,739 ----
{ /* Add new patch number below this line */
+ /**/
+ 5146,
/**/

--
Team-building exercises come in many forms but they all trace their roots back
to the prison system. In your typical team-building exercise the employees
are subjected to a variety of unpleasant situations until they become either a
cohesive team or a ring of car jackers.
(Scott Adams - The Dilbert principle)

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

[Christ van Willegen]

Typo:

Op di 21 jun. 2022 23:16 schreef Bram Moolenaar <Br...@moolenaar.net>:

+           // Although unlikely, it is possible that the expression invokes a
+           // substitute command (it might fail, but still).  Therefore keep
+           // an array if eval results.

Array of

[Bram Moolenaar]

Thanks for paying attention!

--
Managers are like cats in a litter box. They instinctively shuffle things
around to conceal what they've done.