patch 8.2.5146: memory leak when substitute expression nests v8.2.5146
authorBram Moolenaar <Bram@vim.org>
Tue, 21 Jun 2022 21:15:25 +0000 (22:15 +0100)
committerBram Moolenaar <Bram@vim.org>
Tue, 21 Jun 2022 21:15:25 +0000 (22:15 +0100)
Problem:    Memory leak when substitute expression nests.
Solution:   Use an array of expression results.

src/alloc.c
src/errors.h
src/ex_cmds.c
src/proto/regexp.pro
src/regexp.c
src/testdir/test_substitute.vim
src/version.c

index 6d4db629e1aa48e7424bbd719f8c5fc1d396b914..6e2a30adf21bf055e7c1ed8de7753eed96936826 100644 (file)
@@ -586,6 +586,9 @@ free_all_mem(void)
 # ifdef FEAT_QUICKFIX
     check_quickfix_busy();
 # endif
+# ifdef FEAT_EVAL
+    free_resub_eval_result();
+# endif
 }
 #endif
 
index 109d9e8348bb12487d8c8e6899e018f79d391337..43a1c9b95507b201cf4d605adbf56a6ab88d70fb 100644 (file)
@@ -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
index 34a740da24bc89d455847028cb306698587bfc25..eb3016fe571e20d230f2ddba3c481c42a3c1f3b4 100644 (file)
@@ -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;
@@ -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
@@ -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,
@@ -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
index 2ff3b253af920de475cfaa911defd2558503154a..3fd91eefaafc42c10fae3e1c0462701e08d3c21b 100644 (file)
@@ -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);
index 3c1334ddd9dd47a281dc11d9606c084a861a001f..e6b75ea59ea51c249a7636809f4d3bf1fea91d2b 100644 (file)
@@ -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,
@@ -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;
 
@@ -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;
 
@@ -1969,11 +1995,11 @@ 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
@@ -1981,7 +2007,7 @@ vim_regsub_both(
            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
@@ -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];
@@ -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().
@@ -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;
index c056fa9656929b2ac052b42912d78a75ac0e5e2e..7e7ccff0f5ee19a4fdfcc6b7644e240a13147d1b 100644 (file)
@@ -995,7 +995,7 @@ func Test_using_old_sub()
     ~
     s/
   endfunc
-  silent!  s/\%')/\=Repl()
+  silent! s/\%')/\=Repl()
 
   delfunc Repl
   bwipe!
@@ -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
index 0353e90cff447b8367b9ae6f8cb69879af985d4d..4530f8d5e28be76c5cfa0679f1516ad57f1b7126 100644 (file)
@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    5146,
 /**/
     5145,
 /**/