re PR c++/60272 (atomic<>::compare_exchange_weak has spurious store and can cause...

PR c++/60272

gcc/
	* builtins.c (expand_builtin_atomic_compare_exchange): Conditionalize
	on failure the store back into EXPECT.
libatomic/
	* cas_n.c (libat_compare_exchange): Conditionalize on failure
	the store back to EPTR.

From-SVN: r207973

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 12781b762e30ad34268b865bc41738d503b32af8..48edfb50aa4b27ea08a1ceda73fb90a20c1b776d 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2014-02-20  Richard Henderson  <rth@redhat.com>
+
+       PR c++/60272
+       * builtins.c (expand_builtin_atomic_compare_exchange): Conditionalize
+       on failure the store back into EXPECT.  Always make a new pseudo for
+       OLDVAL.
+
 2014-02-20  Jakub Jelinek  <jakub@redhat.com>
 
        PR target/57896

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 1be69686beef4f5439b5a61311052077a08b43ff..1df41f878852922e076dde645e75acd077343c6f 100644 (file)
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -5350,7 +5350,7 @@ static rtx
 expand_builtin_atomic_compare_exchange (enum machine_mode mode, tree exp, 
                                        rtx target)
 {
-  rtx expect, desired, mem, oldval;
+  rtx expect, desired, mem, oldval, label;
   enum memmodel success, failure;
   tree weak;
   bool is_weak;
@@ -5388,14 +5388,26 @@ expand_builtin_atomic_compare_exchange (enum machine_mode mode, tree exp,
   if (host_integerp (weak, 0) && tree_low_cst (weak, 0) != 0)
     is_weak = true;
 
-  oldval = expect;
-  if (!expand_atomic_compare_and_swap ((target == const0_rtx ? NULL : &target),
-                                      &oldval, mem, oldval, desired,
+  if (target == const0_rtx)
+    target = NULL;
+
+  /* Lest the rtl backend create a race condition with an imporoper store
+     to memory, always create a new pseudo for OLDVAL.  */
+  oldval = NULL;
+
+  if (!expand_atomic_compare_and_swap (&target, &oldval, mem, expect, desired,
                                       is_weak, success, failure))
     return NULL_RTX;
 
-  if (oldval != expect)
-    emit_move_insn (expect, oldval);
+  /* Conditionally store back to EXPECT, lest we create a race condition
+     with an improper store to memory.  */
+  /* ??? With a rearrangement of atomics at the gimple level, we can handle
+     the normal case where EXPECT is totally private, i.e. a register.  At
+     which point the store can be unconditional.  */
+  label = gen_label_rtx ();
+  emit_cmp_and_jump_insns (target, const0_rtx, NE, NULL, VOIDmode, 1, label);
+  emit_move_insn (expect, oldval);
+  emit_label (label);
 
   return target;
 }

diff --git a/libatomic/ChangeLog b/libatomic/ChangeLog
index b4492695bcf056b966d4b930dcd47c16da492dd6..de450dca4b1f99391bd12313a780aef582b67ce0 100644 (file)
--- a/libatomic/ChangeLog
+++ b/libatomic/ChangeLog
@@ -1,3 +1,9 @@
+2014-02-20  Richard Henderson  <rth@redhat.com>
+
+       PR c++/60272
+       * cas_n.c (libat_compare_exchange): Conditionalize on failure
+       the store back to EPTR.
+
 2013-10-16  Release Manager
 
        * GCC 4.8.2 released.

diff --git a/libatomic/cas_n.c b/libatomic/cas_n.c
index a4702683899e68992ca5a23be5179314bc4f81b8..fa5c0ada8c974f838572c5b05a9643a2a8d6143e 100644 (file)
--- a/libatomic/cas_n.c
+++ b/libatomic/cas_n.c
@@ -51,10 +51,9 @@ SIZE(libat_compare_exchange) (UTYPE *mptr, UTYPE *eptr, UTYPE newval,
 #if !DONE && N <= WORDSIZE && defined(atomic_compare_exchange_w)
 bool
 SIZE(libat_compare_exchange) (UTYPE *mptr, UTYPE *eptr, UTYPE newval,
-                             int smodel, int fmodel UNUSED)
+                             int smodel, int fmodel)
 {
   UWORD mask, shift, weval, woldval, wnewval, t, *wptr;
-  bool ret = false;
 
   pre_barrier (smodel);
 
@@ -82,12 +81,13 @@ SIZE(libat_compare_exchange) (UTYPE *mptr, UTYPE *eptr, UTYPE newval,
     }
   while (!atomic_compare_exchange_w (wptr, &woldval, t, true,
                                     __ATOMIC_RELAXED, __ATOMIC_RELAXED));
-  ret = true;
+  post_barrier (smodel);
+  return true;
+
  failure:
   *eptr = woldval >> shift;
-
-  post_barrier (smodel);
-  return ret;
+  post_barrier (fmodel);
+  return false;
 }
 
 #define DONE 1
@@ -102,18 +102,17 @@ SIZE(libat_compare_exchange) (UTYPE *mptr, UTYPE *eptr, UTYPE newval,
 {
   UTYPE oldval;
   UWORD magic;
-  bool ret = false;
+  bool ret;
 
   pre_seq_barrier (smodel);
   magic = protect_start (mptr);
 
   oldval = *mptr;
-  if (oldval == *eptr)
-    {
-      *mptr = newval;
-      ret = true;
-    }
-  *eptr = oldval;
+  ret = (oldval == *eptr);
+  if (ret)
+    *mptr = newval;
+  else
+    *eptr = oldval;
 
   protect_end (mptr, magic);
   post_seq_barrier (smodel);