[Bug c++/60272] atomic<>::compare_exchange_weak has spurious store and can cause race conditions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60272 --- Comment #10 from Max Wittal --- Sorry, I'm pretty sure now that there is some hard to find bug in my code, like the ABA problem. Please disregard my comment above.
[Bug c++/60272] atomic<>::compare_exchange_weak has spurious store and can cause race conditions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60272 --- Comment #9 from Max Wittal --- I still get this bug in gcc version 5.2.1 20151010 on x86_64-linux-gnu. Please see attached code.
[Bug c++/60272] atomic<>::compare_exchange_weak has spurious store and can cause race conditions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60272 Max Wittal changed: What|Removed |Added CC||max.wittal at mwittal dot de --- Comment #8 from Max Wittal --- Created attachment 38642 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=38642=edit Example code that still triggers this bug.
[Bug c++/60272] atomic::compare_exchange_weak has spurious store and can cause race conditions
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60272 --- Comment #4 from Richard Henderson rth at gcc dot gnu.org --- Author: rth Date: Thu Feb 20 17:43:53 2014 New Revision: 207966 URL: http://gcc.gnu.org/viewcvs?rev=207966root=gccview=rev Log: 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. Modified: trunk/gcc/ChangeLog trunk/gcc/builtins.c trunk/libatomic/ChangeLog trunk/libatomic/cas_n.c
[Bug c++/60272] atomic::compare_exchange_weak has spurious store and can cause race conditions
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60272 --- Comment #5 from Richard Henderson rth at gcc dot gnu.org --- Author: rth Date: Fri Feb 21 00:11:43 2014 New Revision: 207972 URL: http://gcc.gnu.org/viewcvs?rev=207972root=gccview=rev Log: PR c++/60272 * builtins.c (expand_builtin_atomic_compare_exchange): Always make a new pseudo for OLDVAL. Modified: trunk/gcc/ChangeLog trunk/gcc/builtins.c
[Bug c++/60272] atomic::compare_exchange_weak has spurious store and can cause race conditions
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60272 --- Comment #6 from Richard Henderson rth at gcc dot gnu.org --- Author: rth Date: Fri Feb 21 00:14:05 2014 New Revision: 207973 URL: http://gcc.gnu.org/viewcvs?rev=207973root=gccview=rev Log: 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. Modified: branches/gcc-4_8-branch/gcc/ChangeLog branches/gcc-4_8-branch/gcc/builtins.c branches/gcc-4_8-branch/libatomic/ChangeLog branches/gcc-4_8-branch/libatomic/cas_n.c
[Bug c++/60272] atomic::compare_exchange_weak has spurious store and can cause race conditions
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60272 Richard Henderson rth at gcc dot gnu.org changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED Target Milestone|--- |4.8.3 --- Comment #7 from Richard Henderson rth at gcc dot gnu.org --- Fixed.
[Bug c++/60272] atomic::compare_exchange_weak has spurious store and can cause race conditions
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60272 Jonathan Wakely redi at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2014-02-19 Ever confirmed|0 |1
[Bug c++/60272] atomic::compare_exchange_weak has spurious store and can cause race conditions
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60272
Jakub Jelinek jakub at gcc dot gnu.org changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org,
||rth at gcc dot gnu.org,
||torvald at gcc dot gnu.org
--- Comment #1 from Jakub Jelinek jakub at gcc dot gnu.org ---
Even our __atomic_compare_exchange* documentation states that:
If they are not equal, the current contents of
@code{*@var{ptr}} is written into @code{*@var{expected}}.
But then expand_builtin_atomic_compare_exchange doesn't care:
oldval = expect;
if (!expand_atomic_compare_and_swap ((target == const0_rtx ? NULL : target),
oldval, mem, oldval, desired,
is_weak, success, failure))
return NULL_RTX;
if (oldval != expect)
emit_move_insn (expect, oldval);
That effectively means that expect will be stored unconditionally.
So, either we'd need to change this function, so that it sets oldval to
NULL_RTX
first, and passes ..., oldval, mem, expected, ... and needs to also always ask
for target, then conditionally on target store to expected, or perhaps add
extra parameter to expand_atomic_compare_and_swap and do the store only
conditionally in that case. Richard/Torvald?
[Bug c++/60272] atomic::compare_exchange_weak has spurious store and can cause race conditions
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60272 --- Comment #2 from torvald at gcc dot gnu.org --- (In reply to Jakub Jelinek from comment #1) So, either we'd need to change this function, so that it sets oldval to NULL_RTX first, and passes ..., oldval, mem, expected, ... and needs to also always ask for target, then conditionally on target store to expected, or perhaps add extra parameter to expand_atomic_compare_and_swap and do the store only conditionally in that case. Richard/Torvald? I'm not sure what's better. But getting this fixed in 4.9.0 would be good! :)
[Bug c++/60272] atomic::compare_exchange_weak has spurious store and can cause race conditions
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60272 Richard Henderson rth at gcc dot gnu.org changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |rth at gcc dot gnu.org --- Comment #3 from Richard Henderson rth at gcc dot gnu.org --- Agreed. Mine.
