Created attachment 33396 [details] small reproducer The attached short program results in a bus error on powerpc64 top of trunk at -O0, but I believe is a bug that would be exposed on many targets, going back at least to 4.9. It succeeds--probably by luck--on 4.8. The key is that the atomic struct is eight-bytes in size, but only four byte aligned, and gcc takes no care to subsequently align it. GCC chooses an ldarx instruction, which requires eight-byte alignment. Although https://gcc.gnu.org/wiki/Atomic/GCCMM/UnalignedPolicy doesn't directly address this case, a careful reading leads me to believe that this is intended to work. My uneducated guess is that the template at <atomic>:189 should either use &_M_i in calls to __atomic_is_lock_free (instead of nullptr) or should add alignment as necessary. Not sure how that is intended to be done. If I fix <atomic> to pass the pointer, then gcc chooses to call out to an atomic library function, which gcc doesn't provide. google ref b/17136920 g++ -std=c++11 t.cc -O0 #include <atomic> struct twoints { int a; int b; }; int main() { // unalign subsequent variables char b0 = 'a'; twoints one = { 1 }; twoints two = { 2 }; std::atomic<twoints> a(one); twoints e = { 0 }; a.compare_exchange_strong(e, two, std::memory_order_acq_rel); return 0; }
Indeed, when running a simple test program: #include <atomic> #include <stdio.h> struct twoints { int a; int b; }; int main(void) { printf("%d\n", __alignof__ (twoints)); printf("%d\n", __alignof__ (std::atomic<twoints>)); return 0; } we see that the GCC only requires 4 bytes of alignment for the atomic type. However, with the equivalent C11 code using the _Atomic keyword #include <stdatomic.h> #include <stdio.h> struct twoints { int a; int b; }; int main() { printf("%d\n", __alignof__ (struct twoints)); printf("%d\n", __alignof__ (_Atomic (struct twoints))); return 0; } we get an alignment requirement of 8 bytes for the atomic type. In the C case, this is done by the compiler front-end where it implements the _Atomic keyword. In the C++ case, it seems the compiler doesn't really get involved, as it's all done in plain C++ in standard library code ... I suspect the intent was that for C++, we likewise ought to have an increased alignment requirement for the type, but I'm not sure how to implement this in the library. Need some of the library experts to comment here.
Confirmed.
(In reply to saugustine from comment #0) > My uneducated guess is that the template at <atomic>:189 should either use > &_M_i in calls to __atomic_is_lock_free (instead of nullptr) or should add > alignment as necessary. Not sure how that is intended to be done. If I fix > <atomic> to pass the pointer, then gcc chooses to call out to an atomic > library function, which gcc doesn't provide. GCC does provide it, in libatomic, so -latomic should work. But I just tried your suggested change and saw no effect: I didn't need libatomic and I still got a bus error. I suppose what we want is the equivalent of this, but the _Atomic keyword isn't valid in C++: --- a/libstdc++-v3/include/std/atomic +++ b/libstdc++-v3/include/std/atomic @@ -161,7 +161,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION struct atomic { private: - _Tp _M_i; + alignas(alignof(_Atomic _Tp)) _Tp _M_i; // TODO: static_assert(is_trivially_copyable<_Tp>::value, "");
Yeah... up until now, CRIS was the only port that this was an issue for. The original C11 work had an extension __attribute__(atomic) which would do the same thing the _Atomic keyword does for non C11 compilation, and the type in the libstdc++ atomic classes would be given this attribute. When jsm took over the C11 integration, this attribute code added extra testing and code paths that were beyond the scope of what he was doing with C11, so it was left behind. my original mothballing note was https://gcc.gnu.org/ml/gcc/2013-09/msg00240.html we could probably track down the parts he didn't integrate from the branch if someone wanted to work with them and get them up to snuff.
*** Bug 65149 has been marked as a duplicate of this bug. ***
Author: redi Date: Thu Mar 26 19:27:02 2015 New Revision: 221703 URL: https://gcc.gnu.org/viewcvs?rev=221703&root=gcc&view=rev Log: PR libstdc++/62259 PR libstdc++/65147 * include/std/atomic (atomic<T>): Increase alignment for types with the same size as one of the integral types. * testsuite/29_atomics/atomic/60695.cc: Adjust dg-error line number. * testsuite/29_atomics/atomic/62259.cc: New. Added: trunk/libstdc++-v3/testsuite/29_atomics/atomic/62259.cc - copied, changed from r221701, trunk/libstdc++-v3/testsuite/29_atomics/atomic/60695.cc Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/include/std/atomic trunk/libstdc++-v3/testsuite/29_atomics/atomic/60695.cc
Fixed for gcc5.
(In reply to Andrew Macleod from comment #4) > Yeah... up until now, CRIS was the only port that this was an issue for. And JFTR, the resolution to this PR doesn't solve the similar issue there. (To wit, increasing alignment up to *that of an integer of* the natural size which is not the same as "increasing alignment up to the natural size" which would work.) > The original C11 work had an extension __attribute__(atomic) which would > do the same thing the _Atomic keyword does for non C11 compilation, and the > type in the libstdc++ atomic classes would be given this attribute. This would work.