lib: Removed global variables from module math for thread safety

[sebastianertz]

The srand_called is stored in a registry entry.

[IdWV]

Isn't that overengineered? The 'srand_called' boolean is a cheap hack to prevent that srand() (and mainly gettimeofday(), I suppose) will be called when rand() is never called. Just to save some computer cycles.

I wouldn't be surprised if calling 'uc_vm_registry_set(vm, "math.srand_called", ucv_boolean_new(false));' in uc_module_init() is more expensive than calling gettimeofday()+srand(). So why not call srand() in uc_module_init(), and just skip the whole 'srand_called', removing the necessity of doing the expensive check of the registry in uc_rand()?

Apart from that, I always thought you have to call srand() once in each thread. But now I checked, and that isn't true, at least not for musl and glibc. In musl the implementation of srand()/rand() is:

static uint64_t seed;

void srand(unsigned s)
{
	seed = s-1;
}

int rand(void)
{
	seed = 6364136223846793005ULL*seed + 1;
	return seed>>33;
}

So there is only one global variable for all threads. When you call rand() from several threads simultaneously that will add some extra randomness, I suppose? ;)

Glibc is more sophisticated:

void
__srandom (unsigned int x)
{
  __libc_lock_lock (lock);
  (void) __srandom_r (x, &unsafe_state);
  __libc_lock_unlock (lock);
}

long int
__random (void)
{
  int32_t retval;
  __libc_lock_lock (lock);
  (void) __random_r (&unsafe_state, &retval);
  __libc_lock_unlock (lock);
  return retval;
}

where 'unsafe_state' is a static struct. So in both cases srand() has only to be called once in the whole process. Which the original implementation with a single global 'srand_called' boolean was perfectly able to accomplish. (Well, not completely. When you would run 2 scripts in 2 threads which would call math.rand() for the first time on the same moment, it is possible that one of the two would return with an unseeded 'random' value.)

[jow-]

The intended logic here is modeled after Perl's rand() for which perldoc states:

Automatically calls "srand" unless "srand" has already been called. See also "srand".

So the idea is that naively calling rand() shall provide some randomness ootb, otherwise without implicitly seeding the prng we'd always yield the same sequence of random numbers, which likely would be counter-intuitive.

Always calling srand() in rand() on the other hand would have undesired side effects;

• Calling rand() twice within the same millisecond would yield the same value
• Explicitly seeding the prng in order to reproduce a specific sequence would not be possible anymore

I guess the expectation is that each thread in an application gets a different sequence of random numbers when calling rand() without any explicit seeding, so we likely should make the srand_called guard either a VM registry property or mark it as thread local, to ensure that threads spawned after a first, initial rand() call get different sequences of numbers.

Am I missing something here, or am I overthinking it? The rand() function does not offer any cryptographic property after all but ucode probably should still do some kind of best effort approach to provide random numbers.

[jow-]

Pushed after adjusting the commit subject. Thanks!