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
index 6fd24508ddf5a380bac3ad43bc6db7120536a54f..083fd8f9bec3642e5d684d6f8982ea6096bda683 100644 (file)
--- 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
index 268f3bfb659ce46b2de441271844c0780ec98598..a8e961b375c1d0fba3a2eb87a2851c84b6d2c8fc 100644 (file)
--- 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
index 3d08d5a2043d75ace434946190917dec0c3d36d2..0a6a8af3d51ae011cc955b0f2afb24d2a0b8a26e 100644 (file)
--- 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
index e307c419b68b92139f5a07c2f6dfb7d09ef6e5aa..c007b8157fd8e9c7293788668d37f20f40ce577a 100644 (file)
--- 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
index 191cddbf0ec2c777b115d2cbd95a46e4d53d2e8c..40e74484ea21fbc0daf5d10aa97520441a97923f 100644 (file)
--- 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 regstart, char_u *match_text)
     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
index caeddc567a9e380ec08e8e5c9a83aeb50a3c583a..7b4b9fd32fba671566caf8758af4d0f0c57e700e 100644 (file)
--- 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
index 3edd3188328824fc86c3e9d4936b009bcc1645e8..a7e622eab3d5868c46c9180f27fb2f2ae796005a 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    5115,
 /**/
     5114,
 /**/