To: vim_dev@googlegroups.com Subject: Patch 8.2.5146 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ 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(®match, 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(®match, 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 -- Bram@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 ///