valgrind versus pg_atomic_init()

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: valgrind versus pg_atomic_init()
Date: 2020-06-07 04:23:35
Message-ID: 1714601.1591503815@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I experimented with running "make check" on ARM64 under a reasonably
bleeding-edge valgrind (3.16.0). One thing I ran into is that
regress.c's test_atomic_ops fails; valgrind shows the stack trace

fun:__aarch64_cas8_acq_rel
fun:pg_atomic_compare_exchange_u64_impl
fun:pg_atomic_exchange_u64_impl
fun:pg_atomic_write_u64_impl
fun:pg_atomic_init_u64_impl
fun:pg_atomic_init_u64
fun:test_atomic_uint64
fun:test_atomic_ops
fun:ExecInterpExpr

Now, this is basically the same thing as is already memorialized in
src/tools/valgrind.supp:

# 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
}

so my first thought was that we just needed an architecture-specific
variant of that. But on thinking more about this, it seems like
generic.h's version of pg_atomic_init_u64_impl is just fundamentally
misguided. Why isn't it simply assigning the value with an ordinary
unlocked write? By definition, we had better not be using this function
in any circumstance where there might be conflicting accesses, so I don't
see why we should need to tolerate a valgrind exception here. Moreover,
if a simple assignment *isn't* good enough, then surely the spinlock
version in atomics.c is 100% broken.

regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Drouvot, Bertrand 2020-06-07 06:12:59 Re: Add LWLock blocker(s) information
Previous Message Tom Lane 2020-06-07 03:59:27 Re: valgrind error