From db7db783a2d634d5589ebe12605e3989cb30650c Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 3 Feb 2025 10:49:06 +0800 Subject: vim-patch:9.1.1071: args missing after failing to redefine a function Problem: Arguments of a function are missing after failing to redefine it (after 8.2.2505), and heap-use-after-free with script-local function (after 9.1.1063). Solution: Don't clear arguments or free uf_name_exp when failing to redefine an existing function (zeertzjq) closes: vim/vim#16567 https://github.com/vim/vim/commit/04d2a3fdc051d6a419dc0ea4de7a9640cefccd31 --- src/nvim/eval/userfunc.c | 11 +++++----- test/old/testdir/test_user_func.vim | 40 +++++++++++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/nvim/eval/userfunc.c b/src/nvim/eval/userfunc.c index 402798cafa..8022b37f6b 100644 --- a/src/nvim/eval/userfunc.c +++ b/src/nvim/eval/userfunc.c @@ -2825,11 +2825,11 @@ void ex_function(exarg_T *eap) && (fp->uf_script_ctx.sc_sid != current_sctx.sc_sid || fp->uf_script_ctx.sc_seq == current_sctx.sc_seq)) { emsg_funcname(e_funcexts, name); - goto erret; + goto errret_keep; } if (fp->uf_calls > 0) { emsg_funcname(N_("E127: Cannot redefine function %s: It is in use"), name); - goto erret; + goto errret_keep; } if (fp->uf_refcount > 1) { // This function is referenced somewhere, don't redefine it but @@ -2961,9 +2961,6 @@ erret: ga_init(&fp->uf_def_args, (int)sizeof(char *), 1); } errret_2: - ga_clear_strings(&newargs); - ga_clear_strings(&default_args); - ga_clear_strings(&newlines); if (fp != NULL) { XFREE_CLEAR(fp->uf_name_exp); } @@ -2971,6 +2968,10 @@ errret_2: xfree(fp); fp = NULL; } +errret_keep: + ga_clear_strings(&newargs); + ga_clear_strings(&default_args); + ga_clear_strings(&newlines); ret_free: xfree(line_to_free); xfree(fudi.fd_newkey); diff --git a/test/old/testdir/test_user_func.vim b/test/old/testdir/test_user_func.vim index da6a6d8dc4..b1543c8f24 100644 --- a/test/old/testdir/test_user_func.vim +++ b/test/old/testdir/test_user_func.vim @@ -421,12 +421,48 @@ func Test_func_def_error() call assert_fails('exe l', 'E717:') " Define an autoload function with an incorrect file name - call writefile(['func foo#Bar()', 'return 1', 'endfunc'], 'Xscript') + call writefile(['func foo#Bar()', 'return 1', 'endfunc'], 'Xscript', 'D') call assert_fails('source Xscript', 'E746:') - call delete('Xscript') " Try to list functions using an invalid search pattern call assert_fails('function /\%(/', 'E53:') + + " Use a script-local function to cover uf_name_exp. + func s:TestRedefine(arg1 = 1, arg2 = 10) + let caught_E122 = 0 + try + func s:TestRedefine(arg1 = 1, arg2 = 10) + endfunc + catch /E122:/ + let caught_E122 = 1 + endtry + call assert_equal(1, caught_E122) + + let caught_E127 = 0 + try + func! s:TestRedefine(arg1 = 1, arg2 = 10) + endfunc + catch /E127:/ + let caught_E127 = 1 + endtry + call assert_equal(1, caught_E127) + + " The failures above shouldn't cause heap-use-after-free here. + return [a:arg1 + a:arg2, expand('')] + endfunc + + let stacks = [] + " Call the function twice. + " Failing to redefine a function shouldn't clear its argument list. + for i in range(2) + let [val, stack] = s:TestRedefine(1000) + call assert_equal(1010, val) + call assert_match(expand('') .. 'TestRedefine\[20\]$', stack) + call add(stacks, stack) + endfor + call assert_equal(stacks[0], stacks[1]) + + delfunc s:TestRedefine endfunc " Test for deleting a function -- cgit