Skip to content

Commit

Permalink
generic atomicops: promote Acquire_Store() and Release_Load() to use …
Browse files Browse the repository at this point in the history
…SEQ_CST fence

__atomic_store_n() cannot take a memory model argument of
__ATOMIC_ACQUIRE, and __atomic_load_n() cannot take a memory model
argument of __ATOMIC_RELEASE, per the GCC documentation:

    https://gcc.gnu.org/onlinedocs/gcc-4.9.1/gcc/_005f_005fatomic-Builtins.html

On Clang this generates a -Watomic-memory-ordering warning.

Promote the fences in Acquire_Store() and Release_Load() to the stronger
__ATOMIC_SEQ_CST memory model, which ought to be safe.

Note that there are no actual uses of Acquire_Store() or Release_Load()
in protobuf, though.

This follows the TSAN atomicops implementation, which also uses SEQ_CST
fences for these functions.

(Fixes #25.)
  • Loading branch information
edmonds committed Sep 19, 2014
1 parent 8d5f1e1 commit cc0a047
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/google/protobuf/stubs/atomicops_internals_generic_gcc.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ inline void MemoryBarrier() {
}

inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) {
__atomic_store_n(ptr, value, __ATOMIC_ACQUIRE);
__atomic_store_n(ptr, value, __ATOMIC_SEQ_CST);
}

inline void Release_Store(volatile Atomic32* ptr, Atomic32 value) {
Expand All @@ -99,7 +99,7 @@ inline Atomic32 Acquire_Load(volatile const Atomic32* ptr) {
}

inline Atomic32 Release_Load(volatile const Atomic32* ptr) {
return __atomic_load_n(ptr, __ATOMIC_RELEASE);
return __atomic_load_n(ptr, __ATOMIC_SEQ_CST);
}

#ifdef __LP64__
Expand Down

0 comments on commit cc0a047

Please sign in to comment.