Re: [patch] SEGV when autocmd BufUnload with bwipe

2016-09-04 Fir de Conversatie Bram Moolenaar

Hirohito Higashi wrote:

> 2016-9-4(Sun) 21:33:17 UTC+9 h_east:
> > 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  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  tabfirst | 2bwipeout
> > augroup END
> > normal! i1
> > edit a.txt
> > call feedkeys("\")
> > 
> > - 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!)
> 
> My patch also fixed the following case.
> 
>  Case 3 
> How to reproduce:
> - Create the following file:
>   $ cat sample3.vim
> tabedit
> augroup sample
>   autocmd!
>   autocmd BufWinLeave  tabfirst
> augroup END
> :%!ls
> edit! a.txt
> normal! gt
> :%!ls
> call feedkeys("\q::q\")
> 
> 
> - Run vanilla Vim with above script file
>   $ vim -Nu NONE -S sample3.vim
> 
> Expected behavior:
> SEGV does not occur.
> 
> Actual behavior:
> SEGVed.

That can be simplified to:

tabedit
augroup sample
  autocmd!
  autocmd BufWinLeave  tabfirst
augroup END
call setline(1, ['a', 'b', 'c'])
edit! a.txt

It still crashes.  Need to check the window changed also for
BufWinLeave.

-- 
"Hit any key to continue" is very confusing when you have two keyboards.

 /// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

-- 
-- 
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.


Re: [patch] SEGV when autocmd BufUnload with bwipe

2016-09-04 Fir de Conversatie Bram Moolenaar

Hirohito Higashi wrote:

> 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  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  tabfirst | 2bwipeout
> augroup END
> normal! i1
> edit a.txt
> call feedkeys("\")
> 
> - 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 think we need a more drastic solution.  Autocommands wiping out a
buffer that we rely on should not happen.  We already had the b_closing
flag, let's turn that into a b_locked flag.

> I've also written test.  --> `autocmd_bufunload_with_bwipe_test.patch`
> Unfortunately, it did not SEGV in the pre-patch binary :-/

That's because it deletes buffer 2, but in the test the buffer number
will be higher.  I managed to get the crash using bufnr('$') + 1.

> NOTE: This issue was reported by Norio Takagi.  (Thanks!)

-- 
You're as much use as a condom machine at the Vatican.
  -- Rimmer to Holly in Red Dwarf 'Queeg'

 /// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

-- 
-- 
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.


Re: [patch] SEGV when autocmd BufUnload with bwipe

2016-09-04 Fir de Conversatie h_east
Hi Bram,

2016-9-4(Sun) 21:33:17 UTC+9 h_east:
> 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  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  tabfirst | 2bwipeout
> augroup END
> normal! i1
> edit a.txt
> call feedkeys("\")
> 
> - 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!)

My patch also fixed the following case.

 Case 3 
How to reproduce:
- Create the following file:
  $ cat sample3.vim
tabedit
augroup sample
  autocmd!
  autocmd BufWinLeave  tabfirst
augroup END
:%!ls
edit! a.txt
normal! gt
:%!ls
call feedkeys("\q::q\")


- Run vanilla Vim with above script file
  $ vim -Nu NONE -S sample3.vim

Expected behavior:
SEGV does not occur.

Actual behavior:
SEGVed.


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.


[patch] SEGV when autocmd BufUnload with bwipe

2016-09-04 Fir de Conversatie h_east
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  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  tabfirst | 2bwipeout
augroup END
normal! i1
edit a.txt
call feedkeys("\")

- 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())
 	/* 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())
 	/* 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())
 	/* 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(>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