patch 9.2.0252: Crash when ending Visual mode after curbuf was unloaded

Problem:  if close_buffer() in set_curbuf() unloads curbuf, NULL pointer
          accesses may occur from enter_buffer() calling
          end_visual_mode(), as curbuf is already abandoned and possibly
          unloaded.  Also, selection registers may not contain the
          selection with clipboard+=autoselect(plus).
Solution: Move close_buffer()'s end_visual_mode() call to buf_freeall(), after
          any autocmds that may restart it, but just before freeing anything
          (Sean Dewar)

related: #19728

Signed-off-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/src/buffer.c b/src/buffer.c
index 26e3ec969d22affadde8e181b9abc2f0e489b691..dd7285da07447a6c61e6e7892900dd95ae32332a 100644 (file)
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -739,15 +739,6 @@ aucmd_abort:
     if (buf->b_ffname == NULL)
        del_buf = TRUE;
 
-    // When closing the current buffer stop Visual mode before freeing
-    // anything.
-    if (buf == curbuf && VIsual_active
-#if defined(EXITFREE)
-           && !entered_free_all_mem
-#endif
-           )
-       end_visual_mode();
-
     // Free all things allocated for this buffer.
     // Also calls the "BufDelete" autocommands when del_buf is TRUE.
     //
@@ -944,6 +935,16 @@ buf_freeall(buf_T *buf, int flags)
     // Therefore only return if curbuf changed to the deleted buffer.
     if (buf == curbuf && !is_curbuf)
        return;
+
+    // If curbuf, stop Visual mode just before freeing, but after autocmds that
+    // may restart it.  May trigger TextYankPost, but with textlock set.
+    if (buf == curbuf && VIsual_active
+#if defined(EXITFREE)
+           && !entered_free_all_mem
+#endif
+           )
+       end_visual_mode();
+
 #ifdef FEAT_DIFF
     diff_buf_delete(buf);          // Can't use 'diff' for unloaded buffer.
 #endif
@@ -1976,7 +1977,9 @@ set_curbuf(buf_T *buf, int action)
     static void
 enter_buffer(buf_T *buf)
 {
-    // when closing the current buffer stop Visual mode
+    // Stop Visual mode before changing curbuf.  May trigger TextYankPost, but
+    // with textlock set.  Assumes curbuf and curwin->w_buffer is valid; if not,
+    // buf_freeall() should've done this already!
     if (VIsual_active
 #if defined(EXITFREE)
            && !entered_free_all_mem

diff --git a/src/testdir/test_visual.vim b/src/testdir/test_visual.vim
index 3957d0a2d48baedd5d6e21f969add35a29f9acf4..a2bb12588fc88322b51fabac805944d27b5a4941 100644 (file)
--- a/src/testdir/test_visual.vim
+++ b/src/testdir/test_visual.vim
@@ -2970,4 +2970,57 @@ func Test_getregionpos_block_linebreak_matches_getpos()
   let &columns = save_columns
   bw!
 endfunc
+
+func Test_visual_ended_in_wiped_buffer()
+  edit Xfoo
+  edit Xbar
+  setlocal bufhidden=wipe
+  augroup testing
+    autocmd BufWipeout * ++once normal! v
+  augroup END
+  " Must be the last window.
+  call assert_equal(1, winnr('$'))
+  call assert_equal(1, tabpagenr('$'))
+  " Was a member access on a NULL curbuf from Vim ending Visual mode.
+  buffer #
+  call assert_equal(0, bufexists('Xbar'))
+  call assert_equal('n', mode())
+
+  autocmd! testing
+  %bw!
+endfunc
+
+func Test_visual_ended_in_unloaded_buffer()
+  CheckFeature clipboard
+  CheckNotGui
+  set clipboard+=autoselect
+  edit Xfoo
+  edit Xbar
+  call setline(1, 'hi')
+  setlocal nomodified
+  let s:fired = 0
+  augroup testing
+    autocmd BufUnload Xbar call assert_equal('Xbar', bufname())
+          \| execute 'normal! V'
+          \| call assert_equal('V', mode())
+
+    " From Vim ending Visual mode.  Used to occur too late, after the buffer was
+    " unloaded, so @* didn't contain the selection.  Window also had a NULL
+    " w_buffer here!
+    autocmd TextYankPost * ++once let s:fired = 1
+          \| if has('clipboard_working') | call assert_equal("hi\n", @*) | endif
+          \| call tabpagebuflist() " was a NULL member access on w_buffer
+  augroup END
+
+  buffer Xfoo
+  call assert_equal(0, bufloaded('Xbar'))
+  call assert_equal('n', mode())
+  call assert_equal(1, s:fired)
+
+  autocmd! testing
+  unlet! s:fired
+  set clipboard&
+  %bw!
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

diff --git a/src/version.c b/src/version.c
index 862b5b12ae0d8e6751135b03e162a41469abfbe3..da34b93b8ed0f2a5cf11793a4e4dd3b1fee92b33 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    252,
 /**/
     251,
 /**/