Fix typos in compiler-rt/lib/builtins/atomic.c

[xingxue]

Summary

This patch fixes typos in file compiler-rt/lib/builtins/atomic.c.

[jfb]

How did this ever work?

[hubert.reinterpretcast]

This leaves me wondering where the tests for this file are.

compiler-rt/lib/builtins/atomic.c

137 ↗

(On Diff #190138)

Is IS_LOCK_FREE_1 omitted on purpose?

142 ↗

(On Diff #190138)

The LOCK_FREE_ACTION is not supposed to fall-through to here. I am not sure the change is helpful (and it may reduce the ability of compiler warnings to catch bad definitions of LOCK_FREE_ACTION).

[hubert.reinterpretcast]

compiler-rt/lib/builtins/atomic.c

137 ↗

(On Diff #190138)

__atomic_compare_exchange_c is lockful for 1-byte size regardless of platform support. A use of __atomic_store_1 is potentially lock-free depending on platform support. It is therefore possible for an __atomic_store_1 to violate the locking performed by __atomic_compare_exchange_c. This can potentially manifest as the __atomic_store_1 being never observed by the thread performing __atomic_compare_exchange_c.

[xingxue]

compiler-rt/lib/builtins/atomic.c

137 ↗

(On Diff #190138)

I think there should be a case for 1 byte as well. Will add:

case 1:\
  if (IS_LOCK_FREE_1) {\
    LOCK_FREE_ACTION(uint8_t);\
  }\
  break; \

142 ↗

(On Diff #190138)

LOCK_FREE_ACTION is not supposed to fall-through. However, when IS_LOCK_FREE_* is false, the 'if' statement will fall-through.

[xingxue]

Address review comments:

• Added a case for 1 byte to macro LOCK_FREE_CASES.

compiler-rt/trunk/lib/builtins/atomic.c

 #define IS_LOCK_FREE_8 __c11_atomic_is_lock_free(8)
 #define IS_LOCK_FREE_16 0
 
 /// Macro that calls the compiler-generated lock-free versions of functions
 /// when they exist.
 #define LOCK_FREE_CASES() \
   do {\
   switch (size) {\
+    case 1:\
+      if (IS_LOCK_FREE_1) {\
+        LOCK_FREE_ACTION(uint8_t);\
+      }\
+      break; \
     case 2:\
       if (IS_LOCK_FREE_2) {\
         LOCK_FREE_ACTION(uint16_t);\
       }\
+      break; \
     case 4:\
       if (IS_LOCK_FREE_4) {\
         LOCK_FREE_ACTION(uint32_t);\
       }\
+      break; \
     case 8:\
       if (IS_LOCK_FREE_8) {\
         LOCK_FREE_ACTION(uint64_t);\
       }\
+      break; \
     case 16:\
       if (IS_LOCK_FREE_16) {\
         /* FIXME: __uint128_t isn't available on 32 bit platforms.
         LOCK_FREE_ACTION(__uint128_t);*/\
       }\
+      break; \
   }\
   } while (0)
 
 
 /// An atomic load operation.  This is atomic with respect to the source
 /// pointer only.
 void __atomic_load_c(int size, void *src, void *dest, int model) {
 #define LOCK_FREE_ACTION(type) \
     *((type*)dest) = __c11_atomic_load((_Atomic(type)*)src, model);\
     return;
   LOCK_FREE_CASES();
 #undef LOCK_FREE_ACTION
   Lock *l = lock_for_pointer(src);
   lock(l);
   memcpy(dest, src, size);
   unlock(l);
 }
 
 /// An atomic store operation.  This is atomic with respect to the destination
 /// pointer only.
 void __atomic_store_c(int size, void *dest, void *src, int model) {
 #define LOCK_FREE_ACTION(type) \
-    __c11_atomic_store((_Atomic(type)*)dest, *(type*)dest, model);\
+    __c11_atomic_store((_Atomic(type)*)dest, *(type*)src, model);\
     return;
   LOCK_FREE_CASES();
 #undef LOCK_FREE_ACTION
   Lock *l = lock_for_pointer(dest);
   lock(l);
   memcpy(dest, src, size);
   unlock(l);
 }