From ac9f8669a8e4bd0cf13468d316d1746be65d3cdc Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Tue, 25 Apr 2023 23:19:00 +0800 Subject: [PATCH] vim-patch:9.0.0875: using freed memory when executing delfunc at more prompt (#23314) Problem: Using freed memory when executing delfunc at the more prompt. Solution: Check function list not changed in another place. (closes vim/vim#11437) https://github.com/vim/vim/commit/398a26f7fcd58fbc6e2329f892edbb7479a971bb Co-authored-by: Bram Moolenaar --- src/nvim/eval/userfunc.c | 80 ++++++++++++++++++------- test/functional/vimscript/eval_spec.lua | 31 +++++++++- test/old/testdir/test_functions.vim | 27 +++++++++ 3 files changed, 113 insertions(+), 25 deletions(-) diff --git a/src/nvim/eval/userfunc.c b/src/nvim/eval/userfunc.c index b71e6c9cff..854a0732ab 100644 --- a/src/nvim/eval/userfunc.c +++ b/src/nvim/eval/userfunc.c @@ -77,6 +77,8 @@ static const char *e_funcexts = N_("E122: Function %s already exists, add ! to r static const char *e_funcdict = N_("E717: Dictionary entry already exists"); static const char *e_funcref = N_("E718: Funcref required"); static const char *e_nofunc = N_("E130: Unknown function: %s"); +static const char e_function_list_was_modified[] + = N_("E454: Function list was modified"); static const char e_no_white_space_allowed_before_str_str[] = N_("E1068: No white space allowed before '%s': %s"); static const char e_missing_heredoc_end_marker_str[] @@ -1752,14 +1754,33 @@ char *printable_func_name(ufunc_T *fp) return fp->uf_name_exp != NULL ? fp->uf_name_exp : fp->uf_name; } +/// When "prev_ht_changed" does not equal "ht_changed" give an error and return +/// true. Otherwise return false. +static int function_list_modified(const int prev_ht_changed) +{ + if (prev_ht_changed != func_hashtab.ht_changed) { + emsg(_(e_function_list_was_modified)); + return true; + } + return false; +} + /// List the head of the function: "name(arg1, arg2)". /// /// @param[in] fp Function pointer. /// @param[in] indent Indent line. /// @param[in] force Include bang "!" (i.e.: "function!"). -static void list_func_head(ufunc_T *fp, int indent, bool force) +static int list_func_head(ufunc_T *fp, bool indent, bool force) { + const int prev_ht_changed = func_hashtab.ht_changed; + msg_start(); + + // a callback at the more prompt may have deleted the function + if (function_list_modified(prev_ht_changed)) { + return FAIL; + } + if (indent) { msg_puts(" "); } @@ -1805,6 +1826,8 @@ static void list_func_head(ufunc_T *fp, int indent, bool force) if (p_verbose > 0) { last_set_msg(fp->uf_script_ctx); } + + return OK; } /// Get a function name, translating "" and "". @@ -2085,7 +2108,7 @@ char *save_function_name(char **name, bool skip, int flags, funcdict_T *fudi) /// Otherwise functions matching "regmatch". static void list_functions(regmatch_T *regmatch) { - const int changed = func_hashtab.ht_changed; + const int prev_ht_changed = func_hashtab.ht_changed; size_t todo = func_hashtab.ht_used; const hashitem_T *const ht_array = func_hashtab.ht_array; @@ -2098,9 +2121,10 @@ static void list_functions(regmatch_T *regmatch) && !func_name_refcount(fp->uf_name)) : (!isdigit((uint8_t)(*fp->uf_name)) && vim_regexec(regmatch, fp->uf_name, 0))) { - list_func_head(fp, false, false); - if (changed != func_hashtab.ht_changed) { - emsg(_("E454: function list was modified")); + if (list_func_head(fp, false, false) == FAIL) { + return; + } + if (function_list_modified(prev_ht_changed)) { return; } } @@ -2229,27 +2253,37 @@ void ex_function(exarg_T *eap) if (!eap->skip && !got_int) { fp = find_func(name); if (fp != NULL) { - list_func_head(fp, !eap->forceit, eap->forceit); - for (int j = 0; j < fp->uf_lines.ga_len && !got_int; j++) { - if (FUNCLINE(fp, j) == NULL) { - continue; - } - msg_putchar('\n'); - if (!eap->forceit) { - msg_outnum((long)j + 1); - if (j < 9) { - msg_putchar(' '); + // Check no function was added or removed from a callback, e.g. at + // the more prompt. "fp" may then be invalid. + const int prev_ht_changed = func_hashtab.ht_changed; + + if (list_func_head(fp, !eap->forceit, eap->forceit) == OK) { + for (int j = 0; j < fp->uf_lines.ga_len && !got_int; j++) { + if (FUNCLINE(fp, j) == NULL) { + continue; } - if (j < 99) { - msg_putchar(' '); + msg_putchar('\n'); + if (!eap->forceit) { + msg_outnum((long)j + 1); + if (j < 9) { + msg_putchar(' '); + } + if (j < 99) { + msg_putchar(' '); + } + if (function_list_modified(prev_ht_changed)) { + break; + } + } + msg_prt_line(FUNCLINE(fp, j), false); + line_breakcheck(); // show multiple lines at a time! + } + if (!got_int) { + msg_putchar('\n'); + if (!function_list_modified(prev_ht_changed)) { + msg_puts(eap->forceit ? "endfunction" : " endfunction"); } } - msg_prt_line(FUNCLINE(fp, j), false); - line_breakcheck(); // show multiple lines at a time! - } - if (!got_int) { - msg_putchar('\n'); - msg_puts(eap->forceit ? "endfunction" : " endfunction"); } } else { emsg_funcname(N_("E123: Undefined function: %s"), name); diff --git a/test/functional/vimscript/eval_spec.lua b/test/functional/vimscript/eval_spec.lua index b411b1e379..b3f2c1bfeb 100644 --- a/test/functional/vimscript/eval_spec.lua +++ b/test/functional/vimscript/eval_spec.lua @@ -191,11 +191,10 @@ describe('listing functions using :function', function() endfunction]]):format(num), exec_capture(('function %s'):format(num))) end) - -- FIXME: If the same function is deleted, the crash still happens. #20790 it('does not crash if another function is deleted while listing', function() local screen = Screen.new(80, 24) screen:attach() - matches('Vim%(function%):E454: function list was modified', pcall_err(exec_lua, [=[ + matches('Vim%(function%):E454: Function list was modified$', pcall_err(exec_lua, [=[ vim.cmd([[ func Func1() endfunc @@ -219,6 +218,34 @@ describe('listing functions using :function', function() ]=])) assert_alive() end) + + it('does not crash if the same function is deleted while listing', function() + local screen = Screen.new(80, 24) + screen:attach() + matches('Vim%(function%):E454: Function list was modified$', pcall_err(exec_lua, [=[ + vim.cmd([[ + func Func1() + endfunc + func Func2() + endfunc + func Func3() + endfunc + ]]) + + local ns = vim.api.nvim_create_namespace('test') + + vim.ui_attach(ns, { ext_messages = true }, function(event, _, content) + if event == 'msg_show' and content[1][2] == 'function Func1()' then + vim.cmd('delfunc Func2') + end + end) + + vim.cmd('function') + + vim.ui_detach(ns) + ]=])) + assert_alive() + end) end) it('no double-free in garbage collection #16287', function() diff --git a/test/old/testdir/test_functions.vim b/test/old/testdir/test_functions.vim index a1f34ea8b6..0dd780e3fb 100644 --- a/test/old/testdir/test_functions.vim +++ b/test/old/testdir/test_functions.vim @@ -2618,4 +2618,31 @@ func Test_virtcol() bwipe! endfunc +func Test_delfunc_while_listing() + CheckRunVimInTerminal + + let lines =<< trim END + set nocompatible + for i in range(1, 999) + exe 'func ' .. 'MyFunc' .. i .. '()' + endfunc + endfor + au CmdlineLeave : call timer_start(0, {-> execute('delfunc MyFunc622')}) + END + call writefile(lines, 'Xfunctionclear', 'D') + let buf = RunVimInTerminal('-S Xfunctionclear', {'rows': 12}) + + " This was using freed memory. The height of the terminal must be so that + " the next function to be listed with "j" is the one that is deleted in the + " timer callback, tricky! + call term_sendkeys(buf, ":func /MyFunc\") + call TermWait(buf, 50) + call term_sendkeys(buf, "j") + call TermWait(buf, 50) + call term_sendkeys(buf, "\") + + call StopVimInTerminal(buf) +endfunc + + " vim: shiftwidth=2 sts=2 expandtab