# HG changeset patch # User Christian Brabandt # Date 1513886406 -3600 # Node ID a0c6910e7fa4240527a3c6222bda57995d06374f # Parent 5127f347a3e49e7c82124d160cf0f4b643cf73a7 patch 8.0.1420: accessing freed memory in vimgrep commit https://github.com/vim/vim/commit/3c09722600e3218905b5d4a7b635a9e6560f87b3 Author: Bram Moolenaar Date: Thu Dec 21 20:54:49 2017 +0100 patch 8.0.1420: accessing freed memory in vimgrep Problem: Accessing freed memory in vimgrep. Solution: Check that the quickfix list is still valid. (Yegappan Lakshmanan, closes #2474) diff --git a/src/quickfix.c b/src/quickfix.c --- a/src/quickfix.c +++ b/src/quickfix.c @@ -144,6 +144,7 @@ static int qf_get_fnum(qf_info_T *qi, in 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(w 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_ } /* + * 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 --- 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 --- 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 --- 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,