Bug 69560 - x86_64: alignof(uint64_t) produces incorrect results with -m32
Summary: x86_64: alignof(uint64_t) produces incorrect results with -m32
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 5.3.1
: P3 normal
Target Milestone: 8.0
Assignee: Jason Merrill
URL:
Keywords:
: 69763 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-01-29 18:09 UTC by David Merillat
Modified: 2024-06-28 19:23 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-04-11 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Merillat 2016-01-29 18:09:23 UTC
Sample Program (test.cpp):

#######################################################################

#include <cstdint>
#include <cstdio>
#include <cstddef>

using namespace std;

typedef struct {
        int32_t i32;
        int64_t i64;
} DefaultStruct;

typedef struct {
        int32_t i32;
        int64_t i64 __attribute__((aligned(8)));
} AlignedStruct;

int main(void)
{
        DefaultStruct s;
        printf( "DefaultStruct: offset=%d, struct align=%d, member align=%d, type align=%d\n", 
                offsetof(DefaultStruct,i64), alignof(s), alignof(s.i64), alignof(int64_t) );


        AlignedStruct s2;
        printf( "AlignedStruct: offset=%d, struct align=%d, member align=%d, type align=%d\n",
                offsetof(AlignedStruct,i64), alignof(s2), alignof(s2.i64), alignof(int64_t) );

        return 0;
}


#####################################################

Compile command line:

g++ -m32 --std=c++14 test.cpp -o test

#####################################################

Expected program output:

DefaultStruct: offset=4, struct align=4, member align=4, type align=4
AlignedStruct: offset=8, struct align=8, member align=8, type align=8

#####################################################

Actual program output:

DefaultStruct: offset=4, struct align=4, member align=4, type align=8
AlignedStruct: offset=8, struct align=8, member align=8, type align=8

#####################################################

Comments:

Note that the difference is alignof(uint64_t) returns 8, while the correct value appears to be 4.

I checked the ISO C++ spec, section 3.11, and it says "The alignment requirement of a complete type can be queried using an alignof expression", so if alignof(int64_t) yields 8, then it shouldn't be legal to place an int64_t at offset 4 in a structure.

Disclaimer:  I checked the draft specification.  I don't have $212 lying around to purchase the final spec.

This is a possible duplicate with 61053 or 61447, however 61053 doesn't deal with the output of alignof(), and 61447 has been suspended because the standards were ambiguous.  Here they seem more clear-cut.
Comment 1 Jonathan Wakely 2016-01-29 18:13:45 UTC
(In reply to David Merillat from comment #0)
> I checked the ISO C++ spec, section 3.11, and it says "The alignment
> requirement of a complete type can be queried using an alignof expression",
> so if alignof(int64_t) yields 8, then it shouldn't be legal to place an
> int64_t at offset 4 in a structure.

That's not true. Some types have weaker alignment requirements when they are structure members.
Comment 2 Jonathan Wakely 2016-01-29 18:16:49 UTC
See ADJUST_FIELD_ALIGN at https://gcc.gnu.org/onlinedocs/gccint/Storage-Layout.html#Storage-Layout

unsigned long long on x86 is such a type.
Comment 3 jsm-csl@polyomino.org.uk 2016-01-29 21:08:11 UTC
This is where C distinguishes C11 _Alignof (the minimum alignment the type 
is guaranteed to have in all contexts, so 4, see min_align_of_type) from 
GNU C __alignof (the normal alignment of the type, so 8).  I don't know if 
a similar distinction is needed for C++; that depends on how the standard 
C++ alignof is specified.
Comment 4 David Merillat 2016-01-29 23:39:34 UTC
(In reply to Jonathan Wakely from comment #2)
> See ADJUST_FIELD_ALIGN at
> https://gcc.gnu.org/onlinedocs/gccint/Storage-Layout.html#Storage-Layout
> 
> unsigned long long on x86 is such a type.

Thank you for the pointers into GCC internals, of which I was unaware.  However, the C++14 specification makes no mention of weaker alignment requirements for structure members that I could find.

In fact, the specification for alignof() in C++14 is remarkably similar to that of _Alignof in C11 (I would assume one is derived from the other.  _Alignof yields what I believe to be the correct answer in this case:

######################################################## test2.c

#include <stdio.h>
#include <stdalign.h>
#include <stdint.h>

int main(void)
{
        printf( "_Alignof(int64_t) = %d\n", _Alignof(int64_t) );
        return 0;
}

##########################################################

gcc -std=C11 test2.c -o test2 -m32

##########################################################

_Alignof(int64_t) = 4

##########################################################
Comment 5 Andrew Pinski 2016-01-30 00:06:14 UTC
Dup of bug 10360 (This has been discussed many times).

*** This bug has been marked as a duplicate of bug 10360 ***
Comment 6 Andrew Pinski 2016-01-30 00:09:04 UTC
Actually bug 52023 is a better reason for the difference between __alignof__ and _Alignof.

*** This bug has been marked as a duplicate of bug 52023 ***
Comment 7 Jonathan Wakely 2016-01-30 00:10:02 UTC
(In reply to David Merillat from comment #0)
>         DefaultStruct s;
>         printf( "DefaultStruct: offset=%d, struct align=%d, member align=%d,
> type align=%d\n", 
>                 offsetof(DefaultStruct,i64), alignof(s), alignof(s.i64),
> alignof(int64_t) );

N.B. this isn't valid C++ or C, because the operand of alignof must be a type, not an object.

So standard C++ doesn't even let you ask for alignof(s.i64), it only lets you ask for the alignment of uint64_t, which for a not-sub-object is 8.
Comment 8 David Merillat 2016-01-30 01:20:12 UTC
(In reply to Andrew Pinski from comment #6)
> Actually bug 52023 is a better reason for the difference between __alignof__
> and _Alignof.
> 
> *** This bug has been marked as a duplicate of bug 52023 ***

Yes, that bug dealt with the issue more thoroughly, dealing with the issues I am looking at.  I wish I had found it earlier.  

One thing remains.  As a result of 52023, C11 _Alignof was "fixed" to return the smallest possible alignment, but C++11 was left alone "deliberately", although "C++11 will need examining to determine what is right for alignof there".  I didn't see any such discussion for C++11 or C++14 alignof(), and by my reading of the specifications, they are defined almost identically to C11 _Alignof.

So I think the central issue in this bug remains open.  I'll leave it to someone else whether to reopen the case.
Comment 9 Andrew Pinski 2016-01-30 01:29:14 UTC
Reopening as I did not realize this was about C++11's alignof.  I had thought it was about GCC's __alignof extension; I should look closer :).
Comment 10 Jonathan Wakely 2016-01-30 12:31:39 UTC
(In reply to David Merillat from comment #8)
> One thing remains.  As a result of 52023, C11 _Alignof was "fixed" to return
> the smallest possible alignment, but C++11 was left alone "deliberately",
> although "C++11 will need examining to determine what is right for alignof
> there".  I didn't see any such discussion for C++11 or C++14 alignof(), and
> by my reading of the specifications, they are defined almost identically to
> C11 _Alignof.

See Joseph's explanation at:
https://gcc.gnu.org/ml/gcc-patches/2013-12/msg00435.html
Comment 11 Jonathan Wakely 2016-01-30 12:38:34 UTC
(In reply to Jonathan Wakely from comment #10)
> (In reply to David Merillat from comment #8)
> > One thing remains.  As a result of 52023, C11 _Alignof was "fixed" to return
> > the smallest possible alignment, but C++11 was left alone "deliberately",
> > although "C++11 will need examining to determine what is right for alignof
> > there".  I didn't see any such discussion for C++11 or C++14 alignof(), and
> > by my reading of the specifications, they are defined almost identically to
> > C11 _Alignof.
> 
> See Joseph's explanation at:
> https://gcc.gnu.org/ml/gcc-patches/2013-12/msg00435.html

Which refers to 3.11 [basic.align] p2:

   The alignment required for a type might be different when it is used as the
   type of a complete object and when it is used as the type of a subobject.
Comment 12 Mike Hommey 2016-07-20 06:01:09 UTC
(In reply to Jonathan Wakely from comment #11)
> > See Joseph's explanation at:
> > https://gcc.gnu.org/ml/gcc-patches/2013-12/msg00435.html
> 
> Which refers to 3.11 [basic.align] p2:
> 
>    The alignment required for a type might be different when it is used as
> the
>    type of a complete object and when it is used as the type of a subobject.

That doesn't seem to explain why the following is not true when compiling with -m32:

struct foo {
  char c;
  long long l;
};

static_assert(alignof(long long) == alignof(foo), "");
Comment 13 Andreas Schwab 2016-07-20 07:21:50 UTC
alignof(long long) "type of a complete object"
alignof(foo) "type of a subobject"
Comment 14 Mike Hommey 2016-07-20 07:51:55 UTC
(In reply to Andreas Schwab from comment #13)
> alignof(long long) "type of a complete object"
> alignof(foo) "type of a subobject"

But that doesn't tell why the alignment of a long long is 8 as a complete object, but 4 as a subobject (the offset of l in foo is 4, not 8).
Comment 15 Andreas Schwab 2016-07-20 08:03:58 UTC
Because the ABI says so.
Comment 16 Jason Merrill 2018-04-11 01:58:40 UTC
(In reply to Andreas Schwab from comment #15)
> Because the ABI says so.

Which ABI?  In https://www.uclibc.org/docs/psABI-i386.pdf I see

long long: sizeof 8, alignment 4.

That GCC chooses to give stronger alignment to long long variables isn't part of the ABI, and shouldn't affect the value of alignof.
Comment 17 Jason Merrill 2018-04-11 02:13:50 UTC
*** Bug 69763 has been marked as a duplicate of this bug. ***
Comment 18 H.J. Lu 2018-04-11 03:36:25 UTC
(In reply to Jason Merrill from comment #16)
> (In reply to Andreas Schwab from comment #15)
> > Because the ABI says so.
> 
> Which ABI?  In https://www.uclibc.org/docs/psABI-i386.pdf I see
> 
> long long: sizeof 8, alignment 4.
> 
> That GCC chooses to give stronger alignment to long long variables isn't
> part of the ABI, and shouldn't affect the value of alignof.

FYI, x86 psABIs are at

https://github.com/hjl-tools/x86-psABI/wiki/X86-psABI
Comment 19 Jason Merrill 2018-04-11 12:37:30 UTC
(In reply to H.J. Lu from comment #18)
> (In reply to Jason Merrill from comment #16)
> > (In reply to Andreas Schwab from comment #15)
> > > Because the ABI says so.
> > 
> > Which ABI?  In https://www.uclibc.org/docs/psABI-i386.pdf I see
> > 
> > long long: sizeof 8, alignment 4.
> > 
> > That GCC chooses to give stronger alignment to long long variables isn't
> > part of the ABI, and shouldn't affect the value of alignof.
> 
> FYI, x86 psABIs are at
> 
> https://github.com/hjl-tools/x86-psABI/wiki/X86-psABI

And those also say that double and long long have alignment 4.
Comment 20 jsm-csl@polyomino.org.uk 2018-04-19 20:08:32 UTC
I consider it part of the ABI that long long 
__attribute__((__aligned__(__alignof__(long long)))), as in the definition 
of max_align_t (and similarly in gnulib's max_align_t definition, for 
example), is 8-byte-aligned on x86.
Comment 21 Jason Merrill 2018-04-23 20:50:10 UTC
Author: jason
Date: Mon Apr 23 20:49:38 2018
New Revision: 259578

URL: https://gcc.gnu.org/viewcvs?rev=259578&root=gcc&view=rev
Log:
	PR c++/69560 - wrong alignof(double) on x86.

	CWG 1879 - Inadequate definition of alignment requirement.
	* cp-tree.h (ALIGNOF_EXPR_STD_P): New.
	* typeck.c (cxx_sizeof_or_alignof_type): Add std_alignof parm.
	(cxx_sizeof_expr, cxx_sizeof_nowarn, cxx_alignas_expr)
	(cxx_alignof_expr): Pass it.
	* parser.c (cp_parser_unary_expression): Pass it.
	* pt.c (tsubst_copy): Copy it.
	(tsubst_copy_and_build): Pass it.
	* decl.c (fold_sizeof_expr): Pass it.

Added:
    trunk/gcc/testsuite/g++.dg/abi/align2.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/decl.c
    trunk/gcc/cp/parser.c
    trunk/gcc/cp/pt.c
    trunk/gcc/cp/typeck.c
    trunk/libcc1/libcp1plugin.cc
Comment 22 Jason Merrill 2018-04-24 20:34:30 UTC
Fixed for GCC 8.
Comment 23 Peter Cordes 2018-04-26 22:54:22 UTC
Just to recap the current situation (gcc/g++ 8.0.1 20180425):

I ported David Marillat's testcase to work as C or C++ https://godbolt.org/g/QdG2V6.  (And changed it to set global variables instead of calling printf, so you can see the results from looking at the asm output instead of running it).

C++11 alignof() now agrees with C11 alignof() (which didn't change) that alignof(int64_t) is 4 when targeting the i386 System V ABI.

Previously G++'s alignof() reported 8, while gcc's C11 alignof (stdalign.h) reported 4.  That was the only change: struct-member alignof results are unchanged, and already matched between C11 and C++11.


4 is the minimum alignment that *any* int64_t, or pointer to int64_t, is assumed to have when generating code for i386 SysV.  gcc / g++ are allowed to generate code that breaks if passed a pointer to int64_t that wasn't 4-byte aligned.  (Auto-vectorization is one case where that can happen on x86: https://stackoverflow.com/q/47510783/224132).

They're *not* allowed to assume that it's 8-byte aligned unless they can see the definition and know that a particular int64_t object is over-aligned, e.g. to its natural alignment of 8, like gcc chooses to do whenever possible (i.e. outside structs).

So in both C++ and C (and in g++/gcc after this patch), alignof(int64_t) is the minimum that any allocator must give an int64_t for correctness (in this funky 32-bit ABI), not the recommended alignment that gcc and g++ both already used whenever ABI struct-packing rules didn't constrain them.

It's also the guaranteed minimum that code can *assume*.  e.g. a manually-vectorized library function might check alignof(T) == sizeof(T) before assuming that using 16-byte aligned loads/stores can line up with element boundaries.  (An array inside a struct { int foo; int64_t arr[10]; } would violate this for i386 SysV).

Anyway, I think use-cases like these are why the standard is worded the way it is, and why it makes sense for alignof() to report the guaranteed/required minimum.  The recommended or actual alignment is useful, too, though, for other cases, so it's nice that GNU __alignof() is also available to report that.

----

Semi-related: gcc depends on 8-byte alignment for C11 _Atomic int64_t but still fails to provide it inside structs on the i386 SysV ABI (Bug 65146), using the same alignment rules as regular int64_t.

C++11 std::atomic<int64_t> is fine, getting the required natural alignment even on i386 SysV so SSE2 movq is atomic and lock add is efficient.

This change to what alignof() reports in C++ had no effect on C at all, or on any alignment choices made by the compiler in either C or C++.  I only mention it as another interesting case where i386 SysV's under-alignment of 64-bit types requiring special care, but that one will require an ABI change of some sort to fix.
Comment 24 James Y Knight 2018-11-20 18:28:22 UTC
Note that I've filed PR88115 and PR88119 for some bugs caused by this change.
Comment 25 GCC Commits 2024-06-28 19:23:16 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:ac8c61b62e71ffdcaebfd4cfc03f58fe542855dd

commit r15-1713-gac8c61b62e71ffdcaebfd4cfc03f58fe542855dd
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jun 26 12:40:51 2024 +0100

    libstdc++: Simplify <ext/aligned_buffer.h> class templates
    
    As noted in a comment, the __gnu_cxx::__aligned_membuf class template
    can be simplified, because alignof(T) and alignas(T) use the correct
    alignment for a data member. That's true since GCC 8 and Clang 8. The
    EDG front end (as used by Intel icc, aka "Intel C++ Compiler Classic")
    does not implement the PR c++/69560 change, so keep using the old
    implementation when __EDG__ is defined, to avoid an ABI change for icc.
    
    For __gnu_cxx::__aligned_buffer<T> all supported compilers agree on the
    value of __alignof__(T), but we can still simplify it by removing the
    dependency on std::aligned_storage<sizeof(T), __alignof__(T)>.
    
    Add a test that checks that the aligned buffer types have the expected
    alignment, so that we can tell if changes like this affect their ABI
    properties.
    
    libstdc++-v3/ChangeLog:
    
            * include/ext/aligned_buffer.h (__aligned_membuf): Use
            alignas(T) directly instead of defining a struct and using 9its
            alignment.
            (__aligned_buffer): Remove use of std::aligned_storage.
            * testsuite/abi/aligned_buffers.cc: New test.