DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D75DD3856962
Date: Mon, 13 Nov 2023 15:17:02 +0100
To: Bruno Haible <bruno AT clisp DOT org>, newlib AT sourceware DOT org
Subject: Re: rand is not ISO C compliant in Cygwin
Message-ID: <ZVIv3oq8UU6GTa0n@calimero.vinschen.de>
Mail-Followup-To: Bruno Haible <bruno AT clisp DOT org>, newlib AT sourceware DOT org,
 cygwin AT cygwin DOT com
References: <9938355 DOT c9vzh5UkMf AT nimes>
MIME-Version: 1.0
In-Reply-To: <9938355.c9vzh5UkMf@nimes>
Content-Disposition: inline
Precedence: list
From: Corinna Vinschen via Cygwin <cygwin AT cygwin DOT com>
Reply-To: newlib AT sourceware DOT org
Cc: Corinna Vinschen <vinschen AT redhat DOT com>, cygwin AT cygwin DOT com
Content-Type: text/plain; charset="utf-8"
Errors-To: cygwin-bounces+archive-cygwin=delorie DOT com AT cygwin DOT com
Sender: "Cygwin" <cygwin-bounces+archive-cygwin=delorie DOT com AT cygwin DOT com>
Content-Transfer-Encoding: 8bit


[redirecting to the newlib mailing list]

This is long-standing code in newlib, not actually inside Cygwin.

On Nov 10 21:19, Bruno Haible via Cygwin wrote:
> ISO C 23 § 7.24.2.1 and 7.24.2.2 document how rand() and srand() are
> expected to behave. In particular:
>   "The srand function uses the argument as a seed for a new sequence
>    of pseudo-random numbers to be returned by subsequent calls to rand.
>    If srand is then called with the same seed value, the sequence of
>    pseudo-random numbers shall be repeated. ...
>    The srand function is not required to avoid data races with other
>    calls to pseudo-random sequence generation functions. ..."
> 
> The two attached programs call srand() in one thread and then rand()
> in another thread. There is no data race, since the second thread
> is only created after the call to srand() has returned. The behaviour
> in Cygwin is that the values in the second thread ignore the srand()
> call done in the first thread.

Struct _reent is a kind of per-execution unit datastructure.  This
could be independent code blocks in bare-metal code, or threads in
Cygwin et al.  So the _reent struct also holds the seed state of the
rand and rand48 generators for a good reason.

But that's only half the picture, because newlib actually has two ways
of storing _reent data:  Either in a pre-thread struct _reent, or (if
_REENT_THREAD_LOCAL is defined) as per-member thread_local storage.

Theoretically, this could be easily fixed:

- Either we maintain a global struct _reent which could be used
  from rand().

- Or, in case of _REENT_THREAD_LOCAL, by making the rand48 data globally
  available as static data, rather than as thread_local data.

The rand() function would still not use locking but AFAICS that's
not actually required by POSIX or ISO C.

However, this is something I don't want to decide single-handedly,
so I'm asking how to go forward on the newlib ML.

As far as Cygwin alone is concerned, a patch like this one would suffice:

diff --git a/newlib/libc/stdlib/rand.c b/newlib/libc/stdlib/rand.c
index ba9cc80f2b21..2b48e7a725b1 100644
--- a/newlib/libc/stdlib/rand.c
+++ b/newlib/libc/stdlib/rand.c
@@ -58,6 +58,12 @@ on two different systems.
 #include <stdlib.h>
 #include <reent.h>
 
+#ifdef __CYGWIN__
+#define _RAND_REENT	_GLOBAL_REENT
+#else
+#define _RAND_REENT	_REENT
+#endif
+
 #ifdef _REENT_THREAD_LOCAL
 _Thread_local unsigned long long _tls_rand_next = 1;
 #endif
@@ -65,7 +71,7 @@ _Thread_local unsigned long long _tls_rand_next = 1;
 void
 srand (unsigned int seed)
 {
-  struct _reent *reent = _REENT;
+  struct _reent *reent = _RAND_REENT;
 
   _REENT_CHECK_RAND48(reent);
   _REENT_RAND_NEXT(reent) = seed;
@@ -74,7 +80,7 @@ srand (unsigned int seed)
 int
 rand (void)
 {
-  struct _reent *reent = _REENT;
+  struct _reent *reent = _RAND_REENT;
 
   /* This multiplier was obtained from Knuth, D.E., "The Art of
      Computer Programming," Vol 2, Seminumerical Algorithms, Third


> 
> How to reproduce the bug:
> 
> $ x86_64-pc-cygwin-gcc -Wall rand-in-posix-thread.c
> $ ./a
> 
> or
> 
> $ x86_64-pc-cygwin-gcc -Wall rand-in-isoc-thread.c
> $ ./a
> 
> Expected output:
> 
> Value from main thread:     1583559764
> Value from separate thread: 1583559764
> 
> Actual output:
> 
> Value from main thread:     1583559764
> Value from separate thread: 1481765933
> 

> #include <assert.h>
> #include <pthread.h>
> #include <stdio.h>
> #include <stdlib.h>
> 
> static void *
> rand_invocator_thread (void *arg)
> {
>   printf ("Value from separate thread: %d\n", rand ());
>   return NULL;
> }
> 
> int
> main ()
> {
>   unsigned int seed = 19891109;
> 
>   srand (seed);
>   printf ("Value from main thread:     %d\n", rand ());
>   srand (seed);
>   pthread_t t;
>   assert (pthread_create (&t, NULL, rand_invocator_thread, NULL) == 0);
>   assert (pthread_join (t, NULL) == 0);
> 
>   return 0;
> }

> #include <assert.h>
> #include <threads.h>
> #include <stdio.h>
> #include <stdlib.h>
> 
> static int
> rand_invocator_thread (void *arg)
> {
>   printf ("Value from separate thread: %d\n", rand ());
>   return 0;
> }
> 
> int
> main ()
> {
>   unsigned int seed = 19891109;
> 
>   srand (seed);
>   printf ("Value from main thread:     %d\n", rand ());
>   srand (seed);
>   thrd_t t;
>   assert (thrd_create (&t, rand_invocator_thread, NULL) == thrd_success);
>   assert (thrd_join (t, NULL) == thrd_success);
> 
>   return 0;
> }

> 
> -- 
> Problem reports:      https://cygwin.com/problems.html
> FAQ:                  https://cygwin.com/faq/
> Documentation:        https://cygwin.com/docs.html
> Unsubscribe info:     https://cygwin.com/ml/#unsubscribe-simple


-- 
Problem reports:      https://cygwin.com/problems.html
FAQ:                  https://cygwin.com/faq/
Documentation:        https://cygwin.com/docs.html
Unsubscribe info:     https://cygwin.com/ml/#unsubscribe-simple