patch 8.2.0233: crash when using garbagecollect() in between rand()

Problem:    Crash when using garbagecollect() in between rand().
Solution:   Redesign the rand() and srand() implementation. (Yasuhiro
            Matsumoto, closes #5587, closes #5588)

diff --git a/runtime/doc/eval.txt b/runtime/doc/eval.txt
index 26e7d12de4a64f91eb85b136cffb501233e9e458..afbfd9f7e5b443f6816a8128396b6bbb88197c67 100644 (file)
--- a/runtime/doc/eval.txt
+++ b/runtime/doc/eval.txt
@@ -2867,6 +2867,7 @@ test_refcount({expr})             Number  get the reference count of {expr}
 test_scrollbar({which}, {value}, {dragging})
                                none    scroll in the GUI for testing
 test_setmouse({row}, {col})    none    set the mouse position for testing
+test_srand_seed([seed])                none    set seed for testing srand()
 test_settime({expr})           none    set current time for testing
 timer_info([{id}])             List    information about timers
 timer_pause({id}, {pause})     none    pause or unpause a timer

diff --git a/runtime/doc/testing.txt b/runtime/doc/testing.txt
index 7fb24e63079f034f6599ef0b581869661c358c6a..f2a7cf16c55cbddb83b4198c3c7a46f16a7e28c4 100644 (file)
--- a/runtime/doc/testing.txt
+++ b/runtime/doc/testing.txt
@@ -210,7 +210,6 @@ test_setmouse({row}, {col})                         *test_setmouse()*
                        call test_setmouse(4, 20)
                        call feedkeys("\<LeftMouse>", "xt")
 
-
 test_settime({expr})                                   *test_settime()*
                Set the time Vim uses internally.  Currently only used for
                timestamps in the history, as they are used in viminfo, and
@@ -223,6 +222,10 @@ test_settime({expr})                                       *test_settime()*
                Can also be used as a |method|: >
                        GetTime()->test_settime()
 
+test_srand_seed([seed])                                        *test_srand_seed()*
+               When [seed] is given this sets the seed value used by
+               `srand()`.  When omitted the test seed is removed.
+
 ==============================================================================
 3. Assert functions                            *assert-functions-details*
 

diff --git a/src/evalfunc.c b/src/evalfunc.c
index aac8ee9058e5fb76dbafd72ad0a335e3c183605a..66be42c241a43fa463c975f885caca2439ccf639 100644 (file)
--- a/src/evalfunc.c
+++ b/src/evalfunc.c
@@ -168,6 +168,7 @@ static void f_pyeval(typval_T *argvars, typval_T *rettv);
 #if defined(FEAT_PYTHON) || defined(FEAT_PYTHON3)
 static void f_pyxeval(typval_T *argvars, typval_T *rettv);
 #endif
+static void f_test_srand_seed(typval_T *argvars, typval_T *rettv);
 static void f_rand(typval_T *argvars, typval_T *rettv);
 static void f_range(typval_T *argvars, typval_T *rettv);
 static void f_reg_executing(typval_T *argvars, typval_T *rettv);
@@ -835,6 +836,7 @@ static funcentry_T global_functions[] =
 #endif
     {"test_setmouse",  2, 2, 0,          &t_void,      f_test_setmouse},
     {"test_settime",   1, 1, FEARG_1,    &t_void,      f_test_settime},
+    {"test_srand_seed",        0, 1, FEARG_1,    &t_void,      f_test_srand_seed},
 #ifdef FEAT_TIMERS
     {"timer_info",     0, 1, FEARG_1,    &t_list_dict_any, f_timer_info},
     {"timer_pause",    2, 2, FEARG_1,    &t_void,      f_timer_pause},
@@ -5225,6 +5227,83 @@ f_pyxeval(typval_T *argvars, typval_T *rettv)
 }
 #endif
 
+static UINT32_T srand_seed_for_testing = 0;
+static int     srand_seed_for_testing_is_used = FALSE;
+
+    static void
+f_test_srand_seed(typval_T *argvars, typval_T *rettv UNUSED)
+{
+    if (argvars[0].v_type == VAR_UNKNOWN)
+        srand_seed_for_testing_is_used = FALSE;
+    else
+    {
+        srand_seed_for_testing = (UINT32_T)tv_get_number(&argvars[0]);
+        srand_seed_for_testing_is_used = TRUE;
+    }
+}
+
+    static void
+init_srand(UINT32_T *x)
+{
+#ifndef MSWIN
+    static int dev_urandom_state = NOTDONE;  // FAIL or OK once tried
+#endif
+
+    if (srand_seed_for_testing_is_used)
+    {
+        *x = srand_seed_for_testing;
+       return;
+    }
+#ifndef MSWIN
+    if (dev_urandom_state != FAIL)
+    {
+       int  fd = open("/dev/urandom", O_RDONLY);
+       struct {
+           union {
+               UINT32_T number;
+               char     bytes[sizeof(UINT32_T)];
+           } contents;
+       } buf;
+
+       // Attempt reading /dev/urandom.
+       if (fd == -1)
+           dev_urandom_state = FAIL;
+       else
+       {
+           buf.contents.number = 0;
+           if (read(fd, buf.contents.bytes, sizeof(UINT32_T))
+                                                          != sizeof(UINT32_T))
+               dev_urandom_state = FAIL;
+           else
+           {
+               dev_urandom_state = OK;
+               *x = buf.contents.number;
+           }
+           close(fd);
+       }
+    }
+    if (dev_urandom_state != OK)
+       // Reading /dev/urandom doesn't work, fall back to time().
+#endif
+       *x = vim_time();
+}
+
+#define ROTL(x, k) ((x << k) | (x >> (32 - k)))
+#define SPLITMIX32(x, z) ( \
+    z = (x += 0x9e3779b9), \
+    z = (z ^ (z >> 16)) * 0x85ebca6b, \
+    z = (z ^ (z >> 13)) * 0xc2b2ae35, \
+    z ^ (z >> 16) \
+    )
+#define SHUFFLE_XOSHIRO128STARSTAR(x, y, z, w) \
+    result = ROTL(y * 5, 7) * 9; \
+    t = y << 9; \
+    z ^= x; \
+    w ^= y; \
+    y ^= z, x ^= w; \
+    z ^= t; \
+    w = ROTL(w, 11);
+
 /*
  * "rand()" function
  */
@@ -5232,66 +5311,57 @@ f_pyxeval(typval_T *argvars, typval_T *rettv)
 f_rand(typval_T *argvars, typval_T *rettv)
 {
     list_T     *l = NULL;
-    static list_T *globl = NULL;
-    UINT32_T   x, y, z, w, t, result;
+    static UINT32_T    gx, gy, gz, gw;
+    static int initialized = FALSE;
     listitem_T *lx, *ly, *lz, *lw;
+    UINT32_T   x, y, z, w, t, result;
 
     if (argvars[0].v_type == VAR_UNKNOWN)
     {
        // When no argument is given use the global seed list.
-       if (globl == NULL)
+       if (initialized == FALSE)
        {
            // Initialize the global seed list.
-           f_srand(argvars, rettv);
-           l = rettv->vval.v_list;
-           if (l == NULL || list_len(l) != 4)
-           {
-               clear_tv(rettv);
-               goto theend;
-           }
-           globl = l;
+           init_srand(&x);
+
+           gx = SPLITMIX32(x, z);
+           gy = SPLITMIX32(x, z);
+           gz = SPLITMIX32(x, z);
+           gw = SPLITMIX32(x, z);
+           initialized = TRUE;
        }
-       else
-           l = globl;
+
+       SHUFFLE_XOSHIRO128STARSTAR(gx, gy, gz, gw);
     }
     else if (argvars[0].v_type == VAR_LIST)
     {
        l = argvars[0].vval.v_list;
        if (l == NULL || list_len(l) != 4)
            goto theend;
+
+       lx = list_find(l, 0L);
+       ly = list_find(l, 1L);
+       lz = list_find(l, 2L);
+       lw = list_find(l, 3L);
+       if (lx->li_tv.v_type != VAR_NUMBER) goto theend;
+       if (ly->li_tv.v_type != VAR_NUMBER) goto theend;
+       if (lz->li_tv.v_type != VAR_NUMBER) goto theend;
+       if (lw->li_tv.v_type != VAR_NUMBER) goto theend;
+       x = (UINT32_T)lx->li_tv.vval.v_number;
+       y = (UINT32_T)ly->li_tv.vval.v_number;
+       z = (UINT32_T)lz->li_tv.vval.v_number;
+       w = (UINT32_T)lw->li_tv.vval.v_number;
+
+       SHUFFLE_XOSHIRO128STARSTAR(x, y, z, w);
+
+       lx->li_tv.vval.v_number = (varnumber_T)x;
+       ly->li_tv.vval.v_number = (varnumber_T)y;
+       lz->li_tv.vval.v_number = (varnumber_T)z;
+       lw->li_tv.vval.v_number = (varnumber_T)w;
     }
     else
        goto theend;
 
-    lx = list_find(l, 0L);
-    ly = list_find(l, 1L);
-    lz = list_find(l, 2L);
-    lw = list_find(l, 3L);
-    if (lx->li_tv.v_type != VAR_NUMBER) goto theend;
-    if (ly->li_tv.v_type != VAR_NUMBER) goto theend;
-    if (lz->li_tv.v_type != VAR_NUMBER) goto theend;
-    if (lw->li_tv.v_type != VAR_NUMBER) goto theend;
-    x = (UINT32_T)lx->li_tv.vval.v_number;
-    y = (UINT32_T)ly->li_tv.vval.v_number;
-    z = (UINT32_T)lz->li_tv.vval.v_number;
-    w = (UINT32_T)lw->li_tv.vval.v_number;
-
-    // SHUFFLE_XOSHIRO128STARSTAR
-#define ROTL(x, k) ((x << k) | (x >> (32 - k)))
-    result = ROTL(y * 5, 7) * 9;
-    t = y << 9;
-    z ^= x;
-    w ^= y;
-    y ^= z, x ^= w;
-    z ^= t;
-    w = ROTL(w, 11);
-#undef ROTL
-
-    lx->li_tv.vval.v_number = (varnumber_T)x;
-    ly->li_tv.vval.v_number = (varnumber_T)y;
-    lz->li_tv.vval.v_number = (varnumber_T)z;
-    lw->li_tv.vval.v_number = (varnumber_T)w;
-
     rettv->v_type = VAR_NUMBER;
     rettv->vval.v_number = (varnumber_T)result;
     return;
@@ -5302,6 +5372,39 @@ theend:
     rettv->vval.v_number = -1;
 }
 
+/*
+ * "srand()" function
+ */
+    static void
+f_srand(typval_T *argvars, typval_T *rettv)
+{
+    UINT32_T x = 0, z;
+
+    if (rettv_list_alloc(rettv) == FAIL)
+       return;
+    if (argvars[0].v_type == VAR_UNKNOWN)
+    {
+       init_srand(&x);
+    }
+    else
+    {
+       int         error = FALSE;
+
+       x = (UINT32_T)tv_get_number_chk(&argvars[0], &error);
+       if (error)
+           return;
+    }
+
+    list_append_number(rettv->vval.v_list, (varnumber_T)SPLITMIX32(x, z));
+    list_append_number(rettv->vval.v_list, (varnumber_T)SPLITMIX32(x, z));
+    list_append_number(rettv->vval.v_list, (varnumber_T)SPLITMIX32(x, z));
+    list_append_number(rettv->vval.v_list, (varnumber_T)SPLITMIX32(x, z));
+}
+
+#undef ROTL
+#undef SPLITMIX32
+#undef SHUFFLE_XOSHIRO128STARSTAR
+
 /*
  * "range()" function
  */
@@ -7217,73 +7320,6 @@ f_sqrt(typval_T *argvars, typval_T *rettv)
 }
 #endif
 
-/*
- * "srand()" function
- */
-    static void
-f_srand(typval_T *argvars, typval_T *rettv)
-{
-    static int dev_urandom_state = -1;  // FAIL or OK once tried
-    UINT32_T x = 0, z;
-
-    if (rettv_list_alloc(rettv) == FAIL)
-       return;
-    if (argvars[0].v_type == VAR_UNKNOWN)
-    {
-       if (dev_urandom_state != FAIL)
-       {
-           int  fd = open("/dev/urandom", O_RDONLY);
-           struct {
-               union {
-                   UINT32_T number;
-                   char     bytes[sizeof(UINT32_T)];
-               } cont;
-           } buf;
-
-           // Attempt reading /dev/urandom.
-           if (fd == -1)
-               dev_urandom_state = FAIL;
-           else
-           {
-               buf.cont.number = 0;
-               if (read(fd, buf.cont.bytes, sizeof(UINT32_T))
-                                                          != sizeof(UINT32_T))
-                   dev_urandom_state = FAIL;
-               else
-               {
-                   dev_urandom_state = OK;
-                   x = buf.cont.number;
-               }
-               close(fd);
-           }
-
-       }
-       if (dev_urandom_state != OK)
-           // Reading /dev/urandom doesn't work, fall back to time().
-           x = vim_time();
-    }
-    else
-    {
-       int         error = FALSE;
-
-       x = (UINT32_T)tv_get_number_chk(&argvars[0], &error);
-       if (error)
-           return;
-    }
-
-#define SPLITMIX32 ( \
-    z = (x += 0x9e3779b9), \
-    z = (z ^ (z >> 16)) * 0x85ebca6b, \
-    z = (z ^ (z >> 13)) * 0xc2b2ae35, \
-    z ^ (z >> 16) \
-    )
-
-    list_append_number(rettv->vval.v_list, (varnumber_T)SPLITMIX32);
-    list_append_number(rettv->vval.v_list, (varnumber_T)SPLITMIX32);
-    list_append_number(rettv->vval.v_list, (varnumber_T)SPLITMIX32);
-    list_append_number(rettv->vval.v_list, (varnumber_T)SPLITMIX32);
-}
-
 #ifdef FEAT_FLOAT
 /*
  * "str2float()" function

diff --git a/src/testdir/test_random.vim b/src/testdir/test_random.vim
index 747e438486b6ed0ab17aa2b91ab48845c562f2c3..bdb7e1db0e9d07fce70df502e440a2f2b1b3d9cf 100644 (file)
--- a/src/testdir/test_random.vim
+++ b/src/testdir/test_random.vim
@@ -11,7 +11,7 @@ func Test_Rand()
 
   call test_settime(12341234)
   let s = srand()
-  if filereadable('/dev/urandom')
+  if !has('win32') && filereadable('/dev/urandom')
     " using /dev/urandom
     call assert_notequal(s, srand())
   else
@@ -21,9 +21,10 @@ func Test_Rand()
     call assert_notequal(s, srand())
   endif
 
-  call srand()
-  let v = rand()
-  call assert_notequal(v, rand())
+  call test_srand_seed(123456789)
+  call assert_equal(4284103975, rand())
+  call assert_equal(1001954530, rand())
+  call test_srand_seed()
 
   if has('float')
     call assert_fails('echo srand(1.2)', 'E805:')
@@ -38,3 +39,9 @@ func Test_Rand()
 
   call test_settime(0)
 endfunc
+
+func Test_issue_5587()
+  call rand()
+  call garbagecollect()
+  call rand()
+endfunc

diff --git a/src/version.c b/src/version.c
index e1fde105f99ba3c86cf0f7d81250c7a88877f20a..99c63569d14871fabf0064023c7b7a22dfed1ebf 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -742,6 +742,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    233,
 /**/
     232,
 /**/