[compiler-rt] Allow atomic_test to pass on x86_64

[arichardson]

Summary

c11_atomic_load is implemented using cmpxcghg16b on x86_64 (assuming
compiler-rt is compiled with a new-enough -march= flag) and this
instruction requires a writable memory mapping.
To allow for this case, use a writable mapping and make the atomic_load
function signature match the one atomic.c (non-const src pointer).

I do not know if __atomic_load needs to support read-only mappings, but if
it does we need to disable the lock-free path for 16-byte atomic load.

I believe this is currently only an issue on macOS systems since the clang
driver defaults to enabling cmpxchg16b support, whereas other targets need an
explicit -march= flag.
This change allows atomic_test.c to pass on my macOS system, before it would
fail with a EXC_BAD_ACCESS error.

[jrtc27]

Note that this shows there is currently another bug in x86_64 clang (ignoring OS, since with manual -march=athlon64 on macOS you can also get the non-macOS behaviour). Whether or not libatomic is used is part of the ABI, as libatomic may (and likely will for 16B atomics on your standard Linux distro) use hash tables of locks to emulate atomic operations, which will not correctly synchronise with the inlined hardware atomics. See https://godbolt.org/z/fqfvev for evidence of the ABI breakage.

[arichardson]

It appears that GCC changed its behaviour from cmpxchg to calling the helper function for GCC 7: https://godbolt.org/z/s9fxfY

See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70490 and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80878

[luismarques]

It seems that everything is pointing towards keeping it non-writeable?

• DR 459 was resolved by adding const.
• GCC seems to have moved away from cmpxchg.
• Jessica's point about clang's behavior leading to ABI incompatibilities.

Even if we accept that's the case, should this test still have conditional workarounds for problematic implementations?

[arichardson]

Not planning to continue work on this.

compiler-rt/test/builtins/Unit/atomic_test.c

 #include <stdlib.h>
 #include <string.h>
 
 // We directly test the library atomic functions, not using the C builtins. This
 // should avoid confounding factors, ensuring that we actually test the
 // functions themselves, regardless of how the builtins are lowered. We need to
 // use asm labels because we can't redeclare the builtins.
 
-void __atomic_load_c(int size, const void *src, void *dest,
+void __atomic_load_c(int size, void *src, void *dest,
                      int model) asm("__atomic_load");
 
 uint8_t __atomic_load_1(uint8_t *src, int model);
 uint16_t __atomic_load_2(uint16_t *src, int model);
 uint32_t __atomic_load_4(uint32_t *src, int model);
 uint64_t __atomic_load_8(uint64_t *src, int model);
 
 void __atomic_store_c(int size, void *dest, const void *src,
static int atomic_load_models[] = {
       __ATOMIC_SEQ_CST,
   };
 
   for (int m = 0; m < LEN(atomic_load_models); m++) {
     int model = atomic_load_models[m];
 
     // Test with aligned data.
     for (int n = 1; n <= LEN(data); n++) {
+      // Note: __atomic_load requires a writable mapping, so we copy the data.
+      __attribute__((aligned(16))) char src[LEN(data)] = {0};
       __attribute__((aligned(16))) char dst[LEN(data)] = {0};
-      __atomic_load_c(n, data, dst, model);
+      memcpy(src, data, sizeof(src));
+      __atomic_load_c(n, src, dst, model);
       if (memcmp(dst, data, n) != 0)
         abort();
     }
 
     // Test with unaligned data.
     for (int n = 1; n < LEN(data); n++) {
+      // Note: __atomic_load requires a writable mapping, so we copy the data.
+      __attribute__((aligned(16))) char src[LEN(data)] = {0};
       __attribute__((aligned(16))) char dst[LEN(data)] = {0};
-      __atomic_load_c(n, data + 1, dst + 1, model);
+      memcpy(src, data, sizeof(src));
+      __atomic_load_c(n, src + 1, dst + 1, model);
       if (memcmp(dst + 1, data + 1, n) != 0)
         abort();
     }
 
     set_a_values(V + m);
     if (__atomic_load_1(&a8, model) != U8(V + m))
       abort();
     if (__atomic_load_2(&a16, model) != U16(V + m))