From 7432af823a45b954a8bc6018cdc7fa4c1f8fd16f Mon Sep 17 00:00:00 2001 From: Robert Edmonds Date: Thu, 18 Sep 2014 18:03:12 -0400 Subject: [PATCH 1/2] Fix atomicops build failure on non-Clang We cannot use Clang's __has_extension macro unless we really are compiling on Clang, which means we cannot use this expression: #if (defined(__clang__) && __has_extension(c_atomic))) // ... #endif On GCC, this generates the following errors: In file included from ./google/protobuf/stubs/atomicops.h:59:0, from google/protobuf/stubs/atomicops_internals_x86_gcc.cc:36: ./google/protobuf/stubs/platform_macros.h:67:41: error: missing binary operator before token "(" (defined(__clang__) && __has_extension(c_atomic))) ^ In file included from google/protobuf/stubs/atomicops_internals_x86_gcc.cc:36:0: ./google/protobuf/stubs/atomicops.h:196:40: error: missing binary operator before token "(" (defined(__clang__) && __has_extension(c_atomic)) ^ Instead, we have to protect the __has_extension expression by only executing it when __clang__ is defined: #if defined(__clang__) # if __has_extension(c_atomic) // ... # endif #endif --- src/google/protobuf/stubs/atomicops.h | 9 +++++++-- src/google/protobuf/stubs/platform_macros.h | 18 +++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/google/protobuf/stubs/atomicops.h b/src/google/protobuf/stubs/atomicops.h index 8ddd2508f5875..17b7be7f969f4 100644 --- a/src/google/protobuf/stubs/atomicops.h +++ b/src/google/protobuf/stubs/atomicops.h @@ -192,9 +192,14 @@ GOOGLE_PROTOBUF_ATOMICOPS_ERROR #include #elif defined(__native_client__) #include -#elif (((__GNUC__ == 4) && (__GNUC_MINOR__ >= 7)) || (__GNUC__ > 4)) || \ - (defined(__clang__) && __has_extension(c_atomic)) +#elif (((__GNUC__ == 4) && (__GNUC_MINOR__ >= 7)) || (__GNUC__ > 4)) #include +#elif defined(__clang__) +#if __has_extension(c_atomic) +#include +#else +GOOGLE_PROTOBUF_ATOMICOPS_ERROR +#endif #else GOOGLE_PROTOBUF_ATOMICOPS_ERROR #endif diff --git a/src/google/protobuf/stubs/platform_macros.h b/src/google/protobuf/stubs/platform_macros.h index 02c79a6159a19..1d1d59a778f5d 100644 --- a/src/google/protobuf/stubs/platform_macros.h +++ b/src/google/protobuf/stubs/platform_macros.h @@ -33,6 +33,9 @@ #include +#define GOOGLE_PROTOBUF_PLATFORM_ERROR \ +#error "Host platform was not detected as supported by protobuf" + // Processor architecture detection. For more info on what's defined, see: // http://msdn.microsoft.com/en-us/library/b0084kay.aspx // http://www.agner.org/optimize/calling_conventions.pdf @@ -62,17 +65,22 @@ #endif #elif defined(__pnacl__) #define GOOGLE_PROTOBUF_ARCH_32_BIT 1 -#elif defined(__GNUC__) && \ - ((((__GNUC__ == 4) && (__GNUC_MINOR__ >= 7)) || (__GNUC__ > 4)) || \ - (defined(__clang__) && __has_extension(c_atomic))) -// We fallback to the generic GCC >= 4.7 implementation in atomicops.h +#elif defined(__GNUC__) +# if (((__GNUC__ == 4) && (__GNUC_MINOR__ >= 7)) || (__GNUC__ > 4)) +// We fallback to the generic Clang/GCC >= 4.7 implementation in atomicops.h +# elif defined(__clang__) +# if !__has_extension(c_atomic) +GOOGLE_PROTOBUF_PLATFORM_ERROR +# endif +// We fallback to the generic Clang/GCC >= 4.7 implementation in atomicops.h +# endif # if __LP64__ # define GOOGLE_PROTOBUF_ARCH_64_BIT 1 # else # define GOOGLE_PROTOBUF_ARCH_32_BIT 1 # endif #else -#error Host architecture was not detected as supported by protobuf +GOOGLE_PROTOBUF_PLATFORM_ERROR #endif #if defined(__APPLE__) From f252c07250c42fcacb7f4bd4ff3d9cc1a5145fcb Mon Sep 17 00:00:00 2001 From: Robert Edmonds Date: Thu, 18 Sep 2014 17:40:11 -0400 Subject: [PATCH 2/2] Remove Acquire_Store() and Release_Load() functions These functions are not used anywhere in protobuf. As a side effect, this fixes #25, since the buggy implementations are removed entirely. --- src/google/protobuf/stubs/atomicops.h | 4 --- .../stubs/atomicops_internals_arm64_gcc.h | 20 ------------- .../stubs/atomicops_internals_arm_gcc.h | 10 ------- .../stubs/atomicops_internals_arm_qnx.h | 10 ------- .../atomicops_internals_atomicword_compat.h | 8 ----- .../stubs/atomicops_internals_generic_gcc.h | 8 ----- .../stubs/atomicops_internals_macosx.h | 20 ------------- .../stubs/atomicops_internals_mips_gcc.h | 19 ------------ .../protobuf/stubs/atomicops_internals_tsan.h | 20 ------------- .../stubs/atomicops_internals_x86_gcc.h | 29 ------------------- .../stubs/atomicops_internals_x86_msvc.h | 20 ------------- 11 files changed, 168 deletions(-) diff --git a/src/google/protobuf/stubs/atomicops.h b/src/google/protobuf/stubs/atomicops.h index 17b7be7f969f4..235e0bf28f664 100644 --- a/src/google/protobuf/stubs/atomicops.h +++ b/src/google/protobuf/stubs/atomicops.h @@ -125,12 +125,10 @@ Atomic32 Release_CompareAndSwap(volatile Atomic32* ptr, #endif void MemoryBarrier(); void NoBarrier_Store(volatile Atomic32* ptr, Atomic32 value); -void Acquire_Store(volatile Atomic32* ptr, Atomic32 value); void Release_Store(volatile Atomic32* ptr, Atomic32 value); Atomic32 NoBarrier_Load(volatile const Atomic32* ptr); Atomic32 Acquire_Load(volatile const Atomic32* ptr); -Atomic32 Release_Load(volatile const Atomic32* ptr); // 64-bit atomic operations (only available on 64-bit processors). #ifdef GOOGLE_PROTOBUF_ARCH_64_BIT @@ -148,11 +146,9 @@ Atomic64 Release_CompareAndSwap(volatile Atomic64* ptr, Atomic64 old_value, Atomic64 new_value); void NoBarrier_Store(volatile Atomic64* ptr, Atomic64 value); -void Acquire_Store(volatile Atomic64* ptr, Atomic64 value); void Release_Store(volatile Atomic64* ptr, Atomic64 value); Atomic64 NoBarrier_Load(volatile const Atomic64* ptr); Atomic64 Acquire_Load(volatile const Atomic64* ptr); -Atomic64 Release_Load(volatile const Atomic64* ptr); #endif // GOOGLE_PROTOBUF_ARCH_64_BIT } // namespace internal diff --git a/src/google/protobuf/stubs/atomicops_internals_arm64_gcc.h b/src/google/protobuf/stubs/atomicops_internals_arm64_gcc.h index fe9727ad09da5..00b434ebdb39a 100644 --- a/src/google/protobuf/stubs/atomicops_internals_arm64_gcc.h +++ b/src/google/protobuf/stubs/atomicops_internals_arm64_gcc.h @@ -146,11 +146,6 @@ inline void NoBarrier_Store(volatile Atomic32* ptr, Atomic32 value) { *ptr = value; } -inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) { - *ptr = value; - MemoryBarrier(); -} - inline void Release_Store(volatile Atomic32* ptr, Atomic32 value) { __asm__ __volatile__ ( // NOLINT "stlr %w[value], %[ptr] \n\t" @@ -177,11 +172,6 @@ inline Atomic32 Acquire_Load(volatile const Atomic32* ptr) { return value; } -inline Atomic32 Release_Load(volatile const Atomic32* ptr) { - MemoryBarrier(); - return *ptr; -} - // 64-bit versions of the operations. // See the 32-bit versions for comments. @@ -282,11 +272,6 @@ inline void NoBarrier_Store(volatile Atomic64* ptr, Atomic64 value) { *ptr = value; } -inline void Acquire_Store(volatile Atomic64* ptr, Atomic64 value) { - *ptr = value; - MemoryBarrier(); -} - inline void Release_Store(volatile Atomic64* ptr, Atomic64 value) { __asm__ __volatile__ ( // NOLINT "stlr %x[value], %[ptr] \n\t" @@ -313,11 +298,6 @@ inline Atomic64 Acquire_Load(volatile const Atomic64* ptr) { return value; } -inline Atomic64 Release_Load(volatile const Atomic64* ptr) { - MemoryBarrier(); - return *ptr; -} - } // namespace internal } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/stubs/atomicops_internals_arm_gcc.h b/src/google/protobuf/stubs/atomicops_internals_arm_gcc.h index 1f4dedc0f3785..455e1f0ebfb11 100644 --- a/src/google/protobuf/stubs/atomicops_internals_arm_gcc.h +++ b/src/google/protobuf/stubs/atomicops_internals_arm_gcc.h @@ -119,11 +119,6 @@ inline void MemoryBarrier() { pLinuxKernelMemoryBarrier(); } -inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) { - *ptr = value; - MemoryBarrier(); -} - inline void Release_Store(volatile Atomic32* ptr, Atomic32 value) { MemoryBarrier(); *ptr = value; @@ -139,11 +134,6 @@ inline Atomic32 Acquire_Load(volatile const Atomic32* ptr) { return value; } -inline Atomic32 Release_Load(volatile const Atomic32* ptr) { - MemoryBarrier(); - return *ptr; -} - } // namespace internal } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/stubs/atomicops_internals_arm_qnx.h b/src/google/protobuf/stubs/atomicops_internals_arm_qnx.h index f05076978683f..e49df3c70c909 100644 --- a/src/google/protobuf/stubs/atomicops_internals_arm_qnx.h +++ b/src/google/protobuf/stubs/atomicops_internals_arm_qnx.h @@ -114,11 +114,6 @@ inline void MemoryBarrier() { __sync_synchronize(); } -inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) { - *ptr = value; - MemoryBarrier(); -} - inline void Release_Store(volatile Atomic32* ptr, Atomic32 value) { MemoryBarrier(); *ptr = value; @@ -134,11 +129,6 @@ inline Atomic32 Acquire_Load(volatile const Atomic32* ptr) { return value; } -inline Atomic32 Release_Load(volatile const Atomic32* ptr) { - MemoryBarrier(); - return *ptr; -} - } // namespace internal } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/stubs/atomicops_internals_atomicword_compat.h b/src/google/protobuf/stubs/atomicops_internals_atomicword_compat.h index e9d86797b5370..87d9a75714b20 100644 --- a/src/google/protobuf/stubs/atomicops_internals_atomicword_compat.h +++ b/src/google/protobuf/stubs/atomicops_internals_atomicword_compat.h @@ -93,10 +93,6 @@ inline void NoBarrier_Store(volatile AtomicWord *ptr, AtomicWord value) { NoBarrier_Store(reinterpret_cast(ptr), value); } -inline void Acquire_Store(volatile AtomicWord* ptr, AtomicWord value) { - return Acquire_Store(reinterpret_cast(ptr), value); -} - inline void Release_Store(volatile AtomicWord* ptr, AtomicWord value) { return Release_Store(reinterpret_cast(ptr), value); } @@ -109,10 +105,6 @@ inline AtomicWord Acquire_Load(volatile const AtomicWord* ptr) { return Acquire_Load(reinterpret_cast(ptr)); } -inline AtomicWord Release_Load(volatile const AtomicWord* ptr) { - return Release_Load(reinterpret_cast(ptr)); -} - } // namespace internal } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/stubs/atomicops_internals_generic_gcc.h b/src/google/protobuf/stubs/atomicops_internals_generic_gcc.h index e30bb44494ef0..969f2b185a150 100644 --- a/src/google/protobuf/stubs/atomicops_internals_generic_gcc.h +++ b/src/google/protobuf/stubs/atomicops_internals_generic_gcc.h @@ -82,10 +82,6 @@ inline void MemoryBarrier() { __sync_synchronize(); } -inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) { - __atomic_store_n(ptr, value, __ATOMIC_ACQUIRE); -} - inline void Release_Store(volatile Atomic32* ptr, Atomic32 value) { __atomic_store_n(ptr, value, __ATOMIC_RELEASE); } @@ -98,10 +94,6 @@ inline Atomic32 Acquire_Load(volatile const Atomic32* ptr) { return __atomic_load_n(ptr, __ATOMIC_ACQUIRE); } -inline Atomic32 Release_Load(volatile const Atomic32* ptr) { - return __atomic_load_n(ptr, __ATOMIC_RELEASE); -} - #ifdef __LP64__ inline void Release_Store(volatile Atomic64* ptr, Atomic64 value) { diff --git a/src/google/protobuf/stubs/atomicops_internals_macosx.h b/src/google/protobuf/stubs/atomicops_internals_macosx.h index f9b7581ad5682..88b1d00e04e30 100644 --- a/src/google/protobuf/stubs/atomicops_internals_macosx.h +++ b/src/google/protobuf/stubs/atomicops_internals_macosx.h @@ -101,11 +101,6 @@ inline void NoBarrier_Store(volatile Atomic32* ptr, Atomic32 value) { *ptr = value; } -inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) { - *ptr = value; - MemoryBarrier(); -} - inline void Release_Store(volatile Atomic32* ptr, Atomic32 value) { MemoryBarrier(); *ptr = value; @@ -121,11 +116,6 @@ inline Atomic32 Acquire_Load(volatile const Atomic32* ptr) { return value; } -inline Atomic32 Release_Load(volatile const Atomic32* ptr) { - MemoryBarrier(); - return *ptr; -} - #ifdef __LP64__ // 64-bit implementation on 64-bit platform @@ -191,11 +181,6 @@ inline void NoBarrier_Store(volatile Atomic64* ptr, Atomic64 value) { *ptr = value; } -inline void Acquire_Store(volatile Atomic64* ptr, Atomic64 value) { - *ptr = value; - MemoryBarrier(); -} - inline void Release_Store(volatile Atomic64* ptr, Atomic64 value) { MemoryBarrier(); *ptr = value; @@ -211,11 +196,6 @@ inline Atomic64 Acquire_Load(volatile const Atomic64* ptr) { return value; } -inline Atomic64 Release_Load(volatile const Atomic64* ptr) { - MemoryBarrier(); - return *ptr; -} - #endif // defined(__LP64__) } // namespace internal diff --git a/src/google/protobuf/stubs/atomicops_internals_mips_gcc.h b/src/google/protobuf/stubs/atomicops_internals_mips_gcc.h index cf79a13cb68a1..df6ca2e385505 100644 --- a/src/google/protobuf/stubs/atomicops_internals_mips_gcc.h +++ b/src/google/protobuf/stubs/atomicops_internals_mips_gcc.h @@ -153,11 +153,6 @@ inline void MemoryBarrier() { __asm__ __volatile__("sync" : : : "memory"); } -inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) { - *ptr = value; - MemoryBarrier(); -} - inline void Release_Store(volatile Atomic32* ptr, Atomic32 value) { MemoryBarrier(); *ptr = value; @@ -173,11 +168,6 @@ inline Atomic32 Acquire_Load(volatile const Atomic32* ptr) { return value; } -inline Atomic32 Release_Load(volatile const Atomic32* ptr) { - MemoryBarrier(); - return *ptr; -} - #if defined(__LP64__) // 64-bit versions of the atomic ops. @@ -278,11 +268,6 @@ inline void NoBarrier_Store(volatile Atomic64* ptr, Atomic64 value) { *ptr = value; } -inline void Acquire_Store(volatile Atomic64* ptr, Atomic64 value) { - *ptr = value; - MemoryBarrier(); -} - inline void Release_Store(volatile Atomic64* ptr, Atomic64 value) { MemoryBarrier(); *ptr = value; @@ -298,10 +283,6 @@ inline Atomic64 Acquire_Load(volatile const Atomic64* ptr) { return value; } -inline Atomic64 Release_Load(volatile const Atomic64* ptr) { - MemoryBarrier(); - return *ptr; -} #endif } // namespace internal diff --git a/src/google/protobuf/stubs/atomicops_internals_tsan.h b/src/google/protobuf/stubs/atomicops_internals_tsan.h index 8112ee02cd916..9bcf4a50202f5 100644 --- a/src/google/protobuf/stubs/atomicops_internals_tsan.h +++ b/src/google/protobuf/stubs/atomicops_internals_tsan.h @@ -104,11 +104,6 @@ inline void NoBarrier_Store(volatile Atomic32 *ptr, Atomic32 value) { __tsan_atomic32_store(ptr, value, __tsan_memory_order_relaxed); } -inline void Acquire_Store(volatile Atomic32 *ptr, Atomic32 value) { - __tsan_atomic32_store(ptr, value, __tsan_memory_order_relaxed); - __tsan_atomic_thread_fence(__tsan_memory_order_seq_cst); -} - inline void Release_Store(volatile Atomic32 *ptr, Atomic32 value) { __tsan_atomic32_store(ptr, value, __tsan_memory_order_release); } @@ -121,11 +116,6 @@ inline Atomic32 Acquire_Load(volatile const Atomic32 *ptr) { return __tsan_atomic32_load(ptr, __tsan_memory_order_acquire); } -inline Atomic32 Release_Load(volatile const Atomic32 *ptr) { - __tsan_atomic_thread_fence(__tsan_memory_order_seq_cst); - return __tsan_atomic32_load(ptr, __tsan_memory_order_relaxed); -} - inline Atomic64 NoBarrier_CompareAndSwap(volatile Atomic64 *ptr, Atomic64 old_value, Atomic64 new_value) { @@ -166,11 +156,6 @@ inline void NoBarrier_Store(volatile Atomic64 *ptr, Atomic64 value) { __tsan_atomic64_store(ptr, value, __tsan_memory_order_relaxed); } -inline void Acquire_Store(volatile Atomic64 *ptr, Atomic64 value) { - __tsan_atomic64_store(ptr, value, __tsan_memory_order_relaxed); - __tsan_atomic_thread_fence(__tsan_memory_order_seq_cst); -} - inline void Release_Store(volatile Atomic64 *ptr, Atomic64 value) { __tsan_atomic64_store(ptr, value, __tsan_memory_order_release); } @@ -183,11 +168,6 @@ inline Atomic64 Acquire_Load(volatile const Atomic64 *ptr) { return __tsan_atomic64_load(ptr, __tsan_memory_order_acquire); } -inline Atomic64 Release_Load(volatile const Atomic64 *ptr) { - __tsan_atomic_thread_fence(__tsan_memory_order_seq_cst); - return __tsan_atomic64_load(ptr, __tsan_memory_order_relaxed); -} - inline Atomic64 Acquire_CompareAndSwap(volatile Atomic64 *ptr, Atomic64 old_value, Atomic64 new_value) { diff --git a/src/google/protobuf/stubs/atomicops_internals_x86_gcc.h b/src/google/protobuf/stubs/atomicops_internals_x86_gcc.h index 5324dfbcb7b45..3de7ebb437a3b 100644 --- a/src/google/protobuf/stubs/atomicops_internals_x86_gcc.h +++ b/src/google/protobuf/stubs/atomicops_internals_x86_gcc.h @@ -123,11 +123,6 @@ inline void MemoryBarrier() { __asm__ __volatile__("mfence" : : : "memory"); } -inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) { - *ptr = value; - MemoryBarrier(); -} - #else inline void MemoryBarrier() { @@ -139,15 +134,6 @@ inline void MemoryBarrier() { } } -inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) { - if (AtomicOps_Internalx86CPUFeatures.has_sse2) { - *ptr = value; - __asm__ __volatile__("mfence" : : : "memory"); - } else { - NoBarrier_AtomicExchange(ptr, value); - // acts as a barrier on PIII - } -} #endif inline void Release_Store(volatile Atomic32* ptr, Atomic32 value) { @@ -167,11 +153,6 @@ inline Atomic32 Acquire_Load(volatile const Atomic32* ptr) { return value; } -inline Atomic32 Release_Load(volatile const Atomic32* ptr) { - MemoryBarrier(); - return *ptr; -} - #if defined(__x86_64__) // 64-bit low-level operations on 64-bit platform. @@ -223,11 +204,6 @@ inline void NoBarrier_Store(volatile Atomic64* ptr, Atomic64 value) { *ptr = value; } -inline void Acquire_Store(volatile Atomic64* ptr, Atomic64 value) { - *ptr = value; - MemoryBarrier(); -} - inline void Release_Store(volatile Atomic64* ptr, Atomic64 value) { ATOMICOPS_COMPILER_BARRIER(); @@ -261,11 +237,6 @@ inline Atomic64 Acquire_Load(volatile const Atomic64* ptr) { return value; } -inline Atomic64 Release_Load(volatile const Atomic64* ptr) { - MemoryBarrier(); - return *ptr; -} - inline Atomic64 Acquire_CompareAndSwap(volatile Atomic64* ptr, Atomic64 old_value, Atomic64 new_value) { diff --git a/src/google/protobuf/stubs/atomicops_internals_x86_msvc.h b/src/google/protobuf/stubs/atomicops_internals_x86_msvc.h index 6f9869d1fcd35..13314ba8acb5f 100644 --- a/src/google/protobuf/stubs/atomicops_internals_x86_msvc.h +++ b/src/google/protobuf/stubs/atomicops_internals_x86_msvc.h @@ -62,11 +62,6 @@ inline void NoBarrier_Store(volatile Atomic32* ptr, Atomic32 value) { *ptr = value; } -inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) { - NoBarrier_AtomicExchange(ptr, value); - // acts as a barrier in this implementation -} - inline void Release_Store(volatile Atomic32* ptr, Atomic32 value) { *ptr = value; // works w/o barrier for current Intel chips as of June 2005 // See comments in Atomic64 version of Release_Store() below. @@ -81,11 +76,6 @@ inline Atomic32 Acquire_Load(volatile const Atomic32* ptr) { return value; } -inline Atomic32 Release_Load(volatile const Atomic32* ptr) { - MemoryBarrier(); - return *ptr; -} - #if defined(_WIN64) // 64-bit low-level operations on 64-bit platform. @@ -99,11 +89,6 @@ inline void NoBarrier_Store(volatile Atomic64* ptr, Atomic64 value) { *ptr = value; } -inline void Acquire_Store(volatile Atomic64* ptr, Atomic64 value) { - NoBarrier_AtomicExchange(ptr, value); - // acts as a barrier in this implementation -} - inline void Release_Store(volatile Atomic64* ptr, Atomic64 value) { *ptr = value; // works w/o barrier for current Intel chips as of June 2005 @@ -124,11 +109,6 @@ inline Atomic64 Acquire_Load(volatile const Atomic64* ptr) { return value; } -inline Atomic64 Release_Load(volatile const Atomic64* ptr) { - MemoryBarrier(); - return *ptr; -} - inline Atomic64 Acquire_CompareAndSwap(volatile Atomic64* ptr, Atomic64 old_value, Atomic64 new_value) {