# HG changeset patch # User Bram Moolenaar # Date 1655476206 -7200 # Node ID 0af5fe160e4e0a38e5f1106d88695e14cbde2e95 # Parent a4f0e5e61728d4c2ec4e6635fbd1035896dab011 patch 8.2.5115: search timeout is overrun with some patterns Commit: https://github.com/vim/vim/commit/616592e0816d2d9f893fcd95e3e1e0fbc5893168 Author: Bram Moolenaar Date: Fri Jun 17 15:17:10 2022 +0100 patch 8.2.5115: search timeout is overrun with some patterns Problem: Search timeout is overrun with some patterns. Solution: Check for timeout in more places. Make the flag volatile and atomic. Use assert_inrange() to see what happened. diff --git a/src/os_unix.c b/src/os_unix.c --- a/src/os_unix.c +++ b/src/os_unix.c @@ -8251,7 +8251,7 @@ xsmp_close(void) /* * Implement timeout with timer_create() and timer_settime(). */ -static int timeout_flag = FALSE; +static volatile int timeout_flag = FALSE; static timer_t timer_id; static int timer_created = FALSE; @@ -8296,7 +8296,7 @@ stop_timeout(void) * This function is not expected to fail, but if it does it will still return a * valid flag pointer; the flag will remain stuck as FALSE . */ - const int * + volatile int * start_timeout(long msec) { struct itimerspec interval = { @@ -8324,6 +8324,8 @@ start_timeout(long msec) timer_created = TRUE; } + ch_log(NULL, "setting timeout timer to %d sec %ld nsec", + (int)interval.it_value.tv_sec, (long)interval.it_value.tv_nsec); ret = timer_settime(timer_id, 0, &interval, NULL); if (ret < 0) semsg(_(e_could_not_set_timeout_str), strerror(errno)); @@ -8351,7 +8353,7 @@ delete_timer(void) */ static struct itimerval prev_interval; static struct sigaction prev_sigaction; -static int timeout_flag = FALSE; +static volatile int timeout_flag = FALSE; static int timer_active = FALSE; static int timer_handler_active = FALSE; static int alarm_pending = FALSE; @@ -8409,7 +8411,7 @@ stop_timeout(void) * This function is not expected to fail, but if it does it will still return a * valid flag pointer; the flag will remain stuck as FALSE . */ - const int * + volatile int * start_timeout(long msec) { struct itimerval interval = { diff --git a/src/proto/os_unix.pro b/src/proto/os_unix.pro --- a/src/proto/os_unix.pro +++ b/src/proto/os_unix.pro @@ -87,6 +87,6 @@ int xsmp_handle_requests(void); void xsmp_init(void); void xsmp_close(void); void stop_timeout(void); -const int *start_timeout(long msec); +volatile int *start_timeout(long msec); void delete_timer(void); /* vim: set ft=c : */ diff --git a/src/regexp.c b/src/regexp.c --- a/src/regexp.c +++ b/src/regexp.c @@ -22,7 +22,7 @@ #ifdef FEAT_RELTIME static int dummy_timeout_flag = 0; -static const int *timeout_flag = &dummy_timeout_flag; +static volatile int *timeout_flag = &dummy_timeout_flag; #endif /* diff --git a/src/regexp_bt.c b/src/regexp_bt.c --- a/src/regexp_bt.c +++ b/src/regexp_bt.c @@ -3152,6 +3152,27 @@ regstack_pop(char_u **scan) regstack.ga_len -= sizeof(regitem_T); } +#ifdef FEAT_RELTIME +/* + * Check if the timer expired, return TRUE if so. + */ + static int +bt_did_time_out(int *timed_out) +{ + if (*timeout_flag) + { + if (timed_out != NULL) + { + if (!*timed_out) + ch_log(NULL, "BT regexp timed out"); + *timed_out = TRUE; + } + return TRUE; + } + return FALSE; +} +#endif + /* * Save the current subexpr to "bp", so that they can be restored * later by restore_subexpr(). @@ -3267,10 +3288,8 @@ regmatch( break; } #ifdef FEAT_RELTIME - if (*timeout_flag) + if (bt_did_time_out(timed_out)) { - if (timed_out != NULL) - *timed_out = TRUE; status = RA_FAIL; break; } @@ -4687,6 +4706,14 @@ regmatch( if (status == RA_CONT || rp == (regitem_T *) ((char *)regstack.ga_data + regstack.ga_len) - 1) break; + +#ifdef FEAT_RELTIME + if (bt_did_time_out(timed_out)) + { + status = RA_FAIL; + break; + } +#endif } // May need to continue with the inner loop, starting at "scan". @@ -4976,12 +5003,8 @@ bt_regexec_both( else ++col; #ifdef FEAT_RELTIME - if (*timeout_flag) - { - if (timed_out != NULL) - *timed_out = TRUE; + if (bt_did_time_out(timed_out)) break; - } #endif } } diff --git a/src/regexp_nfa.c b/src/regexp_nfa.c --- a/src/regexp_nfa.c +++ b/src/regexp_nfa.c @@ -4237,6 +4237,27 @@ sub_equal(regsub_T *sub1, regsub_T *sub2 return TRUE; } +#ifdef FEAT_RELTIME +/* + * Check if we are past the time limit, if there is one. + */ + static int +nfa_did_time_out(void) +{ + if (*timeout_flag) + { + if (nfa_timed_out != NULL) + { + if (!*nfa_timed_out) + ch_log(NULL, "NFA regexp timed out"); + *nfa_timed_out = TRUE; + } + return TRUE; + } + return FALSE; +} +#endif + #ifdef ENABLE_LOG static void open_debug_log(int result) @@ -4451,7 +4472,7 @@ state_in_list( /* * Add "state" and possibly what follows to state list ".". * Returns "subs_arg", possibly copied into temp_subs. - * Returns NULL when recursiveness is too deep. + * Returns NULL when recursiveness is too deep or timed out. */ static regsubs_T * addstate( @@ -4480,6 +4501,11 @@ addstate( #endif static int depth = 0; +#ifdef FEAT_RELTIME + if (nfa_did_time_out()) + return NULL; +#endif + // This function is called recursively. When the depth is too much we run // out of stack and crash, limit recursiveness here. if (++depth >= 5000 || subs == NULL) @@ -5643,24 +5669,6 @@ find_match_text(colnr_T startcol, int re return 0L; } -#ifdef FEAT_RELTIME -/* - * Check if we are past the time limit, if there is one. - * To reduce overhead, only check one in "count" times. - */ - static int -nfa_did_time_out(void) -{ - if (*timeout_flag) - { - if (nfa_timed_out != NULL) - *nfa_timed_out = TRUE; - return TRUE; - } - return FALSE; -} -#endif - /* * Main matching routine. * @@ -5708,7 +5716,6 @@ nfa_regmatch( if (got_int) return FALSE; #ifdef FEAT_RELTIME - // Check relatively often here, since this is the toplevel matching. if (nfa_did_time_out()) return FALSE; #endif @@ -7109,7 +7116,6 @@ nextchar: if (got_int) break; #ifdef FEAT_RELTIME - // Check for timeout once in a twenty times to avoid overhead. if (nfa_did_time_out()) break; #endif diff --git a/src/testdir/test_search.vim b/src/testdir/test_search.vim --- a/src/testdir/test_search.vim +++ b/src/testdir/test_search.vim @@ -1576,10 +1576,17 @@ endfunc func Test_search_timeout() new + " use a complicated pattern that should be slow with the BT engine let pattern = '\%#=1a*.*X\@<=b*' - let search_timeout = 0.02 + + " use a timeout of 50 msec + let search_timeout = 0.05 + + " fill the buffer so that it takes 15 times the timeout to search let slow_target_timeout = search_timeout * 15.0 + " Fill the buffer with more and more text until searching takes more time + " than slow_target_timeout. for n in range(40, 400, 30) call setline(1, ['aaa', repeat('abc ', n), 'ccc']) let start = reltime() @@ -1591,11 +1598,14 @@ func Test_search_timeout() endfor call assert_true(elapsed > slow_target_timeout) + " Check that the timeout kicks in, the time should be less than half of what + " we measured without the timeout. This is permissive, because the timer is + " known to overrun, especially when using valgrind. let max_time = elapsed / 2.0 let start = reltime() call search(pattern, '', 0, float2nr(search_timeout * 1000)) let elapsed = reltimefloat(reltime(start)) - call assert_true(elapsed < max_time) + call assert_inrange(search_timeout * 0.9, max_time, elapsed) bwipe! endfunc diff --git a/src/version.c b/src/version.c --- a/src/version.c +++ b/src/version.c @@ -735,6 +735,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 5115, +/**/ 5114, /**/ 5113,