Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
patch 8.2.5146: memory leak when substitute expression nests
Problem:    Memory leak when substitute expression nests.
Solution:   Use an array of expression results.
  • Loading branch information
brammool committed Jun 21, 2022
1 parent cf801d4 commit 44ddf19
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 23 deletions.
3 changes: 3 additions & 0 deletions src/alloc.c
Expand Up @@ -586,6 +586,9 @@ free_all_mem(void)
# ifdef FEAT_QUICKFIX
check_quickfix_busy();
# endif
# ifdef FEAT_EVAL
free_resub_eval_result();
# endif
}
#endif

Expand Down
4 changes: 4 additions & 0 deletions src/errors.h
Expand Up @@ -3300,3 +3300,7 @@ EXTERN char e_could_not_reset_handler_for_timeout_str[]
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
10 changes: 6 additions & 4 deletions src/ex_cmds.c
Expand Up @@ -3701,6 +3701,7 @@ ex_substitute(exarg_T *eap)
int start_nsubs;
#ifdef FEAT_EVAL
int save_ma = 0;
int save_sandbox = 0;
#endif

cmd = eap->arg;
Expand Down Expand Up @@ -4403,6 +4404,7 @@ ex_substitute(exarg_T *eap)
*/
#ifdef FEAT_EVAL
save_ma = curbuf->b_p_ma;
save_sandbox = sandbox;
if (subflags.do_count)
{
// prevent accidentally changing the buffer by a function
Expand All @@ -4416,7 +4418,8 @@ ex_substitute(exarg_T *eap)
// Disallow changing text or switching window in an expression.
++textlock;
#endif
// get length of substitution part
// 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,
Expand All @@ -4429,11 +4432,10 @@ ex_substitute(exarg_T *eap)
// the replacement.
// Don't keep flags set by a recursive call.
subflags = subflags_save;
if (aborting() || subflags.do_count)
if (sublen == 0 || aborting() || subflags.do_count)
{
curbuf->b_p_ma = save_ma;
if (sandbox > 0)
sandbox--;
sandbox = save_sandbox;
goto skip;
}
#endif
Expand Down
1 change: 1 addition & 0 deletions src/proto/regexp.pro
Expand Up @@ -10,6 +10,7 @@ void unref_extmatch(reg_extmatch_T *em);
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);
Expand Down
68 changes: 50 additions & 18 deletions src/regexp.c
Expand Up @@ -1922,6 +1922,23 @@ vim_regsub_multi(
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,
Expand All @@ -1941,7 +1958,8 @@ vim_regsub_both(
linenr_T clnum = 0; // init for GCC
int len = 0; // init for GCC
#ifdef FEAT_EVAL
static char_u *eval_result = NULL;
static int nesting = 0;
int nested;
#endif
int copy = flags & REGSUB_COPY;

Expand All @@ -1953,6 +1971,14 @@ vim_regsub_both(
}
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;

Expand All @@ -1969,19 +1995,19 @@ vim_regsub_both(
// "flags & REGSUB_COPY" != 0.
if (copy)
{
if (eval_result != NULL)
if (eval_result[nested] != NULL)
{
STRCPY(dest, eval_result);
dst += STRLEN(eval_result);
VIM_CLEAR(eval_result);
STRCPY(dest, eval_result[nested]);
dst += STRLEN(eval_result[nested]);
VIM_CLEAR(eval_result[nested]);
}
}
else
{
int prev_can_f_submatch = can_f_submatch;
regsubmatch_T rsm_save;

VIM_CLEAR(eval_result);
VIM_CLEAR(eval_result[nested]);

// The expression may contain substitute(), which calls us
// recursively. Make sure submatch() gets the text from the first
Expand All @@ -1995,6 +2021,11 @@ vim_regsub_both(
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];
Expand Down Expand Up @@ -2034,26 +2065,27 @@ vim_regsub_both(

if (rettv.v_type == VAR_UNKNOWN)
// something failed, no need to report another error
eval_result = NULL;
eval_result[nested] = NULL;
else
{
eval_result = tv_get_string_buf_chk(&rettv, buf);
if (eval_result != NULL)
eval_result = vim_strsave(eval_result);
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 = exe_substitute_instr();
eval_result[nested] = exe_substitute_instr();
else
eval_result = eval_to_string(source + 2, TRUE);
eval_result[nested] = eval_to_string(source + 2, TRUE);
--nesting;

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

for (s = eval_result; *s != NUL; MB_PTR_ADV(s))
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().
Expand All @@ -2077,15 +2109,15 @@ vim_regsub_both(
if (had_backslash && (flags & REGSUB_BACKSLASH))
{
// Backslashes will be consumed, need to double them.
s = vim_strsave_escaped(eval_result, (char_u *)"\\");
s = vim_strsave_escaped(eval_result[nested], (char_u *)"\\");
if (s != NULL)
{
vim_free(eval_result);
eval_result = s;
vim_free(eval_result[nested]);
eval_result[nested] = s;
}
}

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

can_f_submatch = prev_can_f_submatch;
Expand Down
12 changes: 11 additions & 1 deletion src/testdir/test_substitute.vim
Expand Up @@ -995,7 +995,7 @@ func Test_using_old_sub()
~
s/
endfunc
silent! s/\%')/\=Repl()
silent! s/\%')/\=Repl()

delfunc Repl
bwipe!
Expand Down Expand Up @@ -1359,4 +1359,14 @@ func Test_substitute_short_cmd()
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
2 changes: 2 additions & 0 deletions src/version.c
Expand Up @@ -734,6 +734,8 @@ static char *(features[]) =

static int included_patches[] =
{ /* Add new patch number below this line */
/**/
5146,
/**/
5145,
/**/
Expand Down

0 comments on commit 44ddf19

Please sign in to comment.