To: vim_dev@googlegroups.com Subject: Patch 8.0.1420 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.0.1420 Problem: Accessing freed memory in vimgrep. Solution: Check that the quickfix list is still valid. (Yegappan Lakshmanan, closes #2474) Files: src/quickfix.c, src/testdir/test_autocmd.vim, src/testdir/test_quickfix.vim *** ../vim-8.0.1419/src/quickfix.c 2017-12-19 16:48:50.926397195 +0100 --- src/quickfix.c 2017-12-21 20:52:46.422018472 +0100 *************** *** 144,149 **** --- 144,150 ---- 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,182 **** --- 178,186 ---- 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,1933 **** --- 1932,1960 ---- } /* + * 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,2356 **** else { int old_qf_curlist = qi->qf_curlist; retval = buflist_getfile(qf_ptr->qf_fnum, (linenr_T)1, GETF_SETMARK | GETF_SWITCH, forceit); ! if (qi != &ql_info && !win_valid_any_tab(oldwin)) { ! EMSG(_("E924: Current window was closed")); ! *abort = TRUE; ! *opened_window = FALSE; } else if (old_qf_curlist != qi->qf_curlist || !is_qf_entry_present(qi, qf_ptr)) --- 2370,2397 ---- 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) { ! /* ! * 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,2364 **** if (qi == &ql_info) EMSG(_("E925: Current quickfix was changed")); else ! EMSG(_("E926: Current location list was changed")); *abort = TRUE; } --- 2399,2405 ---- if (qi == &ql_info) EMSG(_("E925: Current quickfix was changed")); else ! EMSG(_(e_loc_list_changed)); *abort = TRUE; } *************** *** 4065,4070 **** --- 4106,4112 ---- qf_info_T *qi = &ql_info; #ifdef FEAT_AUTOCMD char_u *au_name = NULL; + int save_qfid; #endif int res; *************** *** 4122,4129 **** --- 4164,4178 ---- 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,4156 **** --- 4198,4208 ---- 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,4209 **** --- 4256,4262 ---- qi = ll_get_or_alloc_list(curwin); if (qi == NULL) return; + loclist_cmd = TRUE; } if (eap->addr_count > 0) *************** *** 4274,4281 **** 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. */ cur_qf_start = qi->qf_lists[qi->qf_curlist].qf_start; #endif --- 4327,4335 ---- mch_dirname(dirname_start, MAXPATHL); #ifdef FEAT_AUTOCMD ! /* 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,4340 **** --- 4389,4406 ---- 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,4496 **** --- 4557,4569 ---- 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,5549 **** #endif /* Must come after autocommands. */ ! if (eap->cmdidx == CMD_lbuffer || eap->cmdidx == CMD_lgetbuffer || eap->cmdidx == CMD_laddbuffer) { qi = ll_get_or_alloc_list(curwin); --- 5616,5623 ---- #endif /* Must come after autocommands. */ ! if (eap->cmdidx == CMD_lbuffer ! || eap->cmdidx == CMD_lgetbuffer || eap->cmdidx == CMD_laddbuffer) { qi = ll_get_or_alloc_list(curwin); *************** *** 5614,5627 **** #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) { --- 5688,5693 ---- *************** *** 5643,5648 **** --- 5709,5723 ---- } #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); *** ../vim-8.0.1419/src/testdir/test_autocmd.vim 2017-12-19 16:41:09.309369068 +0100 --- src/testdir/test_autocmd.vim 2017-12-21 20:46:30.972631215 +0100 *************** *** 1178,1187 **** call assert_fails('lvĀ½ /x', 'E480') au! endfunc - - func Test_wipe_cbuffer() - sv x - au * * bw - lb - au! - endfunc --- 1178,1180 ---- *** ../vim-8.0.1419/src/testdir/test_quickfix.vim 2017-12-19 16:48:50.926397195 +0100 --- src/testdir/test_quickfix.vim 2017-12-21 20:46:30.972631215 +0100 *************** *** 3038,3040 **** --- 3038,3080 ---- 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 *** ../vim-8.0.1419/src/version.c 2017-12-21 20:27:40.768178638 +0100 --- src/version.c 2017-12-21 20:54:37.085259565 +0100 *************** *** 773,774 **** --- 773,776 ---- { /* Add new patch number below this line */ + /**/ + 1420, /**/ -- hundred-and-one symptoms of being an internet addict: 145. You e-mail your boss, informing him you'll be late. /// Bram Moolenaar -- Bram@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 ///