Patch 8.1.0488
Problem: Using freed memory in quickfix code. (Dominique Pelle)
Solution: Add the quickfix_busy() flag to postpone deleting quickfix lists
until it is safe. (Yegappan Lakshmanan, closes #3538)
Files: src/quickfix.c, src/proto/quickfix.pro, src/misc2.c,
src/testdir/test_quickfix.vim
*** ../vim-8.1.0487/src/quickfix.c 2018-10-11 17:39:09.169107531 +0200
--- src/quickfix.c 2018-10-20 20:53:18.999667086 +0200
***************
*** 130,135 ****
--- 130,148 ----
int conthere; // %> used
};
+ // List of location lists to be deleted.
+ // Used to delay the deletion of locations lists by autocmds.
+ typedef struct qf_delq_S
+ {
+ struct qf_delq_S *next;
+ qf_info_T *qi;
+ } qf_delq_T;
+ static qf_delq_T *qf_delq_head = NULL;
+
+ // Counter to prevent autocmds from freeing up location lists when they are
+ // still being used.
+ static int quickfix_busy = 0;
+
static efm_T *fmt_start = NULL; // cached across qf_parse_line() calls
static void qf_new_list(qf_info_T *qi, char_u *qf_title);
***************
*** 1838,1843 ****
--- 1851,1873 ----
}
/*
+ * Queue location list stack delete request.
+ */
+ static void
+ locstack_queue_delreq(qf_info_T *qi)
+ {
+ qf_delq_T *q;
+
+ q = (qf_delq_T *)alloc((unsigned)sizeof(qf_delq_T));
+ if (q != NULL)
+ {
+ q->qi = qi;
+ q->next = qf_delq_head;
+ qf_delq_head = q;
+ }
+ }
+
+ /*
* Free a location list stack
*/
static void
***************
*** 1854,1863 ****
qi->qf_refcount--;
if (qi->qf_refcount < 1)
{
! // No references to this location list
! for (i = 0; i < qi->qf_listcount; ++i)
! qf_free(&qi->qf_lists[i]);
! vim_free(qi);
}
}
--- 1884,1900 ----
qi->qf_refcount--;
if (qi->qf_refcount < 1)
{
! // No references to this location list.
! // If the location list is still in use, then queue the delete request
! // to be processed later.
! if (quickfix_busy > 0)
! locstack_queue_delreq(qi);
! else
! {
! for (i = 0; i < qi->qf_listcount; ++i)
! qf_free(&qi->qf_lists[i]);
! vim_free(qi);
! }
}
}
***************
*** 1883,1888 ****
--- 1920,1979 ----
}
/*
+ * Delay freeing of location list stacks when the quickfix code is running.
+ * Used to avoid problems with autocmds freeing location list stacks when the
+ * quickfix code is still referencing the stack.
+ * Must always call decr_quickfix_busy() exactly once after this.
+ */
+ static void
+ incr_quickfix_busy(void)
+ {
+ quickfix_busy++;
+ }
+
+ /*
+ * Safe to free location list stacks. Process any delayed delete requests.
+ */
+ static void
+ decr_quickfix_busy(void)
+ {
+ if (--quickfix_busy == 0)
+ {
+ // No longer referencing the location lists. Process all the pending
+ // delete requests.
+ while (qf_delq_head != NULL)
+ {
+ qf_delq_T *q = qf_delq_head;
+
+ qf_delq_head = q->next;
+ ll_free_all(&q->qi);
+ vim_free(q);
+ }
+ }
+ #ifdef ABORT_ON_INTERNAL_ERROR
+ if (quickfix_busy < 0)
+ {
+ EMSG("quickfix_busy has become negative");
+ abort();
+ }
+ #endif
+ }
+
+ #if defined(EXITFREE) || defined(PROTO)
+ void
+ check_quickfix_busy(void)
+ {
+ if (quickfix_busy != 0)
+ {
+ EMSGN("quickfix_busy not zero on exit: %ld", (long)quickfix_busy);
+ # ifdef ABORT_ON_INTERNAL_ERROR
+ abort();
+ # endif
+ }
+ }
+ #endif
+
+ /*
* Add an entry to the end of the list of errors.
* Returns OK or FAIL.
*/
***************
*** 2387,2393 ****
* 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;
--- 2478,2484 ----
* 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;
***************
*** 3939,3944 ****
--- 4030,4036 ----
qf_list_T *qfl;
int height;
int status = FAIL;
+ int lnum;
if (is_loclist_cmd(eap->cmdidx))
{
***************
*** 3950,3955 ****
--- 4042,4049 ----
}
}
+ incr_quickfix_busy();
+
if (eap->addr_count != 0)
height = eap->line2;
else
***************
*** 3966,3980 ****
cmdmod.split & WSP_VERT);
if (status == FAIL)
if (qf_open_new_cwindow(qi, height) == FAIL)
return;
qfl = &qi->qf_lists[qi->qf_curlist];
qf_set_title_var(qfl);
// Fill the buffer with the quickfix list.
qf_fill_buffer(qi, curbuf, NULL);
! curwin->w_cursor.lnum = qfl->qf_index;
curwin->w_cursor.col = 0;
check_cursor();
update_topline(); // scroll to show the line
--- 4060,4082 ----
cmdmod.split & WSP_VERT);
if (status == FAIL)
if (qf_open_new_cwindow(qi, height) == FAIL)
+ {
+ decr_quickfix_busy();
return;
+ }
qfl = &qi->qf_lists[qi->qf_curlist];
qf_set_title_var(qfl);
+ // Save the current index here, as updating the quickfix buffer may free
+ // the quickfix list
+ lnum = qfl->qf_index;
// Fill the buffer with the quickfix list.
qf_fill_buffer(qi, curbuf, NULL);
! decr_quickfix_busy();
!
! curwin->w_cursor.lnum = lnum;
curwin->w_cursor.col = 0;
check_cursor();
update_topline(); // scroll to show the line
***************
*** 4592,4597 ****
--- 4694,4701 ----
(void)char_avail();
#endif
+ incr_quickfix_busy();
+
res = qf_init(wp, fname, (eap->cmdidx != CMD_make
&& eap->cmdidx != CMD_lmake) ? p_gefm : p_efm,
(eap->cmdidx != CMD_grepadd
***************
*** 4617,4622 ****
--- 4721,4727 ----
qf_jump_first(qi, save_qfid, FALSE);
cleanup:
+ decr_quickfix_busy();
mch_remove(fname);
vim_free(fname);
vim_free(cmd);
***************
*** 4927,4932 ****
--- 5032,5039 ----
if (is_loclist_cmd(eap->cmdidx))
wp = curwin;
+ incr_quickfix_busy();
+
// This function is used by the :cfile, :cgetfile and :caddfile
// commands.
// :cfile always creates a new quickfix list and jumps to the
***************
*** 4942,4948 ****
--- 5049,5058 ----
{
qi = GET_LOC_LIST(wp);
if (qi == NULL)
+ {
+ decr_quickfix_busy();
return;
+ }
}
if (res >= 0)
qf_list_changed(&qi->qf_lists[qi->qf_curlist]);
***************
*** 4956,4961 ****
--- 5066,5073 ----
&& qflist_valid(wp, save_qfid))
// display the first error
qf_jump_first(qi, save_qfid, eap->forceit);
+
+ decr_quickfix_busy();
}
/*
***************
*** 5304,5309 ****
--- 5416,5423 ----
// ":lcd %:p:h" changes the meaning of short path names.
mch_dirname(dirname_start, MAXPATHL);
+ incr_quickfix_busy();
+
// Remember the current quickfix list identifier, so that we can check for
// autocommands changing the current quickfix list.
save_qfid = qi->qf_lists[qi->qf_curlist].qf_id;
***************
*** 5339,5344 ****
--- 5453,5459 ----
if (!vgr_qflist_valid(wp, qi, save_qfid, qf_cmdtitle(*eap->cmdlinep)))
{
FreeWild(fcount, fnames);
+ decr_quickfix_busy();
goto theend;
}
save_qfid = qi->qf_lists[qi->qf_curlist].qf_id;
***************
*** 5434,5444 ****
curbuf->b_fname, TRUE, curbuf);
// The QuickFixCmdPost autocmd may free the quickfix list. Check the list
// is still valid.
! if (!qflist_valid(wp, save_qfid))
! goto theend;
!
! if (qf_restore_list(qi, save_qfid) == FAIL)
goto theend;
// Jump to first match.
if (!qf_list_empty(qi, qi->qf_curlist))
--- 5549,5560 ----
curbuf->b_fname, TRUE, curbuf);
// The QuickFixCmdPost autocmd may free the quickfix list. Check the list
// is still valid.
! if (!qflist_valid(wp, save_qfid)
! || qf_restore_list(qi, save_qfid) == FAIL)
! {
! decr_quickfix_busy();
goto theend;
+ }
// Jump to first match.
if (!qf_list_empty(qi, qi->qf_curlist))
***************
*** 5450,5455 ****
--- 5566,5573 ----
else
EMSG2(_(e_nomatch2), s);
+ decr_quickfix_busy();
+
// If we loaded a dummy buffer into the current window, the autocommands
// may have messed up things, need to redraw and recompute folds.
if (redraw_for_dummy)
***************
*** 6516,6523 ****
{
// Free the entire quickfix or location list stack
qf_free_stack(wp, qi);
}
! else if (what != NULL)
retval = qf_set_properties(qi, what, action, title);
else
{
--- 6634,6645 ----
{
// Free the entire quickfix or location list stack
qf_free_stack(wp, qi);
+ return OK;
}
!
! incr_quickfix_busy();
!
! if (what != NULL)
retval = qf_set_properties(qi, what, action, title);
else
{
***************
*** 6526,6531 ****
--- 6648,6655 ----
qf_list_changed(&qi->qf_lists[qi->qf_curlist]);
}
+ decr_quickfix_busy();
+
return retval;
}
***************
*** 6663,6673 ****
--- 6787,6804 ----
qf_title = IObuff;
}
+ incr_quickfix_busy();
+
res = qf_init_ext(qi, qi->qf_curlist, NULL, buf, NULL, p_efm,
(eap->cmdidx != CMD_caddbuffer
&& eap->cmdidx != CMD_laddbuffer),
eap->line1, eap->line2,
qf_title, NULL);
+ if (qf_stack_empty(qi))
+ {
+ decr_quickfix_busy();
+ return;
+ }
if (res >= 0)
qf_list_changed(&qi->qf_lists[qi->qf_curlist]);
***************
*** 6692,6697 ****
--- 6823,6830 ----
&& qflist_valid(wp, save_qfid))
// display the first error
qf_jump_first(qi, save_qfid, eap->forceit);
+
+ decr_quickfix_busy();
}
}
}
***************
*** 6746,6756 ****
--- 6879,6895 ----
if ((tv->v_type == VAR_STRING && tv->vval.v_string != NULL)
|| (tv->v_type == VAR_LIST && tv->vval.v_list != NULL))
{
+ incr_quickfix_busy();
res = qf_init_ext(qi, qi->qf_curlist, NULL, NULL, tv, p_efm,
(eap->cmdidx != CMD_caddexpr
&& eap->cmdidx != CMD_laddexpr),
(linenr_T)0, (linenr_T)0,
qf_cmdtitle(*eap->cmdlinep), NULL);
+ if (qf_stack_empty(qi))
+ {
+ decr_quickfix_busy();
+ goto cleanup;
+ }
if (res >= 0)
qf_list_changed(&qi->qf_lists[qi->qf_curlist]);
***************
*** 6768,6776 ****
--- 6907,6917 ----
&& qflist_valid(wp, save_qfid))
// display the first error
qf_jump_first(qi, save_qfid, eap->forceit);
+ decr_quickfix_busy();
}
else
EMSG(_("E777: String or List expected"));
+ cleanup:
free_tv(tv);
}
}
***************
*** 6958,6964 ****
convert_setup(&vc, (char_u *)"utf-8", p_enc);
#endif
-
// Go through all the directories in 'runtimepath'
p = p_rtp;
while (*p != NUL && !got_int)
--- 7099,7104 ----
***************
*** 7020,7025 ****
--- 7160,7167 ----
return;
}
+ incr_quickfix_busy();
+
#ifdef FEAT_MULTI_LANG
// Check for a specified language
lang = check_help_lang(eap->arg);
***************
*** 7056,7063 ****
--- 7198,7208 ----
apply_autocmds(EVENT_QUICKFIXCMDPOST, au_name,
curbuf->b_fname, TRUE, curbuf);
if (!new_qi && IS_LL_STACK(qi) && qf_find_buf(qi) == NULL)
+ {
// autocommands made "qi" invalid
+ decr_quickfix_busy();
return;
+ }
}
// Jump to first match.
***************
*** 7066,7071 ****
--- 7211,7218 ----
else
EMSG2(_(e_nomatch2), eap->arg);
+ decr_quickfix_busy();
+
if (eap->cmdidx == CMD_lhelpgrep)
{
// If the help window is not opened or if it already points to the
*** ../vim-8.1.0487/src/proto/quickfix.pro 2018-09-25 22:08:10.933806882
+0200
--- src/proto/quickfix.pro 2018-10-20 20:36:34.383299437 +0200
***************
*** 1,6 ****
--- 1,7 ----
/* quickfix.c */
int qf_init(win_T *wp, char_u *efile, char_u *errorformat, int newlist,
char_u *qf_title, char_u *enc);
void qf_free_all(win_T *wp);
+ void check_quickfix_busy(void);
void copy_loclist_stack(win_T *from, win_T *to);
void qf_jump(qf_info_T *qi, int dir, int errornr, int forceit);
void qf_list(exarg_T *eap);
*** ../vim-8.1.0487/src/misc2.c 2018-10-07 21:36:07.389878130 +0200
--- src/misc2.c 2018-10-20 20:41:53.263021777 +0200
***************
*** 1231,1248 ****
buf = firstbuf;
}
! #ifdef FEAT_ARABIC
free_cmdline_buf();
! #endif
/* Clear registers. */
clear_registers();
ResetRedobuff();
ResetRedobuff();
! #if defined(FEAT_CLIENTSERVER) && defined(FEAT_X11)
vim_free(serverDelayedStartName);
! #endif
/* highlight info */
free_highlight();
--- 1231,1248 ----
buf = firstbuf;
}
! # ifdef FEAT_ARABIC
free_cmdline_buf();
! # endif
/* Clear registers. */
clear_registers();
ResetRedobuff();
ResetRedobuff();
! # if defined(FEAT_CLIENTSERVER) && defined(FEAT_X11)
vim_free(serverDelayedStartName);
! # endif
/* highlight info */
free_highlight();
***************
*** 1265,1273 ****
# ifdef FEAT_JOB_CHANNEL
channel_free_all();
# endif
! #ifdef FEAT_TIMERS
timer_free_all();
! #endif
# ifdef FEAT_EVAL
/* must be after channel_free_all() with unrefs partials */
eval_clear();
--- 1265,1273 ----
# ifdef FEAT_JOB_CHANNEL
channel_free_all();
# endif
! # ifdef FEAT_TIMERS
timer_free_all();
! # endif
# ifdef FEAT_EVAL
/* must be after channel_free_all() with unrefs partials */
eval_clear();
***************
*** 1282,1297 ****
/* screenlines (can't display anything now!) */
free_screenlines();
! #if defined(USE_XSMP)
xsmp_close();
! #endif
! #ifdef FEAT_GUI_GTK
gui_mch_free_all();
! #endif
clear_hl_tables();
vim_free(IObuff);
vim_free(NameBuff);
}
#endif
--- 1282,1300 ----
/* screenlines (can't display anything now!) */
free_screenlines();
! # if defined(USE_XSMP)
xsmp_close();
! # endif
! # ifdef FEAT_GUI_GTK
gui_mch_free_all();
! # endif
clear_hl_tables();
vim_free(IObuff);
vim_free(NameBuff);
+ # ifdef FEAT_QUICKFIX
+ check_quickfix_busy();
+ # endif
}
#endif
*** ../vim-8.1.0487/src/testdir/test_quickfix.vim 2018-10-11
17:39:09.173107491 +0200
--- src/testdir/test_quickfix.vim 2018-10-20 20:25:50.775968747 +0200
***************
*** 3220,3226 ****
--- 3220,3247 ----
augroup QF_Test
au!
augroup END
+
enew | only
+ augroup QF_Test
+ au!
+ au BufNew * call setloclist(0, [], 'f')
+ augroup END
+ lexpr 'x:1:x'
+ augroup QF_Test
+ au!
+ augroup END
+
+ enew | only
+ lexpr ''
+ lopen
+ augroup QF_Test
+ au!
+ au FileType * call setloclist(0, [], 'f')
+ augroup END
+ lexpr ''
+ augroup QF_Test
+ au!
+ augroup END
endfunc
" The following test used to crash Vim
*** ../vim-8.1.0487/src/version.c 2018-10-19 22:35:04.889189955 +0200
--- src/version.c 2018-10-20 20:27:51.427127219 +0200
***************
*** 794,795 ****
--- 794,797 ----
{ /* Add new patch number below this line */
+ /**/
+ 488,
/**/
--
hundred-and-one symptoms of being an internet addict:
243. You unsuccessfully try to download a pizza from www.dominos.com.
/// Bram Moolenaar -- [email protected] -- 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 [email protected].
For more options, visit https://groups.google.com/d/optout.
