Merge pull request #18182 from zeertzjq/vim-8.2.2472

vim-patch:8.1.1756,8.2.{2472,2474,2475,2476,2477,4791,4802}: autocommand fixes

8 files changed, 155 insertions, 27 deletions

diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c
index 4d914acea4..30bd37fe7f 100644
--- a/src/nvim/buffer.c
+++ b/src/nvim/buffer.c
@@ -466,6 +466,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i
   // When the buffer is no longer in a window, trigger BufWinLeave
   if (buf->b_nwindows == 1) {
     buf->b_locked++;
+    buf->b_locked_split++;
     if (apply_autocmds(EVENT_BUFWINLEAVE, buf->b_fname, buf->b_fname, false,
                        buf) && !bufref_valid(&bufref)) {
       // Autocommands deleted the buffer.
@@ -473,6 +474,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i
       return false;
     }
     buf->b_locked--;
+    buf->b_locked_split--;
     if (abort_if_last && last_nonfloat(win)) {
       // Autocommands made this the only window.
       emsg(_(e_auabort));
@@ -483,6 +485,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i
     // BufHidden
     if (!unload_buf) {
       buf->b_locked++;
+      buf->b_locked_split++;
       if (apply_autocmds(EVENT_BUFHIDDEN, buf->b_fname, buf->b_fname, false,
                          buf) && !bufref_valid(&bufref)) {
         // Autocommands deleted the buffer.
@@ -490,6 +493,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i
         return false;
       }
       buf->b_locked--;
+      buf->b_locked_split--;
       if (abort_if_last && last_nonfloat(win)) {
         // Autocommands made this the only window.
         emsg(_(e_auabort));
@@ -678,6 +682,7 @@ void buf_freeall(buf_T *buf, int flags)
 
   // Make sure the buffer isn't closed by autocommands.
   buf->b_locked++;
+  buf->b_locked_split++;
 
   bufref_T bufref;
   set_bufref(&bufref, buf);
@@ -703,6 +708,7 @@ void buf_freeall(buf_T *buf, int flags)
     return;
   }
   buf->b_locked--;
+  buf->b_locked_split--;
 
   // If the buffer was in curwin and the window has changed, go back to that
   // window, if it still exists.  This avoids that ":edit x" triggering a
@@ -1466,8 +1472,8 @@ void set_curbuf(buf_T *buf, int action)
   set_bufref(&prevbufref, prevbuf);
   set_bufref(&newbufref, buf);
 
-  // Autocommands may delete the current buffer and/or the buffer we want to go
-  // to.  In those cases don't close the buffer.
+  // Autocommands may delete the current buffer and/or the buffer we want to
+  // go to.  In those cases don't close the buffer.
   if (!apply_autocmds(EVENT_BUFLEAVE, NULL, NULL, false, curbuf)
       || (bufref_valid(&prevbufref) && bufref_valid(&newbufref)
           && !aborting())) {
@@ -1742,21 +1748,14 @@ buf_T *buflist_new(char_u *ffname_arg, char_u *sfname_arg, linenr_T lnum, int fl
     buf = curbuf;
     // It's like this buffer is deleted.  Watch out for autocommands that
     // change curbuf!  If that happens, allocate a new buffer anyway.
-    if (curbuf->b_p_bl) {
-      apply_autocmds(EVENT_BUFDELETE, NULL, NULL, false, curbuf);
-    }
-    if (buf == curbuf) {
-      apply_autocmds(EVENT_BUFWIPEOUT, NULL, NULL, false, curbuf);
+    buf_freeall(buf, BFA_WIPE | BFA_DEL);
+    if (buf != curbuf) {  // autocommands deleted the buffer!
+      return NULL;
     }
     if (aborting()) {           // autocmds may abort script processing
       xfree(ffname);
       return NULL;
     }
-    if (buf == curbuf) {
-      // Make sure 'bufhidden' and 'buftype' are empty
-      clear_string_option(&buf->b_p_bh);
-      clear_string_option(&buf->b_p_bt);
-    }
   }
   if (buf != curbuf || curbuf == NULL) {
     buf = xcalloc(1, sizeof(buf_T));
@@ -1776,14 +1775,6 @@ buf_T *buflist_new(char_u *ffname_arg, char_u *sfname_arg, linenr_T lnum, int fl
   buf->b_wininfo = xcalloc(1, sizeof(wininfo_T));
 
   if (buf == curbuf) {
-    // free all things allocated for this buffer
-    buf_freeall(buf, 0);
-    if (buf != curbuf) {         // autocommands deleted the buffer!
-      return NULL;
-    }
-    if (aborting()) {           // autocmds may abort script processing
-      return NULL;
-    }
     free_buffer_stuff(buf, kBffInitChangedtick);  // delete local vars et al.
 
     // Init the options.
@@ -4855,6 +4846,10 @@ void do_arg_all(int count, int forceit, int keep_tabs)
             if (keep_tabs) {
               new_curwin = wp;
               new_curtab = curtab;
+            } else if (wp->w_frame->fr_parent != curwin->w_frame->fr_parent) {
+              emsg(_("E249: window layout changed unexpectedly"));
+              i = count;
+              break;
             } else {
               win_move_after(wp, curwin);
             }
diff --git a/src/nvim/buffer_defs.h b/src/nvim/buffer_defs.h
index 4917a628ff..baa0d1f102 100644
--- a/src/nvim/buffer_defs.h
+++ b/src/nvim/buffer_defs.h
@@ -532,6 +532,8 @@ struct file_buffer {
   int b_flags;                  // various BF_ flags
   int b_locked;                 // Buffer is being closed or referenced, don't
                                 // let autocommands wipe it out.
+  int b_locked_split;           // Buffer is being closed, don't allow opening
+                                // a new window with it.
   int b_ro_locked;              // Non-zero when the buffer can't be changed.
                                 // Used for FileChangedRO
 
diff --git a/src/nvim/ex_cmds.c b/src/nvim/ex_cmds.c
index 61bd9571b5..e16537c192 100644
--- a/src/nvim/ex_cmds.c
+++ b/src/nvim/ex_cmds.c
@@ -2497,16 +2497,19 @@ int do_ecmd(int fnum, char_u *ffname, char_u *sfname, exarg_T *eap, linenr_T new
       if (buf->b_fname != NULL) {
         new_name = vim_strsave(buf->b_fname);
       }
+      const bufref_T save_au_new_curbuf = au_new_curbuf;
       set_bufref(&au_new_curbuf, buf);
       apply_autocmds(EVENT_BUFLEAVE, NULL, NULL, false, curbuf);
       cmdwin_type = save_cmdwin_type;
       if (!bufref_valid(&au_new_curbuf)) {
         // New buffer has been deleted.
         delbuf_msg(new_name);  // Frees new_name.
+        au_new_curbuf = save_au_new_curbuf;
         goto theend;
       }
       if (aborting()) {             // autocmds may abort script processing
         xfree(new_name);
+        au_new_curbuf = save_au_new_curbuf;
         goto theend;
       }
       if (buf == curbuf) {  // already in new buffer
@@ -2540,12 +2543,14 @@ int do_ecmd(int fnum, char_u *ffname, char_u *sfname, exarg_T *eap, linenr_T new
         // autocmds may abort script processing
         if (aborting() && curwin->w_buffer != NULL) {
           xfree(new_name);
+          au_new_curbuf = save_au_new_curbuf;
           goto theend;
         }
         // Be careful again, like above.
         if (!bufref_valid(&au_new_curbuf)) {
           // New buffer has been deleted.
           delbuf_msg(new_name);  // Frees new_name.
+          au_new_curbuf = save_au_new_curbuf;
           goto theend;
         }
         if (buf == curbuf) {  // already in new buffer
@@ -2585,8 +2590,7 @@ int do_ecmd(int fnum, char_u *ffname, char_u *sfname, exarg_T *eap, linenr_T new
         did_get_winopts = true;
       }
       xfree(new_name);
-      au_new_curbuf.br_buf = NULL;
-      au_new_curbuf.br_buf_free_count = 0;
+      au_new_curbuf = save_au_new_curbuf;
     }
 
     curwin->w_pcmark.lnum = 1;
diff --git a/src/nvim/ex_getln.c b/src/nvim/ex_getln.c
index 91e93a236a..b7d75855d6 100644
--- a/src/nvim/ex_getln.c
+++ b/src/nvim/ex_getln.c
@@ -6376,6 +6376,7 @@ static int open_cmdwin(void)
   // Create a window for the command-line buffer.
   if (win_split((int)p_cwh, WSP_BOT) == FAIL) {
     beep_flush();
+    ga_clear(&winsizes);
     return K_IGNORE;
   }
   cmdwin_type = get_cmdline_type();
diff --git a/src/nvim/quickfix.c b/src/nvim/quickfix.c
index f8d2d37a91..a12fb70388 100644
--- a/src/nvim/quickfix.c
+++ b/src/nvim/quickfix.c
@@ -2313,7 +2313,10 @@ static bool qflist_valid(win_T *wp, unsigned int qf_id)
   qf_info_T *qi = &ql_info;
 
   if (wp) {
-    qi = GET_LOC_LIST(wp);
+    if (!win_valid(wp)) {
+      return false;
+    }
+    qi = GET_LOC_LIST(wp);  // Location list
     if (!qi) {
       return false;
     }
diff --git a/src/nvim/testdir/test_autocmd.vim b/src/nvim/testdir/test_autocmd.vim
index 228145ec4d..13be82a71d 100644
--- a/src/nvim/testdir/test_autocmd.vim
+++ b/src/nvim/testdir/test_autocmd.vim
@@ -2695,9 +2695,9 @@ func Test_autocmd_closes_window()
   au BufNew,BufWinLeave * e %e
   file yyy
   au BufNew,BufWinLeave * ball
-  call assert_fails('n xxx', 'E143:')
+  n xxx
 
-  bwipe %
+  %bwipe
   au! BufNew
   au! BufWinLeave
 endfunc
@@ -2713,9 +2713,34 @@ func Test_autocmd_quit_psearch()
   augroup aucmd_win_test
     au!
   augroup END
+  new
+  pclose
+endfunc
+
+" Fuzzer found some strange combination that caused a crash.
+func Test_autocmd_normal_mess()
+  " For unknown reason this hangs on MS-Windows
+  CheckNotMSWindows
+
+  augroup aucmd_normal_test
+    au BufLeave,BufWinLeave,BufHidden,BufUnload,BufDelete,BufWipeout * norm 7q/qc
+  augroup END
+  " Nvim has removed :open
+  " call assert_fails('o4', 'E1159')
+  call assert_fails('e4', 'E1159')
+  silent! H
+  call assert_fails('e xx', 'E1159')
+  normal G
+
+  augroup aucmd_normal_test
+    au!
+  augroup END
 endfunc
 
 func Test_autocmd_closing_cmdwin()
+  " For unknown reason this hangs on MS-Windows
+  CheckNotMSWindows
+
   au BufWinLeave * nested q
   call assert_fails("norm 7q?\n", 'E855:')
 
@@ -2724,6 +2749,20 @@ func Test_autocmd_closing_cmdwin()
   only
 endfunc
 
+func Test_autocmd_vimgrep()
+  augroup aucmd_vimgrep
+    au QuickfixCmdPre,BufNew,BufReadCmd * sb
+    " Nvim makes aucmd_win the last window
+    " au QuickfixCmdPre,BufNew,BufReadCmd * q9
+    au QuickfixCmdPre,BufNew,BufReadCmd * exe 'q' .. (winnr('$') - (win_gettype(winnr('$')) == 'autocmd'))
+  augroup END
+  call assert_fails('lv ?a? foo', 'E926:')
+
+  augroup aucmd_vimgrep
+    au!
+  augroup END
+endfunc
+
 func Test_bufwipeout_changes_window()
   " This should not crash, but we don't have any expectations about what
   " happens, changing window in BufWipeout has unpredictable results.
@@ -2759,4 +2798,22 @@ func Test_v_event_readonly()
 endfunc
 
 
+func Test_noname_autocmd()
+  augroup test_noname_autocmd_group
+    autocmd!
+    autocmd BufEnter * call add(s:li, ["BufEnter", expand("<afile>")])
+    autocmd BufDelete * call add(s:li, ["BufDelete", expand("<afile>")])
+    autocmd BufLeave * call add(s:li, ["BufLeave", expand("<afile>")])
+    autocmd BufUnload * call add(s:li, ["BufUnload", expand("<afile>")])
+    autocmd BufWipeout * call add(s:li, ["BufWipeout", expand("<afile>")])
+  augroup END
+
+  let s:li = []
+  edit foo
+  call assert_equal([['BufUnload', ''], ['BufDelete', ''], ['BufWipeout', ''], ['BufEnter', 'foo']], s:li)
+
+  au! test_noname_autocmd_group
+  augroup! test_noname_autocmd_group
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab
diff --git a/src/nvim/testdir/test_window_cmd.vim b/src/nvim/testdir/test_window_cmd.vim
index ef6dec580f..798122dc5d 100644
--- a/src/nvim/testdir/test_window_cmd.vim
+++ b/src/nvim/testdir/test_window_cmd.vim
@@ -513,14 +513,15 @@ func Test_window_colon_command()
 endfunc
 
 func Test_access_freed_mem()
+  call assert_equal(&columns, winwidth(0))
   " This was accessing freed memory (but with what events?)
   au BufEnter,BufLeave,WinEnter,WinLeave 0 vs xxx
   arg 0
   argadd
-  all
-  all
+  call assert_fails("all", "E242:")
   au!
   bwipe xxx
+  call assert_equal(&columns, winwidth(0))
 endfunc
 
 func Test_visual_cleared_after_window_split()
diff --git a/src/nvim/window.c b/src/nvim/window.c
index 2ca5128445..f68cfe4c9c 100644
--- a/src/nvim/window.c
+++ b/src/nvim/window.c
@@ -72,8 +72,40 @@ typedef enum {
   WEE_TRIGGER_LEAVE_AUTOCMDS = 0x10,
 } wee_flags_T;
 
+static char e_cannot_split_window_when_closing_buffer[]
+  = N_("E1159: Cannot split a window when closing the buffer");
+
 static char *m_onlyone = N_("Already only one window");
 
+/// When non-zero splitting a window is forbidden.  Used to avoid that nasty
+/// autocommands mess up the window structure.
+static int split_disallowed = 0;
+
+// #define WIN_DEBUG
+#ifdef WIN_DEBUG
+/// Call this method to log the current window layout.
+static void log_frame_layout(frame_T *frame)
+{
+  DLOG("layout %s, wi: %d, he: %d, wwi: %d, whe: %d, id: %d",
+       frame->fr_layout == FR_LEAF ? "LEAF" : frame->fr_layout == FR_ROW ? "ROW" : "COL",
+       frame->fr_width,
+       frame->fr_height,
+       frame->fr_win == NULL ? -1 : frame->fr_win->w_width,
+       frame->fr_win == NULL ? -1 : frame->fr_win->w_height,
+       frame->fr_win == NULL ? -1 : frame->fr_win->w_id);
+  if (frame->fr_child != NULL) {
+    DLOG("children");
+    log_frame_layout(frame->fr_child);
+    if (frame->fr_next != NULL) {
+      DLOG("END of children");
+    }
+  }
+  if (frame->fr_next != NULL) {
+    log_frame_layout(frame->fr_next);
+  }
+}
+#endif
+
 /// @return the current window, unless in the cmdline window and "prevwin" is
 /// set, then return "prevwin".
 win_T *prevwin_curwin(void)
@@ -909,6 +941,21 @@ void ui_ext_win_viewport(win_T *wp)
   }
 }
 
+/// If "split_disallowed" is set given an error and return FAIL.
+/// Otherwise return OK.
+static int check_split_disallowed(void)
+{
+  if (split_disallowed > 0) {
+    emsg(_("E242: Can't split a window while closing another"));
+    return FAIL;
+  }
+  if (curwin->w_buffer->b_locked_split) {
+    emsg(_(e_cannot_split_window_when_closing_buffer));
+    return FAIL;
+  }
+  return OK;
+}
+
 /*
  * split the current window, implements CTRL-W s and :split
  *
@@ -926,6 +973,10 @@ void ui_ext_win_viewport(win_T *wp)
  */
 int win_split(int size, int flags)
 {
+  if (check_split_disallowed() == FAIL) {
+    return FAIL;
+  }
+
   // When the ":tab" modifier was used open a new tab page instead.
   if (may_open_tabpage() == OK) {
     return OK;
@@ -1886,6 +1937,9 @@ static void win_totop(int size, int flags)
   if (curwin == aucmd_win) {
     return;
   }
+  if (check_split_disallowed() == FAIL) {
+    return;
+  }
 
   if (curwin->w_floating) {
     ui_comp_remove_grid(&curwin->w_grid_alloc);
@@ -1929,6 +1983,11 @@ void win_move_after(win_T *win1, win_T *win2)
 
   // check if there is something to do
   if (win2->w_next != win1) {
+    if (win1->w_frame->fr_parent != win2->w_frame->fr_parent) {
+      iemsg("INTERNAL: trying to move a window into another frame");
+      return;
+    }
+
     // may need move the status line, horizontal or vertical separator of the last window
     if (win1 == lastwin) {
       height = win1->w_prev->w_status_height;
@@ -2742,6 +2801,10 @@ int win_close(win_T *win, bool free_buf, bool force)
     return FAIL;
   }
 
+  // Now we are really going to close the window.  Disallow any autocommand
+  // to split a window to avoid trouble.
+  split_disallowed++;
+
   // let terminal buffers know that this window dimensions may be ignored
   win->w_closing = true;
 
@@ -2809,6 +2872,8 @@ int win_close(win_T *win, bool free_buf, bool force)
     }
   }
 
+  split_disallowed--;
+
   /*
    * If last window has a status line now and we don't want one,
    * remove the status line.