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: #16567

Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/src/testdir/test_user_func.vim b/src/testdir/test_user_func.vim
index bae98ed1f541524e1964f182211b5b994f45a34f..af7d466f6bb4be1c352853daa1b5dea5e4605b7b 100644 (file)
--- a/src/testdir/test_user_func.vim
+++ b/src/testdir/test_user_func.vim
@@ -473,6 +473,43 @@ func Test_func_def_error()
 
   " 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('<stack>')]
+  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('<SID>') .. 'TestRedefine\[20\]$', stack)
+    call add(stacks, stack)
+  endfor
+  call assert_equal(stacks[0], stacks[1])
+
+  delfunc s:TestRedefine
 endfunc
 
 " Test for deleting a function

diff --git a/src/userfunc.c b/src/userfunc.c
index 0cdfa38792042e98aa391588df0cc49cda235a32..a60eeb2d76a0d2a19e4a735b7da96e5ecd97e3e8 100644 (file)
--- a/src/userfunc.c
+++ b/src/userfunc.c
@@ -5404,13 +5404,13 @@ define_function(
                    emsg_funcname(e_name_already_defined_str, name);
                else
                    emsg_funcname(e_function_str_already_exists_add_bang_to_replace, name);
-               goto erret;
+               goto errret_keep;
            }
            if (fp->uf_calls > 0)
            {
                emsg_funcname(
                            e_cannot_redefine_function_str_it_is_in_use, name);
-               goto erret;
+               goto errret_keep;
            }
            if (fp->uf_refcount > 1)
            {
@@ -5630,9 +5630,6 @@ erret:
        ga_init(&fp->uf_def_args);
     }
 errret_2:
-    ga_clear_strings(&newargs);
-    ga_clear_strings(&default_args);
-    ga_clear_strings(&newlines);
     if (fp != NULL)
     {
        VIM_CLEAR(fp->uf_arg_types);
@@ -5642,6 +5639,10 @@ errret_2:
     }
     if (free_fp)
        VIM_CLEAR(fp);
+errret_keep:
+    ga_clear_strings(&newargs);
+    ga_clear_strings(&default_args);
+    ga_clear_strings(&newlines);
 ret_free:
     ga_clear_strings(&argtypes);
     ga_clear(&arg_objm);

diff --git a/src/version.c b/src/version.c
index 5ed5bcf3190b058d239f4db400a97b9bbe554583..b953f905adb97f807a25410b6301c69ae5185768 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -704,6 +704,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1071,
 /**/
     1070,
 /**/