[Bug c++/65147] New: alignment of std::atomic object is not correct

[Bug c++/65147] New: alignment of std::atomic object is not correct

[alexey.lapshin at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

            Bug ID: 65147
           Summary: alignment of std::atomic object is not correct
           Product: gcc
           Version: 4.9.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: alexey.lapshin at oracle dot com

According to the documentation -
https://gcc.gnu.org/wiki/Atomic/GCCMM/UnalignedPolicy alignment of atomic
object should match it`s size.
Alignment in the test case below does not match with documentation.

~/atomic_test$ cat unaligned_atomic.cpp

#include <atomic>
#include <stdio.h>

typedef struct {
    char c [8];
} power_of_two_obj;

typedef struct {
   char c[1];
   std::atomic<power_of_two_obj> ao;
} container_struct; 

int main ( void ) {

    std::atomic<power_of_two_obj> obj1;
    container_struct              obj2; 


    printf("\n Size and Alignment of std::atomic  object "); 
    printf(" : sizeof(obj1) %d alignof(obj1) %d ", sizeof(obj1), alignof(obj1)
);

    printf("\n Size and Alignment of std::atomic member object "); 
    printf(" : sizeof(obj2.ao) %d alignof(obj2.ao) %d \n", sizeof(obj2.ao),
alignof(obj2.ao) );

    return 0;
}

~/atomic_test$ g++ -v
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/opt/gcc/libexec/gcc/i386-pc-solaris2.11/4.9.2/lto-wrapper
Target: i386-pc-solaris2.11
Configured with: ./configure --prefix=/opt/gcc/
Thread model: posix
gcc version 4.9.2 (GCC) 

~/atomic_test$ g++ -O -latomic -std=c++11 unaligned_atomic.cpp -m32

~/atomic_test$ ./a.out

 Size and Alignment of std::atomic  object  : sizeof(obj1) 8 alignof(obj1) 1 
 Size and Alignment of std::atomic member object  : sizeof(obj2.ao) 8
alignof(obj2.ao) 1 

Alignment should be 8-bytes in above test case.

This behavior also differs from gcc. Gcc aligns 8-bytes objects to 8-bytes.

The bug was found on Solarix x86 -m32, but it could be on other platforms -
SPARC, Linux, -m64.

There is another problem with similar test case. If size of object is not power
of two then size of corresponding atomic object is also not power of two.
According to  https://gcc.gnu.org/wiki/Atomic/GCCMM/UnalignedPolicy the size
should be upsized in such case.

[Bug c++/65147] alignment of std::atomic object is not correct

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

--- Comment #1 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
On Fri, 20 Feb 2015, alexey.lapshin at oracle dot com wrote:

> According to the documentation -
> https://gcc.gnu.org/wiki/Atomic/GCCMM/UnalignedPolicy alignment of atomic
> object should match it`s size.
> Alignment in the test case below does not match with documentation.

Not in itself a bug to fail to follow preliminary plans.  *But*:

> This behavior also differs from gcc. Gcc aligns 8-bytes objects to 8-bytes.

Differences between C and C++ atomics might be a bug - there may be intent 
for them to be ABI-compatible (although that didn't get implemented), 
unlike other aspects of the early plans you point to where intent changed 
over the course of implementation.

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ABI
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-03-20
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |jwakely.gcc at gmail dot com
     Ever confirmed|0                           |1

--- Comment #2 from Jason Merrill <jason at gcc dot gnu.org> ---
This does seem like a bug.

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[alexey.lapshin at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

--- Comment #3 from Alexey Lapshin <alexey.lapshin at oracle dot com> ---
(In reply to Jason Merrill from comment #2)
> This does seem like a bug.

What is a proper behavior for G++ in this case ?

should it always align std::atomic object of size 8 at 8 bytes ?

Or should G++ just never inline implementation for atomic routine ?

(see bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65149 when implementation
of atomic routine was inlined for incorrectly aligned atomic object, which
leads to BusError)

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I think std::atomic<T> should increase the alignment of its T member. That will
have the advantage of being layout-compatible with _Atomic T.

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
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

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |5.0

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for gcc5.

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[alexey.lapshin at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

--- Comment #7 from Alexey Lapshin <alexey.lapshin at oracle dot com> ---
(In reply to Jonathan Wakely from comment #5)
> 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


It looks like this fix makes alignment of atomic object to be the same as
alignment of integral non-atomic object of the same size.

The gcc behavior is different it makes alignment of atomic objects of sizes
1,2,4,8,16 to match with size :

G++ :
$ cat all.cc

#include <atomic>
#include <stdio.h>

typedef struct {
   char c[16];
} S16;

int main ( void ) {
   std::atomic<char>      ac;
   std::atomic<short>     as;
   std::atomic<long>      al;
   std::atomic<long long> all;
   std::atomic<S16>       a16;

   printf("\n sizeof(ac) %d alignof(ac) %d",  sizeof(ac), alignof(ac) );
   printf("\n sizeof(as) %d alignof(as) %d",  sizeof(as), alignof(as) );
   printf("\n sizeof(al) %d alignof(al) %d",  sizeof(al), alignof(al) );
   printf("\n sizeof(all) %d alignof(all) %d",  sizeof(all), alignof(all) );
   printf("\n sizeof(a16) %d alignof(a16) %d",  sizeof(a16), alignof(a16) );
   printf("\n");
}
$g++ -latomic -std=c++11 -m32 all.cc 
$./a.out

 sizeof(ac) 1 alignof(ac) 1
 sizeof(as) 2 alignof(as) 2
 sizeof(al) 4 alignof(al) 4
 sizeof(all) 8 alignof(all) 4
 sizeof(a16) 16 alignof(a16) 1


gcc : 

$ cat all.c

#include <stdatomic.h>
#include <stdio.h>

typedef struct {
   char c[16];
} S16;

int main ( void ) {
   _Atomic char      ac;
   _Atomic short     as;
   _Atomic long      al;
   _Atomic long long all;
   _Atomic S16       a16;

   printf("\n sizeof(ac) %d alignof(ac) %d",  sizeof(ac), __alignof__(ac) );
   printf("\n sizeof(as) %d alignof(as) %d",  sizeof(as), __alignof__(as) );
   printf("\n sizeof(al) %d alignof(al) %d",  sizeof(al), __alignof__(al) );
   printf("\n sizeof(all) %d alignof(all) %d",  sizeof(all), __alignof__(all)
);
   printf("\n sizeof(a16) %d alignof(a16) %d",  sizeof(a16), __alignof__(a16)
);

   printf("\n");
}
$ gcc -latomic -std=c11 -m32 all.c 
$ ./a.out

 sizeof(ac) 1 alignof(ac) 1
 sizeof(as) 2 alignof(as) 2
 sizeof(al) 4 alignof(al) 4
 sizeof(all) 8 alignof(all) 8
 sizeof(a16) 16 alignof(a16) 16
avl@ficus:~/atomic_test$ 

Note 8-bytes and 16-bytes objects aligned at their size at -m32.
>From gcc-bugs-return-482046-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Mar 27 15:47:57 2015
Return-Path: <gcc-bugs-return-482046-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 86775 invoked by alias); 27 Mar 2015 15:47:56 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 86702 invoked by uid 48); 27 Mar 2015 15:47:53 -0000
From: "jvdelisle at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug libfortran/65605] [4.9 Regression] Namelist mishandles delim
Date: Fri, 27 Mar 2015 16:02:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: libfortran
X-Bugzilla-Version: 4.9.3
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: jvdelisle at gcc dot gnu.org
X-Bugzilla-Status: RESOLVED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: jvdelisle at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_status resolution
Message-ID: <bug-65605-4-XSJ7rM2Dvf@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-65605-4@http.gcc.gnu.org/bugzilla/>
References: <bug-65605-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-03/txt/msg03190.txt.bz2
Content-length: 537

https://gcc.gnu.org/bugzilla/show_bug.cgi?ide605

Jerry DeLisle <jvdelisle at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |DUPLICATE

--- Comment #1 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Duplicate. I did not see Dominique captured it.

*** This bug has been marked as a duplicate of bug 65596 ***

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Alexey Lapshin from comment #7)
> It looks like this fix makes alignment of atomic object to be the same as
> alignment of integral non-atomic object of the same size.

That was the intention, yes. Because I understood that to be what _Atomic does.

> The gcc behavior is different it makes alignment of atomic objects of sizes
> 1,2,4,8,16 to match with size :

Oh.

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I don't have a fix for this yet, so let's re-open it ...

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Alexey Lapshin from comment #7)
> It looks like this fix makes alignment of atomic object to be the same as
> alignment of integral non-atomic object of the same size.

Actually it only did that for non-integral atomic objects, e.g. I didn't do
anything to change std::atomic<long long>.

> The gcc behavior is different it makes alignment of atomic objects of sizes
> 1,2,4,8,16 to match with size :

That's not strictly true, there is a target hook (atomic_align_for_mode) which
specifies the alignment for 1/2/4/8/16-byte objects, and the result is not
necessarily the same as the size. Or so I'm told. That's why I used the nested
conditional expressions with alignof(integral type) instead of just using
alignas(sizeof(T)).

> $g++ -latomic -std=c++11 -m32 all.cc 
> $./a.out
> 
>  sizeof(ac) 1 alignof(ac) 1
>  sizeof(as) 2 alignof(as) 2
>  sizeof(al) 4 alignof(al) 4
>  sizeof(all) 8 alignof(all) 4
>  sizeof(a16) 16 alignof(a16) 1

There are two problems here.

The first is that alignof(std::atomic<long long>) is less than alignof(long
long), and my recent changes didn't address that. That is easy to fix:

--- a/libstdc++-v3/include/bits/atomic_base.h
+++ b/libstdc++-v3/include/bits/atomic_base.h
@@ -235,7 +235,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // 8 bytes, since that is what GCC built-in functions for atomic
   // memory access expect.
   template<typename _ITp>
-    struct __atomic_base
+    struct alignas(_ITp) __atomic_base
     {
     private:
       typedef _ITp     __int_type;


The second problem is that alignof(struct S16) is not increased. That's because
libstdc++ doesn't support __int128 on x86, so this bit of code doesn't do
anything:

#ifdef _GLIBCXX_USE_INT128
    : sizeof(_Tp) == sizeof(__int128)  ? alignof(__int128)
#endif

I'm not sure how to fix this. Maybe we should just bodge it like this and hope
it is valid for all important targets:

#ifdef _GLIBCXX_USE_INT128
    : sizeof(_Tp) == sizeof(__int128)  ? alignof(__int128)
#else
    : sizeof(_Tp) == 16  ? 16
#endif

(The real solution is a new attribute that uses the target hook, so we can
guarantee the same result as the C front end, but it's too late to do that for
GCC 5).

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Author: redi
Date: Thu Apr  9 11:15:44 2015
New Revision: 221945

URL: https://gcc.gnu.org/viewcvs?rev=221945&root=gcc&view=rev
Log:
2015-04-09  Jonathan Wakely  <jwakely@redhat.com>
        Richard Henderson  <rth@redhat.com>

    PR libstdc++/65147
    * include/bits/atomic_base.h (__atomic_base<_ITp>): Increase
    alignment.
    * include/std/atomic (atomic): For types with a power of two size set
    alignment to at least the size.
    * testsuite/29_atomics/atomic/60695.cc: Adjust dg-error line number.
    * testsuite/29_atomics/atomic/65147.cc: New.
    * testsuite/29_atomics/atomic_integral/65147.cc: New.

Added:
    trunk/libstdc++-v3/testsuite/29_atomics/atomic/65147.cc
      - copied, changed from r221943,
trunk/libstdc++-v3/testsuite/29_atomics/atomic/60695.cc
    trunk/libstdc++-v3/testsuite/29_atomics/atomic_integral/65147.cc
      - copied, changed from r221943,
trunk/libstdc++-v3/testsuite/29_atomics/atomic/60695.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/bits/atomic_base.h
    trunk/libstdc++-v3/include/std/atomic
    trunk/libstdc++-v3/testsuite/29_atomics/atomic/60695.cc

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Alexey, your testcases in comment 0 and comment 7 give the right results now.

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[alexey.lapshin at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

--- Comment #13 from Alexey Lapshin <alexey.lapshin at oracle dot com> ---
(In reply to Jonathan Wakely from comment #12)
> Alexey, your testcases in comment 0 and comment 7 give the right results now.

Thank You!

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.0                         |5.2

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 5.1 has been released.

[Bug libstdc++/65147] alignment of std::atomic object is not correct

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65147

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|5.2                         |5.0

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> ---
This was fixed.