patch 8.0.1420: accessing freed memory in vimgrepv8.0.1420

Problem:    Accessing freed memory in vimgrep.
Solution:   Check that the quickfix list is still valid. (Yegappan Lakshmanan,
            closes #2474)

4 files changed, 133 insertions, 23 deletions

diff --git a/src/quickfix.c b/src/quickfix.c
index 5deaed531..05e594628 100644
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -144,6 +144,7 @@ static int	qf_get_fnum(qf_info_T *qi, int qf_idx, char_u *, char_u *);
 static char_u	*qf_push_dir(char_u *, struct dir_stack_T **, int is_file_stack);
 static char_u	*qf_pop_dir(struct dir_stack_T **);
 static char_u	*qf_guess_filepath(qf_info_T *qi, int qf_idx, char_u *);
+static int	qflist_valid(win_T *wp, int_u qf_id);
 static void	qf_fmt_text(char_u *text, char_u *buf, int bufsize);
 static void	qf_clean_dir_stack(struct dir_stack_T **);
 static int	qf_win_pos_update(qf_info_T *qi, int old_qf_index);
@@ -177,6 +178,9 @@ static qf_info_T *ll_get_or_alloc_list(win_T *);
 static char_u   *qf_last_bufname = NULL;
 static bufref_T  qf_last_bufref = {NULL, 0, 0};
 
+static char	*e_loc_list_changed =
+				 N_("E926: Current location list was changed");
+
 /*
  * Read the errorfile "efile" into memory, line by line, building the error
  * list. Set the error list's title to qf_title.
@@ -1928,6 +1932,29 @@ qf_guess_filepath(qf_info_T *qi, int qf_idx, char_u *filename)
 }
 
 /*
+ * Returns TRUE if a quickfix/location list with the given identifier exists.
+ */
+    static int
+qflist_valid (win_T *wp, int_u qf_id)
+{
+    qf_info_T	*qi = &ql_info;
+    int		i;
+
+    if (wp != NULL)
+    {
+	qi = GET_LOC_LIST(wp);	    /* Location list */
+	if (qi == NULL)
+	    return FALSE;
+    }
+
+    for (i = 0; i < qi->qf_listcount; ++i)
+	if (qi->qf_lists[i].qf_id == qf_id)
+	    return TRUE;
+
+    return FALSE;
+}
+
+/*
  * When loading a file from the quickfix, the auto commands may modify it.
  * This may invalidate the current quickfix entry.  This function checks
  * whether a entry is still present in the quickfix.
@@ -2343,14 +2370,28 @@ qf_jump_edit_buffer(
     else
     {
 	int old_qf_curlist = qi->qf_curlist;
+	int save_qfid = qi->qf_lists[qi->qf_curlist].qf_id;
 
 	retval = buflist_getfile(qf_ptr->qf_fnum,
 		(linenr_T)1, GETF_SETMARK | GETF_SWITCH, forceit);
-	if (qi != &ql_info && !win_valid_any_tab(oldwin))
+
+	if (qi != &ql_info)
 	{
-	    EMSG(_("E924: Current window was closed"));
-	    *abort = TRUE;
-	    *opened_window = FALSE;
+	    /*
+	     * Location list. Check whether the associated window is still
+	     * present and the list is still valid.
+	     */
+	    if (!win_valid_any_tab(oldwin))
+	    {
+		EMSG(_("E924: Current window was closed"));
+		*abort = TRUE;
+		*opened_window = FALSE;
+	    }
+	    else if (!qflist_valid(oldwin, save_qfid))
+	    {
+		EMSG(_(e_loc_list_changed));
+		*abort = TRUE;
+	    }
 	}
 	else if (old_qf_curlist != qi->qf_curlist
 		|| !is_qf_entry_present(qi, qf_ptr))
@@ -2358,7 +2399,7 @@ qf_jump_edit_buffer(
 	    if (qi == &ql_info)
 		EMSG(_("E925: Current quickfix was changed"));
 	    else
-		EMSG(_("E926: Current location list was changed"));
+		EMSG(_(e_loc_list_changed));
 	    *abort = TRUE;
 	}
 
@@ -4065,6 +4106,7 @@ ex_cfile(exarg_T *eap)
     qf_info_T	*qi = &ql_info;
 #ifdef FEAT_AUTOCMD
     char_u	*au_name = NULL;
+    int		save_qfid;
 #endif
     int		res;
 
@@ -4122,8 +4164,15 @@ ex_cfile(exarg_T *eap)
     if (res >= 0 && qi != NULL)
 	qf_list_changed(qi, qi->qf_curlist);
 #ifdef FEAT_AUTOCMD
+    save_qfid = qi->qf_lists[qi->qf_curlist].qf_id;
     if (au_name != NULL)
 	apply_autocmds(EVENT_QUICKFIXCMDPOST, au_name, NULL, FALSE, curbuf);
+    /*
+     * Autocmd might have freed the quickfix/location list. Check whether it is
+     * still valid
+     */
+    if (!qflist_valid(wp, save_qfid))
+	return;
 #endif
     if (res > 0 && (eap->cmdidx == CMD_cfile || eap->cmdidx == CMD_lfile))
     {
@@ -4149,8 +4198,11 @@ ex_vimgrep(exarg_T *eap)
     char_u	*p;
     int		fi;
     qf_info_T	*qi = &ql_info;
+    int		loclist_cmd = FALSE;
 #ifdef FEAT_AUTOCMD
+    int_u	save_qfid;
     qfline_T	*cur_qf_start;
+    win_T	*wp;
 #endif
     long	lnum;
     buf_T	*buf;
@@ -4204,6 +4256,7 @@ ex_vimgrep(exarg_T *eap)
 	qi = ll_get_or_alloc_list(curwin);
 	if (qi == NULL)
 	    return;
+	loclist_cmd = TRUE;
     }
 
     if (eap->addr_count > 0)
@@ -4274,8 +4327,9 @@ ex_vimgrep(exarg_T *eap)
     mch_dirname(dirname_start, MAXPATHL);
 
 #ifdef FEAT_AUTOCMD
-     /* Remember the value of qf_start, so that we can check for autocommands
-      * changing the current quickfix list. */
+     /* Remember the current values of the quickfix list and qf_start, so that
+      * we can check for autocommands changing the current quickfix list. */
+    save_qfid = qi->qf_lists[qi->qf_curlist].qf_id;
     cur_qf_start = qi->qf_lists[qi->qf_curlist].qf_start;
 #endif
 
@@ -4335,6 +4389,18 @@ ex_vimgrep(exarg_T *eap)
 	    using_dummy = FALSE;
 
 #ifdef FEAT_AUTOCMD
+	if (loclist_cmd)
+	{
+	    /*
+	     * Verify that the location list is still valid. An autocmd might
+	     * have freed the location list.
+	     */
+	    if (!qflist_valid(curwin, save_qfid))
+	    {
+		EMSG(_(e_loc_list_changed));
+		goto theend;
+	    }
+	}
 	if (cur_qf_start != qi->qf_lists[qi->qf_curlist].qf_start)
 	{
 	    int idx;
@@ -4491,6 +4557,13 @@ ex_vimgrep(exarg_T *eap)
     if (au_name != NULL)
 	apply_autocmds(EVENT_QUICKFIXCMDPOST, au_name,
 					       curbuf->b_fname, TRUE, curbuf);
+    /*
+     * The QuickFixCmdPost autocmd may free the quickfix list. Check the list
+     * is still valid.
+     */
+    wp = loclist_cmd ? curwin : NULL;
+    if (!qflist_valid(wp, save_qfid))
+	goto theend;
 #endif
 
     /* Jump to first match. */
@@ -5543,7 +5616,8 @@ ex_cbuffer(exarg_T *eap)
 #endif
 
     /* Must come after autocommands. */
-    if (eap->cmdidx == CMD_lbuffer || eap->cmdidx == CMD_lgetbuffer
+    if (eap->cmdidx == CMD_lbuffer
+	    || eap->cmdidx == CMD_lgetbuffer
 	    || eap->cmdidx == CMD_laddbuffer)
     {
 	qi = ll_get_or_alloc_list(curwin);
@@ -5614,14 +5688,6 @@ ex_cexpr(exarg_T *eap)
 #endif
     int		res;
 
-    if (eap->cmdidx == CMD_lexpr || eap->cmdidx == CMD_lgetexpr
-	    || eap->cmdidx == CMD_laddexpr)
-    {
-	qi = ll_get_or_alloc_list(curwin);
-	if (qi == NULL)
-	    return;
-    }
-
 #ifdef FEAT_AUTOCMD
     switch (eap->cmdidx)
     {
@@ -5643,6 +5709,15 @@ ex_cexpr(exarg_T *eap)
     }
 #endif
 
+    if (eap->cmdidx == CMD_lexpr
+	    || eap->cmdidx == CMD_lgetexpr
+	    || eap->cmdidx == CMD_laddexpr)
+    {
+	qi = ll_get_or_alloc_list(curwin);
+	if (qi == NULL)
+	    return;
+    }
+
     /* Evaluate the expression.  When the result is a string or a list we can
      * use it to fill the errorlist. */
     tv = eval_expr(eap->arg, NULL);
diff --git a/src/testdir/test_autocmd.vim b/src/testdir/test_autocmd.vim
index 0636c4721..d8fddffeb 100644
--- a/src/testdir/test_autocmd.vim
+++ b/src/testdir/test_autocmd.vim
@@ -1178,10 +1178,3 @@ func Test_nocatch_wipe_dummy_buffer()
   call assert_fails('lv½ /x', 'E480')
   au!
 endfunc
-
-func Test_wipe_cbuffer()
-  sv x
-  au * * bw
-  lb
-  au!
-endfunc
diff --git a/src/testdir/test_quickfix.vim b/src/testdir/test_quickfix.vim
index a09ec738e..0787e60e3 100644
--- a/src/testdir/test_quickfix.vim
+++ b/src/testdir/test_quickfix.vim
@@ -3038,3 +3038,43 @@ func Test_lfile_crash()
   call assert_fails('lfile', 'E40')
   au! QuickFixCmdPre
 endfunc
+
+" The following test used to crash vim
+func Test_lbuffer_crash()
+  sv Xtest
+  augroup QF_Test
+    au!
+    au * * bw
+  augroup END
+  lbuffer
+  augroup QF_Test
+    au!
+  augroup END
+endfunc
+
+" The following test used to crash vim
+func Test_lexpr_crash()
+  augroup QF_Test
+    au!
+    au * * call setloclist(0, [], 'f')
+  augroup END
+  lexpr ""
+  augroup QF_Test
+    au!
+  augroup END
+  enew | only
+endfunc
+
+" The following test used to crash Vim
+func Test_lvimgrep_crash()
+  sv Xtest
+  augroup QF_Test
+    au!
+    au * * call setloclist(0, [], 'f')
+  augroup END
+  lvimgrep quickfix test_quickfix.vim
+  augroup QF_Test
+    au!
+  augroup END
+  enew | only
+endfunc
diff --git a/src/version.c b/src/version.c
index 6ae213535..1a217e7b6 100644
--- a/src/version.c
+++ b/src/version.c
@@ -772,6 +772,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    1420,
+/**/
     1419,
 /**/
     1418,