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

base/atomicops_internals_gcc.h

Index: base/atomicops_internals_gcc.h
===================================================================
--- base/atomicops_internals_gcc.h	(revision 152192)
+++ base/atomicops_internals_gcc.h	(working copy)
@@ -1,4 +1,4 @@
-// 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.
 
@@ -6,12 +6,19 @@
 //
 // 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
@@ -27,14 +34,17 @@
 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;
@@ -47,8 +57,7 @@
   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;
 }
 
@@ -63,14 +72,12 @@
     // 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,
@@ -90,7 +97,7 @@
 }
 
 inline void MemoryBarrier() {
-  pLinuxKernelMemoryBarrier();
+  memory_barrier();
 }
 
 inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) {
@@ -118,7 +125,8 @@
   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_
+