vim-patch:8.2.5146: memory leak when substitute expression nests

Problem:    Memory leak when substitute expression nests.
Solution:   Use an array of expression results.
44ddf19ec0

Cherry-pick a comment change from patch 8.2.5057.

N/A patches for version.c:

vim-patch:8.2.5154: still mentioning version8, some cosmetic issues

Problem:    Still mentioning version8, some cosmetic issues.
Solution:   Prefer mentioning version9, cosmetic improvements.
abd56da30b
This commit is contained in:
zeertzjq 2022-06-24 06:50:48 +08:00
parent 589f418fce
commit affeb5c6dd
3 changed files with 62 additions and 27 deletions

View File

@ -3468,7 +3468,6 @@ static int do_sub(exarg_T *eap, proftime_T timeout, long cmdpreview_ns, handle_T
static int pre_hl_id = 0; static int pre_hl_id = 0;
pos_T old_cursor = curwin->w_cursor; pos_T old_cursor = curwin->w_cursor;
int start_nsubs; int start_nsubs;
int save_ma = 0;
bool did_save = false; bool did_save = false;
@ -4060,7 +4059,8 @@ static int do_sub(exarg_T *eap, proftime_T timeout, long cmdpreview_ns, handle_T
// there is a replace pattern. // there is a replace pattern.
if (!cmdpreview || has_second_delim) { if (!cmdpreview || has_second_delim) {
long lnum_start = lnum; // save the start lnum long lnum_start = lnum; // save the start lnum
save_ma = curbuf->b_p_ma; int save_ma = curbuf->b_p_ma;
int save_sandbox = sandbox;
if (subflags.do_count) { if (subflags.do_count) {
// prevent accidentally changing the buffer by a function // prevent accidentally changing the buffer by a function
curbuf->b_p_ma = false; curbuf->b_p_ma = false;
@ -4072,7 +4072,8 @@ static int do_sub(exarg_T *eap, proftime_T timeout, long cmdpreview_ns, handle_T
// Disallow changing text or switching window in an expression. // Disallow changing text or switching window in an expression.
textlock++; textlock++;
// get length of substitution part // Get length of substitution part, including the NUL.
// When it fails sublen is zero.
sublen = vim_regsub_multi(&regmatch, sublen = vim_regsub_multi(&regmatch,
sub_firstlnum - regmatch.startpos[0].lnum, sub_firstlnum - regmatch.startpos[0].lnum,
(char_u *)sub, (char_u *)sub_firstline, 0, (char_u *)sub, (char_u *)sub_firstline, 0,
@ -4083,11 +4084,9 @@ static int do_sub(exarg_T *eap, proftime_T timeout, long cmdpreview_ns, handle_T
// the replacement. // the replacement.
// Don't keep flags set by a recursive call // Don't keep flags set by a recursive call
subflags = subflags_save; subflags = subflags_save;
if (aborting() || subflags.do_count) { if (sublen == 0 || aborting() || subflags.do_count) {
curbuf->b_p_ma = save_ma; curbuf->b_p_ma = save_ma;
if (sandbox > 0) { sandbox = save_sandbox;
sandbox--;
}
goto skip; goto skip;
} }

View File

@ -110,6 +110,7 @@ static char_u e_empty_sb[] = N_("E70: Empty %s%%[]");
static char_u e_recursive[] = N_("E956: Cannot use pattern recursively"); static char_u e_recursive[] = N_("E956: Cannot use pattern recursively");
static char_u e_regexp_number_after_dot_pos_search[] static char_u e_regexp_number_after_dot_pos_search[]
= N_("E1204: No Number allowed after .: '\\%%%c'"); = N_("E1204: No Number allowed after .: '\\%%%c'");
static char_u e_substitute_nesting_too_deep[] = N_("E1290: substitute nesting too deep");
#define NOT_MULTI 0 #define NOT_MULTI 0
#define MULTI_ONE 1 #define MULTI_ONE 1
@ -1722,6 +1723,19 @@ int vim_regsub_multi(regmmatch_T *rmp, linenr_T lnum, char_u *source, char_u *de
return result; return result;
} }
// 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)
void free_resub_eval_result(void)
{
for (int i = 0; i < MAX_REGSUB_NESTING; i++) {
XFREE_CLEAR(eval_result[i]);
}
}
#endif
static int vim_regsub_both(char_u *source, typval_T *expr, char_u *dest, int destlen, int flags) static int vim_regsub_both(char_u *source, typval_T *expr, char_u *dest, int destlen, int flags)
{ {
char_u *src; char_u *src;
@ -1734,7 +1748,7 @@ static int vim_regsub_both(char_u *source, typval_T *expr, char_u *dest, int des
fptr_T func_one = (fptr_T)NULL; fptr_T func_one = (fptr_T)NULL;
linenr_T clnum = 0; // init for GCC linenr_T clnum = 0; // init for GCC
int len = 0; // init for GCC int len = 0; // init for GCC
static char_u *eval_result = NULL; static int nesting = 0;
bool copy = flags & REGSUB_COPY; bool copy = flags & REGSUB_COPY;
// Be paranoid... // Be paranoid...
@ -1745,6 +1759,11 @@ static int vim_regsub_both(char_u *source, typval_T *expr, char_u *dest, int des
if (prog_magic_wrong()) { if (prog_magic_wrong()) {
return 0; return 0;
} }
if (nesting == MAX_REGSUB_NESTING) {
emsg(_(e_substitute_nesting_too_deep));
return 0;
}
int nested = nesting;
src = source; src = source;
dst = dest; dst = dest;
@ -1752,19 +1771,20 @@ static int vim_regsub_both(char_u *source, typval_T *expr, char_u *dest, int des
if (expr != NULL || (source[0] == '\\' && source[1] == '=')) { if (expr != NULL || (source[0] == '\\' && source[1] == '=')) {
// To make sure that the length doesn't change between checking the // To make sure that the length doesn't change between checking the
// length and copying the string, and to speed up things, the // length and copying the string, and to speed up things, the
// resulting string is saved from the call with "flags & REGSUB_COPY" // resulting string is saved from the call with
// == 0 to the call with "flags & REGSUB_COPY" != 0. // "flags & REGSUB_COPY" == 0 to the call with
// "flags & REGSUB_COPY" != 0.
if (copy) { if (copy) {
if (eval_result != NULL) { if (eval_result[nested] != NULL) {
STRCPY(dest, eval_result); STRCPY(dest, eval_result[nested]);
dst += STRLEN(eval_result); dst += STRLEN(eval_result[nested]);
XFREE_CLEAR(eval_result); XFREE_CLEAR(eval_result[nested]);
} }
} else { } else {
const bool prev_can_f_submatch = can_f_submatch; const bool prev_can_f_submatch = can_f_submatch;
regsubmatch_T rsm_save; regsubmatch_T rsm_save;
xfree(eval_result); XFREE_CLEAR(eval_result[nested]);
// The expression may contain substitute(), which calls us // The expression may contain substitute(), which calls us
// recursively. Make sure submatch() gets the text from the first // recursively. Make sure submatch() gets the text from the first
@ -1779,6 +1799,11 @@ static int vim_regsub_both(char_u *source, typval_T *expr, char_u *dest, int des
rsm.sm_maxline = rex.reg_maxline; rsm.sm_maxline = rex.reg_maxline;
rsm.sm_line_lbr = rex.reg_line_lbr; 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 of eval results.
nesting++;
if (expr != NULL) { if (expr != NULL) {
typval_T argv[2]; typval_T argv[2];
typval_T rettv; typval_T rettv;
@ -1806,23 +1831,24 @@ static int vim_regsub_both(char_u *source, typval_T *expr, char_u *dest, int des
} }
if (rettv.v_type == VAR_UNKNOWN) { if (rettv.v_type == VAR_UNKNOWN) {
// something failed, no need to report another error // something failed, no need to report another error
eval_result = NULL; eval_result[nested] = NULL;
} else { } else {
char buf[NUMBUFLEN]; char buf[NUMBUFLEN];
eval_result = (char_u *)tv_get_string_buf_chk(&rettv, buf); eval_result[nested] = (char_u *)tv_get_string_buf_chk(&rettv, buf);
if (eval_result != NULL) { if (eval_result[nested] != NULL) {
eval_result = vim_strsave(eval_result); eval_result[nested] = vim_strsave(eval_result[nested]);
} }
} }
tv_clear(&rettv); tv_clear(&rettv);
} else { } else {
eval_result = (char_u *)eval_to_string((char *)source + 2, NULL, true); eval_result[nested] = (char_u *)eval_to_string((char *)source + 2, NULL, true);
} }
nesting--;
if (eval_result != NULL) { if (eval_result[nested] != NULL) {
int had_backslash = false; 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, // Change NL to CR, so that it becomes a line break,
// unless called from vim_regexec_nl(). // unless called from vim_regexec_nl().
// Skip over a backslashed character. // Skip over a backslashed character.
@ -1844,12 +1870,12 @@ static int vim_regsub_both(char_u *source, typval_T *expr, char_u *dest, int des
} }
if (had_backslash && (flags & REGSUB_BACKSLASH)) { if (had_backslash && (flags & REGSUB_BACKSLASH)) {
// Backslashes will be consumed, need to double them. // 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 *)"\\");
xfree(eval_result); xfree(eval_result[nested]);
eval_result = s; eval_result[nested] = s;
} }
dst += STRLEN(eval_result); dst += STRLEN(eval_result[nested]);
} }
can_f_submatch = prev_can_f_submatch; can_f_submatch = prev_can_f_submatch;

View File

@ -831,7 +831,7 @@ func Test_using_old_sub()
~ ~
s/ s/
endfunc endfunc
silent! s/\%')/\=Repl() silent! s/\%')/\=Repl()
delfunc Repl delfunc Repl
bwipe! bwipe!
@ -1134,4 +1134,14 @@ func Test_substitute_short_cmd()
bw! bw!
endfunc 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: shiftwidth=2 sts=2 expandtab