Hi Bram and developers,
I checked in 7.4.2321
==== Case 1 ====
How to reproduce:
- Create the following file:
$ cat sample1.vim
edit a.txt
augroup sample
autocmd!
autocmd BufUnload <buffer> tabfirst | 2bwipeout!
augroup END
edit b.txt
- Run vanilla Vim with above script file
$ vim -Nu NONE -S sample1.vim
Expected behavior:
SEGV does not occur.
Actual behavior:
SEGVed.
==== Case 2 ====
How to reproduce:
- Create the following file:
$ cat sample2.vim
setlocal buftype=nowrite
augroup sample
autocmd!
autocmd BufUnload <buffer> tabfirst | 2bwipeout
augroup END
normal! i1
edit a.txt
call feedkeys("\<CR>")
- Run vanilla Vim with above script file
$ vim -Nu NONE -S sample2.vim
Expected behavior:
SEGV does not occur.
Actual behavior:
SEGVed.
I know there are rare case and salicious scripts.
But, It is not good to SEGV.
I wrote a patch. --> `fix_autocmd_bufunload_with_bwipe.patch`
check it out.
I've also written test. --> `autocmd_bufunload_with_bwipe_test.patch`
Unfortunately, it did not SEGV in the pre-patch binary :-/
NOTE: This issue was reported by Norio Takagi. (Thanks!)
--
Best regards,
Hirohito Higashi (a.k.a. h_east)
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
diff --git a/src/buffer.c b/src/buffer.c
index 9270c39..bef3d32 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -499,6 +499,10 @@ close_buffer(
/* When the buffer is no longer in a window, trigger BufWinLeave */
if (buf->b_nwindows == 1)
{
+# ifdef FEAT_WINDOWS
+ tabpage_T *save_curtab = curtab;
+# endif
+
buf->b_closing = TRUE;
if (apply_autocmds(EVENT_BUFWINLEAVE, buf->b_fname, buf->b_fname,
FALSE, buf)
@@ -533,6 +537,10 @@ aucmd_abort:
if (aborting()) /* autocmds may abort script processing */
return;
# endif
+# ifdef FEAT_WINDOWS
+ if (save_curtab != curtab)
+ return;
+# endif
}
nwindows = buf->b_nwindows;
#endif
@@ -561,7 +569,9 @@ aucmd_abort:
buf->b_nwindows = nwindows;
#endif
- buf_freeall(buf, (del_buf ? BFA_DEL : 0) + (wipe_buf ? BFA_WIPE : 0));
+ if (buf_freeall(buf, (del_buf ? BFA_DEL : 0) + (wipe_buf ? BFA_WIPE : 0))
+ == FAIL)
+ return;
#ifdef FEAT_AUTOCMD
/* Autocommands may have deleted the buffer. */
@@ -671,8 +681,9 @@ buf_clear_file(buf_T *buf)
* BFA_DEL buffer is going to be deleted
* BFA_WIPE buffer is going to be wiped out
* BFA_KEEP_UNDO do not free undo information
+ * Return FAIL for failure, OK otherwise.
*/
- void
+ int
buf_freeall(buf_T *buf, int flags)
{
#ifdef FEAT_AUTOCMD
@@ -693,7 +704,7 @@ buf_freeall(buf_T *buf, int flags)
FALSE, buf)
&& !bufref_valid(&bufref))
/* autocommands deleted the buffer */
- return;
+ return FAIL;
}
if ((flags & BFA_DEL) && buf->b_p_bl)
{
@@ -701,7 +712,7 @@ buf_freeall(buf_T *buf, int flags)
FALSE, buf)
&& !bufref_valid(&bufref))
/* autocommands deleted the buffer */
- return;
+ return FAIL;
}
if (flags & BFA_WIPE)
{
@@ -709,7 +720,7 @@ buf_freeall(buf_T *buf, int flags)
FALSE, buf)
&& !bufref_valid(&bufref))
/* autocommands deleted the buffer */
- return;
+ return FAIL;
}
buf->b_closing = FALSE;
@@ -717,7 +728,7 @@ buf_freeall(buf_T *buf, int flags)
/* 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
* "tabnext" BufUnload autocmd leaves a window behind without a buffer. */
- if (is_curwin && curwin != the_curwin && win_valid_any_tab(the_curwin))
+ if (is_curwin && curwin != the_curwin && win_valid_any_tab(the_curwin))
{
block_autocmds();
goto_tabpage_win(the_curtab, the_curwin);
@@ -727,7 +738,7 @@ buf_freeall(buf_T *buf, int flags)
# ifdef FEAT_EVAL
if (aborting()) /* autocmds may abort script processing */
- return;
+ return FAIL;
# endif
/*
@@ -737,7 +748,7 @@ buf_freeall(buf_T *buf, int flags)
* Therefore only return if curbuf changed to the deleted buffer.
*/
if (buf == curbuf && !is_curbuf)
- return;
+ return FAIL;
#endif
#ifdef FEAT_DIFF
diff_buf_delete(buf); /* Can't use 'diff' for unloaded buffer. */
@@ -779,6 +790,7 @@ buf_freeall(buf_T *buf, int flags)
syntax_clear(&buf->b_s); /* reset syntax info */
#endif
buf->b_flags &= ~BF_READERR; /* a read error is no longer relevant */
+ return OK;
}
/*
@@ -1960,7 +1972,8 @@ buflist_new(
if (buf == curbuf)
{
/* free all things allocated for this buffer */
- buf_freeall(buf, 0);
+ if (buf_freeall(buf, 0) == FAIL)
+ return NULL;
if (buf != curbuf) /* autocommands deleted the buffer! */
return NULL;
#if defined(FEAT_AUTOCMD) && defined(FEAT_EVAL)
diff --git a/src/ex_cmds.c b/src/ex_cmds.c
index 61ab2ab..b7f749e 100644
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -4101,7 +4101,13 @@ do_ecmd(
goto theend;
}
u_unchanged(curbuf);
- buf_freeall(curbuf, BFA_KEEP_UNDO);
+ if (buf_freeall(curbuf, BFA_KEEP_UNDO) == FAIL)
+ {
+#ifdef FEAT_AUTOCMD
+ vim_free(new_name);
+#endif
+ goto theend;
+ }
/* tell readfile() not to clear or reload undo info */
readfile_flags = READ_KEEP_UNDO;
@@ -4397,6 +4403,11 @@ delbuf_msg(char_u *name)
vim_free(name);
au_new_curbuf.br_buf = NULL;
au_new_curbuf.br_buf_free_count = 0;
+
+# ifdef FEAT_WINDOWS
+ if (curwin->w_buffer == NULL)
+ enter_buffer(curbuf);
+# endif
}
#endif
diff --git a/src/proto/buffer.pro b/src/proto/buffer.pro
index 183f79a..dbe152b 100644
--- a/src/proto/buffer.pro
+++ b/src/proto/buffer.pro
@@ -5,7 +5,7 @@ int bufref_valid(bufref_T *bufref);
int buf_valid(buf_T *buf);
void close_buffer(win_T *win, buf_T *buf, int action, int abort_if_last);
void buf_clear_file(buf_T *buf);
-void buf_freeall(buf_T *buf, int flags);
+int buf_freeall(buf_T *buf, int flags);
void goto_buffer(exarg_T *eap, int start, int dir, int count);
void handle_swap_exists(bufref_T *old_curbuf);
char_u *do_bufdel(int command, char_u *arg, int addr_count, int start_bnr, int end_bnr, int forceit);
diff --git a/src/window.c b/src/window.c
index 4f99271..1e74928 100644
--- a/src/window.c
+++ b/src/window.c
@@ -2305,7 +2305,12 @@ win_close(win_T *win, int free_buf)
* and then close the window and the tab page to avoid that curwin and
* curtab are invalid while we are freeing memory. */
if (close_last_window_tabpage(win, free_buf, prev_curtab))
- return FAIL;
+ {
+ /* Autocommands have close curwin's buf. Restore curwin->w_buffer */
+ if (win_valid(curwin) && curwin->w_buffer == NULL)
+ curwin->w_buffer = curbuf;
+ return FAIL;
+ }
/* When closing the help window, try restoring a snapshot after closing
* the window. Otherwise clear the snapshot, it's now invalid. */
diff --git a/src/testdir/test_autocmd.vim b/src/testdir/test_autocmd.vim
index f05a55f..23cd78f 100644
--- a/src/testdir/test_autocmd.vim
+++ b/src/testdir/test_autocmd.vim
@@ -77,11 +77,47 @@ function Test_autocmd_bufunload_with_tabnext()
quit
call assert_equal(2, tabpagenr('$'))
+ autocmd! test_autocmd_bufunload_with_tabnext_group
augroup! test_autocmd_bufunload_with_tabnext_group
tablast
quit
endfunc
+" SEGV occurs in older versions. (At least 7.4.2321 or older)
+function Test_autocmd_bufunload_avoiding_SEGV_01()
+ edit a.txt
+
+ augroup test_autocmd_bufunload
+ autocmd!
+ autocmd BufUnload <buffer> tabfirst | 2bwipeout!
+ augroup END
+
+ edit b.txt
+ call assert_true(v:true)
+
+ autocmd! test_autocmd_bufunload
+ augroup! test_autocmd_bufunload
+ bwipe!
+endfunc
+
+" SEGV occurs in older versions. (At least 7.4.2321 or older)
+function Test_autocmd_bufunload_avoiding_SEGV_02()
+ setlocal buftype=nowrite
+
+ augroup test_autocmd_bufunload
+ autocmd!
+ autocmd BufUnload <buffer> tabfirst | 2bwipeout
+ augroup END
+
+ normal! i1
+ call assert_fails('edit a.txt', 'E517:')
+ call feedkeys("\<CR>")
+
+ autocmd! test_autocmd_bufunload
+ augroup! test_autocmd_bufunload
+ bwipe!
+endfunc
+
func Test_win_tab_autocmd()
let g:record = []
@@ -196,3 +232,4 @@ func Test_augroup_deleted()
au! VimEnter
endfunc
+" vim: shiftwidth=2 sts=2 expandtab
