Avoid need for valgrind suppressions for pg_atomic_init_u64 on some platforms.
authorAndres Freund <andres@anarazel.de>
Tue, 9 Jun 2020 02:52:19 +0000 (19:52 -0700)
committerAndres Freund <andres@anarazel.de>
Tue, 9 Jun 2020 03:02:56 +0000 (20:02 -0700)
Previously we used pg_atomic_write_64_impl inside
pg_atomic_init_u64. That works correctly, but on platforms without
64bit single copy atomicity it could trigger spurious valgrind errors
about uninitialized memory, because we use compare_and_swap for atomic
writes on such platforms.

I previously suppressed one instance of this problem (6c878edc1df),
but as Tom reports that wasn't enough. As the atomic variable cannot
yet be concurrently accessible during initialization, it seems better
to have pg_atomic_init_64_impl set the value directly.

Change pg_atomic_init_u32_impl for symmetry.

Reported-By: Tom Lane
Author: Andres Freund
Discussion: https://postgr.es/m/1714601.1591503815@sss.pgh.pa.us
Backpatch: 9.5-

src/include/port/atomics/generic.h
src/tools/valgrind.supp

index ea11698a3582e0638b9b3b8bc8fdc9805812b47d..1a66896d902c90bb4c3b4c5ba15ceb0b6402e4b8 100644 (file)
@@ -160,7 +160,7 @@ pg_atomic_clear_flag_impl(volatile pg_atomic_flag *ptr)
 static inline void
 pg_atomic_init_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val_)
 {
-   pg_atomic_write_u32_impl(ptr, val_);
+   ptr->value = val_;
 }
 #endif
 
@@ -330,7 +330,7 @@ pg_atomic_read_u64_impl(volatile pg_atomic_uint64 *ptr)
 static inline void
 pg_atomic_init_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 val_)
 {
-   pg_atomic_write_u64_impl(ptr, val_);
+   ptr->value = val_;
 }
 #endif
 
index ead3085831467a2bd11d4039f77c676145181357..e82ec2ab3941101cf19c46783ffc3bfc69fba9d0 100644 (file)
    fun:IsBinaryCoercible
 }
 
-# Atomic writes to 64bit atomic vars uses compare/exchange to
-# guarantee atomic writes of 64bit variables. pg_atomic_write is used
-# during initialization of the atomic variable; that leads to an
-# initial read of the old, undefined, memory value. But that's just to
-# make sure the swap works correctly.
-{
-   uninitialized_atomic_init_u64
-   Memcheck:Cond
-   fun:pg_atomic_exchange_u64_impl
-   fun:pg_atomic_write_u64_impl
-   fun:pg_atomic_init_u64_impl
-}
-
-
 # Python's allocator does some low-level tricks for efficiency. Those
 # can be disabled for better instrumentation; but few people testing
 # postgres will have such a build of python. So add broad