testsuite: tolerate compare_exchange_weak spurious failure

[Jonathan Wakely]

On Wed, 11 Mar 2026 at 11:41, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>
> On Wed, 11 Mar 2026 at 10:56, Alexandre Oliva <oliva@adacore.com> wrote:
> >
> > On Mar 11, 2026, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> >
> > > On Wed, 11 Mar 2026, 05:29 Alexandre Oliva, <oliva@adacore.com> wrote:
> > >>
> > >> compare_exchange_weak is allowed to fail spuriously, and we're
> > >> actually hitting such spurious failures quite often on ppc-vx7r2,
> > >> presumably when random interrupts hit between ll and sc.
> > >>
> > >> Tolerate one such failure, hopefully that will suffice.
> >
> > > This is a bit of a problem, because we can't tell if the padding was
> > > cleared or not. If it was not clear, the first compare_exchange_weak would
> > > clear it, and the refund would succeed. But we want the test to fail in
> > > that case.
> >
> > Erhm...  My understanding is that, when it returns false, no memory is
> > changed, so the second attempt would start from exactly the same state
> > as the first.
>
> "Remarks: A weak compare-and-exchange operation may fail spuriously.
> That is, even when the contents of memory referred to by expected and
> ptr are equal, it may return false and store back to expected the same
> memory contents that were originally there."
>
> So `expected` is updated on a spurious failure, even if that doesn't
> change any bits.

I might have got a bit carried away, but I rewrote the whole test. How
does this look? This should solve the problem with spurious weak
failures, and no longer require -fno-tree-sra to retrieve the padding
bits from the value in the atomic.
-------------- next part --------------
commit a50d1f44cbb9e14228f6287116e6c0e448e8f952
Author:     Jonathan Wakely <jwakely@redhat.com>
AuthorDate: Wed Mar 11 11:41:56 2026
Commit:     Jonathan Wakely <redi@gcc.gnu.org>
CommitDate: Wed Mar 11 11:48:14 2026

    libstdc++: Rewrite test for compare_exchange padding
    
    Add a new helper function to verify that the padding bits in a struct S
    and in the std::atomic<S> are zero, without needing to disable SRA. Then
    rewrite the test in terms of that helper, so we check directly that the
    atomic object has no padding bits, instead of loading a value (with
    -fno-tree-sra to hopefully preserve padding) and then just using memcmp
    on that value.
    
    Also add a loop around the weak compare-exchange, to tolerate spurious
    failures as described in
    https://gcc.gnu.org/pipermail/gcc-patches/2026-March/710289.html
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/29_atomics/atomic/compare_exchange_padding.cc:
            Rewrite to be robust against spurious failures of weak compare
            exchange, and to check padding bits more directly.

diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
index d9c7b90c1a5c..41c77da823a4 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
@@ -1,22 +1,52 @@
 // { dg-do run { target c++11 } }
 // { dg-require-atomic-cmpxchg-word "" }
 // { dg-add-options libatomic }
-// { dg-additional-options "-fno-tree-sra" }
 
 #include <atomic>
 #include <cstring>
+#include <cstdint>
 
 #include <testsuite_hooks.h>
 
-struct S { char c; short s; };
+struct S
+{
+    char c;
+    alignas(2) std::int16_t s;
+
+    bool operator==(const S& rhs) const { return c == rhs.c && s == rhs.s; }
+};
 
 void __attribute__((noinline,noipa))
 fill_struct(S& s)
 { std::memset(&s, 0xff, sizeof(S)); }
 
+bool __attribute__((noinline,noipa))
+padding_is_zero(const S& s)
+{
+  unsigned char bytes[sizeof(S)];
+  std::memcpy(bytes, &s, sizeof(S));
+  return bytes[1] == 0;
+}
+
 bool
-compare_struct(const S& a, const S& b)
-{ return std::memcmp(&a, &b, sizeof(S)) == 0; }
+padding_is_zero(std::atomic<S>& a)
+{
+  const S dummy{};
+
+  // a.load() does not necessarily preserve padding bits, because the
+  // temporary returned from a.load() might get SRA'd so that the two
+  // data members are copied in separate registers and padding bits are
+  // not copied.
+  S s = a.load();
+
+  // So instead we use compare-exchange with an incorrect 'expected' value,
+  s.c = ~s.c;
+  // so that the compare will fail and return the value (including padding):
+  a.compare_exchange_weak(s, dummy);
+
+  // Now we can inspect the padding bits to check they are zero:
+  return padding_is_zero(s);
+}
 
 int
 main ()
@@ -27,18 +57,35 @@ main ()
   s.s = 42;
 
   std::atomic<S> as{ s };
-  auto ts = as.load(); // SRA might prevent copying of padding bits here.
-  VERIFY( !compare_struct(s, ts) ); // padding cleared on construction
+  VERIFY( as.load() == s );      // members are set correctly
+  VERIFY( padding_is_zero(as) ); // but padding was cleared on construction
+
+  ++s.s;
   as.exchange(s);
-  auto es = as.load(); // SRA might prevent copying of padding bits here.
-  VERIFY( compare_struct(ts, es) ); // padding cleared on exchange
+  VERIFY( as.load() == s );      // members are set correctly
+  VERIFY( padding_is_zero(as) ); // but padding was cleared on construction
 
   S n;
   fill_struct(n);
   n.c = 'b';
   n.s = 71;
-  // padding cleared on compexchg
-  VERIFY( as.compare_exchange_weak(s, n) );
-  VERIFY( as.compare_exchange_strong(n, s) );
-  return 0;
+  // padding in S ignored for compare, and padding in N not stored into AS:
+  VERIFY( as.compare_exchange_strong(s, n) );
+  VERIFY( as.load() == n );      // members are set correctly
+  VERIFY( padding_is_zero(as) ); // but padding was cleared on compare exchange
+
+  S w;
+  fill_struct(w);
+  w.c = 'c';
+  w.s = 100;
+  // Weak compare exchange can fail spuriously, so loop a few times.
+  int count = 10;
+  do
+  {
+    VERIFY( --count ); // Should not keep failing indefinitely.
+    // padding in N ignored for compare, and padding in W not stored into AS:
+  } while ( ! as.compare_exchange_weak(n, w) );
+  auto ws = as.load(); // SRA might prevent copying of padding bits here.
+  VERIFY( as.load() == w );      // members are set correctly
+  VERIFY( padding_is_zero(as) ); // but padding was cleared on compare exchange
 }