mini-gmp mpz_{get,set}_d not fully compatible with GMP

[Vincent Lefevre]

On 2018-03-13 13:55:27 +0100, Torbjorn Granlund wrote:
> Vincent Lefevre <vincent at vinc17.net> writes:
> 
>   On 2018-03-12 18:53:37 +0100, Marco Bodrato wrote:
>   > double
>   > mpz_get_d (const mpz_t u)
>   > {
>   >   static int c = 0;
>   >   static int __initialized = 0;
>   > 
>   >   if (__initialized != 1) {
>   >     c = gmp_tests_dbl_mant_bits ();
>   >     *((volatile int *) &__initialized) = 1;
>   >   }
> 
>   FYI, this is not thread-safe.
> 
> I suppose the externally visible store order is not guaranteed to be the
> same as the program store order, not even with "volatile".

More than an order issue, this is undefined behavior. In C11:

5.1.2.4 Multi-threaded executions and data races

   4  Two expression evaluations conflict if one of them modifies a
      memory location and the other one reads or modifies the same
      memory location.

  25  The execution of a program contains a data race if it contains
      two conflicting actions in different threads, at least one of
      which is not atomic, and neither happens before the other. Any
      such data race results in undefined behavior.

Actually the problem is not just on __initialized, but also on c:
if two threads test __initialized != 1 at the same time for their
first call, they will both execute

  c = gmp_tests_dbl_mant_bits ();

If the modification of c is not atomic, then may result in an
incorrect value for c, but also for other arbitrary variables
or more generally memory corruption (for instance, if int is
on 32 bits but CPU accesses are 64-bit only). Same problem with
__initialized if this has side effects on adjacent memory.

BTW, that's probably why the C standard says "undefined behavior"
and not just "indeterminate value".

> One variable, declared as volatile, should work fine though.

I don't think volatile matters. AFAIK, what you need is _Atomic (C11).

-- 
Vincent Lefèvre <vincent at vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)