Fix atomic ops on ARM to compile in NaCl untrusted targets.

base/atomicops_internals_gcc.h

-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[Lei Zhang, 2012/08/21 18:44:47]

Apparently we don't do this anymore. See relevant chromium-dev thread.

[bbudge, 2012/08/21 21:49:02]

Done.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This file is an internal atomic implementation, use base/atomicops.h instead.

[Lei Zhang, 2012/08/21 18:44:47]

Can you add a comment here to say what platforms this is for? Previously, it was
obvious it was for ARM from the file name.

[bbudge, 2012/08/21 21:49:02]

Done.

 //
 // LinuxKernelCmpxchg and Barrier_AtomicIncrement are from Google Gears.
 
-#ifndef BASE_ATOMICOPS_INTERNALS_ARM_GCC_H_
-#define BASE_ATOMICOPS_INTERNALS_ARM_GCC_H_
+#ifndef BASE_ATOMICOPS_INTERNALS_GCC_H_
+#define BASE_ATOMICOPS_INTERNALS_GCC_H_
 
 namespace base {
 namespace subtle {
 
+#if defined(OS_NACL)
+// For Native Client, define atomic ops in terms of GCC intrinsics.
+#define compare_and_swap(ptr, old_value, new_value) \
+    __sync_val_compare_and_swap(const_cast<Atomic32*>(ptr), \

[nfullagar, 2012/08/21 18:23:20]

The “val” version of __sync_val_compare_and_swap returns the contents of *ptr
before the operation.  But code below looks like it is expecting a bool result?

[bbudge, 2012/08/21 19:33:57]

It does appear that way, but the old function signature returns Atomic32, which
led me to the 'val' version. Neither one works in the NaCl IPC proxy though.

+                                old_value, new_value)
+#define memory_barrier __sync_synchronize
+#else  // !defined(OS_NACL)

[DaleCurtis, 2012/08/21 17:52:01]

Per http://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libgcc/config/arm/linux-atomic.c
the GCC intrinsics are implemented using the same linux primitives, so I think
you can safely have everyone use the gcc intrinsics.

Looks like it's always been implemented that way, so whatever version we're
using should be fine too:

http://gcc.gnu.org/ml/gcc-patches/2008-07/msg00025.html

[bbudge, 2012/08/21 19:33:57]

I'm going to run a trial on the normal Chrome ARM build to see if this is true,
and that all tests pass.

 // 0xffff0fc0 is the hard coded address of a function provided by
 // the kernel which implements an atomic compare-exchange. On older
 // ARM architecture revisions (pre-v6) this may be implemented using
 // a syscall. This address is stable, and in active use (hard coded)
 // by at least glibc-2.7 and the Android C library.
 typedef Atomic32 (*LinuxKernelCmpxchgFunc)(Atomic32 old_value,
                                            Atomic32 new_value,
                                            volatile Atomic32* ptr);
 LinuxKernelCmpxchgFunc pLinuxKernelCmpxchg __attribute__((weak)) =
     (LinuxKernelCmpxchgFunc) 0xffff0fc0;
 
 typedef void (*LinuxKernelMemoryBarrierFunc)(void);
 LinuxKernelMemoryBarrierFunc pLinuxKernelMemoryBarrier __attribute__((weak)) =
     (LinuxKernelMemoryBarrierFunc) 0xffff0fa0;
 
+#define compare_and_swap(ptr, old_value, new_value) \
+    pLinuxKernelCmpxchg(old_value, new_value, const_cast<Atomic32*>(ptr))
+#define memory_barrier pLinuxKernelMemoryBarrier
+#endif  // !defined(OS_NACL)
 
 inline Atomic32 NoBarrier_CompareAndSwap(volatile Atomic32* ptr,
                                          Atomic32 old_value,
                                          Atomic32 new_value) {
   Atomic32 prev_value = *ptr;
   do {
-    if (!pLinuxKernelCmpxchg(old_value, new_value,
-                             const_cast<Atomic32*>(ptr))) {
+    if (!compare_and_swap(ptr, old_value, new_value)) {
       return old_value;
     }
     prev_value = *ptr;
   } while (prev_value == old_value);
   return prev_value;
 }
 
 inline Atomic32 NoBarrier_AtomicExchange(volatile Atomic32* ptr,
                                          Atomic32 new_value) {
   Atomic32 old_value;
   do {
     old_value = *ptr;
-  } while (pLinuxKernelCmpxchg(old_value, new_value,
-                               const_cast<Atomic32*>(ptr)));
+  } while (compare_and_swap(ptr, old_value, new_value));
   return old_value;
 }
 
 inline Atomic32 NoBarrier_AtomicIncrement(volatile Atomic32* ptr,
                                           Atomic32 increment) {
   return Barrier_AtomicIncrement(ptr, increment);
 }
 
 inline Atomic32 Barrier_AtomicIncrement(volatile Atomic32* ptr,
                                         Atomic32 increment) {
   for (;;) {
     // Atomic exchange the old value with an incremented one.
     Atomic32 old_value = *ptr;
     Atomic32 new_value = old_value + increment;
-    if (pLinuxKernelCmpxchg(old_value, new_value,
-                            const_cast<Atomic32*>(ptr)) == 0) {
+    if (compare_and_swap(ptr, old_value, new_value) == 0) {
       // The exchange took place as expected.
       return new_value;
     }
     // Otherwise, *ptr changed mid-loop and we need to retry.
   }
-

[Lei Zhang, 2012/08/21 18:44:47]

nit: extra new line?

[bbudge, 2012/08/21 21:49:02]

Done.

 }
 
 inline Atomic32 Acquire_CompareAndSwap(volatile Atomic32* ptr,
                                        Atomic32 old_value,
                                        Atomic32 new_value) {
   return NoBarrier_CompareAndSwap(ptr, old_value, new_value);
 }
 
 inline Atomic32 Release_CompareAndSwap(volatile Atomic32* ptr,
                                        Atomic32 old_value,
                                        Atomic32 new_value) {
   return NoBarrier_CompareAndSwap(ptr, old_value, new_value);
 }
 
 inline void NoBarrier_Store(volatile Atomic32* ptr, Atomic32 value) {
   *ptr = value;
 }
 
 inline void MemoryBarrier() {
-  pLinuxKernelMemoryBarrier();
+  memory_barrier();
 }
 
 inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) {
   *ptr = value;
   MemoryBarrier();
 }
 
 inline void Release_Store(volatile Atomic32* ptr, Atomic32 value) {
   MemoryBarrier();
   *ptr = value;
 }
 
 inline Atomic32 NoBarrier_Load(volatile const Atomic32* ptr) {
   return *ptr;
 }
 
 inline Atomic32 Acquire_Load(volatile const Atomic32* ptr) {
   Atomic32 value = *ptr;
   MemoryBarrier();
   return value;
 }
 
 inline Atomic32 Release_Load(volatile const Atomic32* ptr) {
   MemoryBarrier();
   return *ptr;
 }
 
-} // namespace base::subtle
-} // namespace base
+}  // namespace base::subtle
+}  // namespace base
 
-#endif  // BASE_ATOMICOPS_INTERNALS_ARM_GCC_H_
+#endif  // BASE_ATOMICOPS_INTERNALS_GCC_H_
+