[Bug c++/65945] New: ARM: unaligned access when stroing nullptr

[Bug c++/65945] New: ARM: unaligned access when stroing nullptr

[npl at chello dot at]

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

            Bug ID: 65945
           Summary: ARM: unaligned access when stroing nullptr
           Product: gcc
           Version: 4.8.4
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: npl at chello dot at
  Target Milestone: ---

Created attachment 35429
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35429&action=edit
code causing the issue

Hello,

the attachedcode produces an unaligned access for arm, on the production code I
ran into this issue with versions 5.1, 4.9.2, 4.8.4. This reduced testcase
doesnt produce the issue with 5.1, but for 4.9.2 and 4.8.4.

The issue seems to be the line
constexpr inline_type bool operator ==(bitmask X, decltype(nullptr)) \
{ return X == bitmask(); }

aparrently gcc converts stores the given parameter on the frame without caring
for alignment ("str      r3, [fp, #-25]"). changing the type to unsigned seems
to fix this

Commandline to reproduce:
arm-none-eabi-g++ -std=c++11 -O0 -c unalignedgcc.cpp

[Bug c++/65945] ARM: unaligned access when stroing nullptr

[npl at chello dot at]

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

--- Comment #1 from npl at chello dot at ---
Created attachment 35430
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35430&action=edit
disassembly showing the issue

The issue is the line
  dc:   e50b3019        str     r3, [fp, #-25]  ; 0xffffffe7

[Bug c++/65945] ARM: unaligned access when stroing nullptr

[npl at chello dot at]

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

--- Comment #2 from npl at chello dot at ---
Well, gcc 5.1 seems to reproduceable aswell, I just looked at the wrong line.
There are 3 stores of the implicitely converted nullptr, gcc 4.8.4, gcc 4.9.2
and gcc 5.1 all produce one unaligned store - just in different places

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-04-30
     Ever confirmed|0                           |1

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The C++ ABI group are currently discussing alignof(nullptr) and have suggested
it should be the same as alignof(void*) which should fix this issue.

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[npl at chello dot at]

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

--- Comment #5 from npl at chello dot at ---
making nullptr_t "similar" to void* would sound very reasonable to me. I tested
clang and it seems to behave that way.

Whatever the C++ ABI group comes up with, the unaligned accesses have to be
fixed. This was really annoying to track down, as the stores will only be
generated without optimizations - in other words I was searching for a
completely different bug.

I searched the bug database and supposedly throwing and catching nullptr`s is
something legal. So you will have to correctly align it anyway, even if its
arguable if nullptrs should ever be explicitly stored.

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[npl at chello dot at]

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

--- Comment #6 from npl at chello dot at ---
btw... there seems to be a logical fallacy in the way gcc produces stores
anyway.

if a type has size 4 and alignment 1 (like a struct with 4 chars), shouldn`t
the compiler generate byte-writes?
Just curious if there could be a more generic bug hidden, which is triggered by
my testcase (which can be fixed whit a bigger alignof(nullptr)) but might be
triggered in other circumstances aswell.

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[pinskia at gcc dot gnu.org]

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

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
How did you configure your gcc?  If it is configured for armv7 or above, gcc
defaults to assuming unaligned accesses can be done. This is a faq.

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[npl at chello dot at]

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

--- Comment #8 from npl at chello dot at ---
@Andrew Pinski: The code in question was compiled for arm4t. 
The toolchain is not configured for arm7, but has a multilib-configuration for
arm4/armv5te/thumb. No defaults where touched in any way, correct unaligned
load/stores are generated otherwise and __ARM_FEATURE_UNALIGNED is NOT defined.

Further I tested the code below and the gcc will generate a per-byte-copy:
-----------------------------
#pragma pack(1)
struct CPacked
{
        unsigned char m_EvilEpsilon;
        unsigned m_Data;
};

void testaligned(CPacked *pPacked)
{
        pPacked->m_Data = 0;
}
-----------------------------

I tested almost the same thing with nullptr, also results in a per-byte-copy:

-----------------------------
struct CNullpointer
{
        unsigned char m_EvilEpsilon;
        decltype(nullptr) m_Data;
};

void testnull(CNullpointer *pPacked)
{
        pPacked->m_Data = 0;
}
-----------------------------

To me that raises the following points:
*) nullptr_t in an struct will generate the correct code (per-byte-copy)
*) nullptr_t when flushed to the stackframe will write a word without adhering
to alignment restrictions.

Since I doubt there is special handling for nullptr_t for flushing to the
stackframe, this seems like a lingering issue that is hard to reproduce. Even
more so since development and testing nowadays seems focused on platforms which
will allow or emulate unaligned accesses.

maybe if you create a struct of 4 chars with a constexpr constructor that takes
an int you will run in the same issue. fixing nullptr_t alignment might just
hide the issue

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[foom at fuhm dot net]

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

James Y Knight <foom at fuhm dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |foom at fuhm dot net

--- Comment #9 from James Y Knight <foom at fuhm dot net> ---
This also happens on Sparc. A test-case is really easy to make:

int foo(bool&&, decltype(nullptr)&&, bool&&);

void doit() {
  foo(false, nullptr, false);
}


        save    %sp, -104, %sp
        stb     %g0, [%fp-6]
  /* WTF unaligned store */
        st      %g0, [%fp-5]
        stb     %g0, [%fp-1]
        add     %fp, -6, %g3
        add     %fp, -5, %g2
        add     %fp, -1, %g1
        mov     %g3, %o0
        mov     %g2, %o1
        mov     %g1, %o2
        call    _Z3fooObODnS_, 0
         nop
        return  %i7+8
         nop

        .ident  "GCC: (Debian 4.9.2-10) 4.9.2"


This breaks building recent versions of clang on Sparc, because it calls
emplace_back with nullptr, which results in an expression like the above.

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu.org

--- Comment #10 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> This also happens on Sparc.

Right, it was also reported under PR C++/65090.

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[foom at fuhm dot net]

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

--- Comment #11 from James Y Knight <foom at fuhm dot net> ---
As far as I can tell, the C++ ABI discussion,
http://comments.gmane.org/gmane.comp.lang.c++.abi/265
concluded that alignof(nullptr_t) should be alignof(void*) already.

Any chance this change can be made and backported?

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[foom at fuhm dot net]

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

--- Comment #12 from James Y Knight <foom at fuhm dot net> ---
Since this would at least theoretically be a c++11 ABI change -- although it
seems likely not to impact the ABI of most actual software -- it seems like
it'd be a really good idea to fix it ASAP, before too many people start
depending on GCC 5.1's C++11 ABI.

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[jason at gcc dot gnu.org]

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

--- Comment #13 from Jason Merrill <jason at gcc dot gnu.org> ---
Author: jason
Date: Wed Jul  1 17:06:52 2015
New Revision: 225265

URL: https://gcc.gnu.org/viewcvs?rev=225265&root=gcc&view=rev
Log:
        PR c++/65945
gcc/c-family/
        * c-opts.c (c_common_post_options): Highest ABI version is 9.
gcc/cp/
        * decl.c (cxx_init_decl_processing): Set TYPE_ALIGN of nullptr_t.
        * class.c (layout_nonempty_base_or_field): Warn if that affects
        the offset of a field.

Added:
    branches/gcc-5-branch/gcc/testsuite/g++.dg/abi/nullptr-align.C
    branches/gcc-5-branch/gcc/testsuite/g++.dg/abi/nullptr-align2.C
Modified:
    branches/gcc-5-branch/gcc/c-family/ChangeLog
    branches/gcc-5-branch/gcc/c-family/c-opts.c
    branches/gcc-5-branch/gcc/common.opt
    branches/gcc-5-branch/gcc/cp/ChangeLog
    branches/gcc-5-branch/gcc/cp/class.c
    branches/gcc-5-branch/gcc/cp/decl.c
    branches/gcc-5-branch/gcc/testsuite/g++.dg/abi/macro0.C

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[jason at gcc dot gnu.org]

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

--- Comment #14 from Jason Merrill <jason at gcc dot gnu.org> ---
Author: jason
Date: Wed Jul  1 17:59:19 2015
New Revision: 225270

URL: https://gcc.gnu.org/viewcvs?rev=225270&root=gcc&view=rev
Log:
        PR c++/65945
        * decl.c (cxx_init_decl_processing): Set TYPE_ALIGN of nullptr_t.
        * class.c (layout_nonempty_base_or_field): Warn if that affects
        the offset of a field.

Added:
    trunk/gcc/testsuite/g++.dg/abi/nullptr-align.C
    trunk/gcc/testsuite/g++.dg/abi/nullptr-align2.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/class.c
    trunk/gcc/cp/decl.c

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[jason at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
           Assignee|unassigned at gcc dot gnu.org      |jason at gcc dot gnu.org
   Target Milestone|---                         |5.2

--- Comment #15 from Jason Merrill <jason at gcc dot gnu.org> ---
Fixed for GCC 5.2.

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[jason at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dirk.bonne@inform-ac.com

--- Comment #16 from Jason Merrill <jason at gcc dot gnu.org> ---
*** Bug 65090 has been marked as a duplicate of this bug. ***

[Bug c++/65945] C++ alignment of nullptr_t is 1 and might cause unaligned stores to the frame

[npl at chello dot at]

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

--- Comment #17 from npl at chello dot at ---
Hi,

so if I understand it right, the access fault itself isnt fixed, but if I use
ABI Version >= 9 it wont occur with this code?

Should I open a separate Bug for the unaligned access issue?