[Bug c/53016] New: memcpy optimization can cause unaligned access on ARM

[Bug c/53016] New: memcpy optimization can cause unaligned access on ARM

[jquesnelle at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53016

             Bug #: 53016
           Summary: memcpy optimization can cause unaligned access on ARM
    Classification: Unclassified
           Product: gcc
           Version: 4.4.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jquesnelle@gmail.com


Created attachment 27174
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27174
reproduction files

The built-in memcpy that -O2 substitutes in seems to cause an unaligned memory
access on ARMv5TE when structs are stacked in a certain way. I originally
discovered this when a release build of native code for inclusion in an Android
program caused a SIGBUS. Attached is a simple test case that replicates this on
Android. There is no main() function but it should be trivial to substitute in
(sorry, I don't have access to a regular ARM Linux box). It appears to involve
over-aggressive use of ldm/stm (possibly ignoring padding?). 

Works fine (-O0):
memcpy((void*)&parent.children[2],(const void*)child3,size);
  24:    4b0a          ldr    r3, [pc, #40]    
  26:    447b          add    r3, pc
  28:    1c19          adds    r1, r3, #0
  2a:    3138          adds    r1, #56
  2c:    4b09          ldr    r3, [pc, #36]    
  2e:    447b          add    r3, pc
  30:    681b          ldr    r3, [r3, #0]
  32:    9a03          ldr    r2, [sp, #12]
  34:    1c08          adds    r0, r1, #0
  36:    1c11          adds    r1, r2, #0
  38:    1c1a          adds    r2, r3, #0
  3a:    f7ff fffe     bl    0 <memcpy>

Gives SIGBUS (-O2):
memcpy((void*)&parent.children[2],(const void*)child3,size);
   2:    4b07          ldr    r3, [pc, #28]
   4:    4907          ldr    r1, [pc, #28]    
   6:    447b          add    r3, pc
   8:    681a          ldr    r2, [r3, #0]
   a:    4479          add    r1, pc
   c:    3138          adds    r1, #56
   e:    1c0b          adds    r3, r1, #0
  10:    323c          adds    r2, #60
  12:    ca31          ldmia    r2!, {r0, r4, r5} <--- Unaligned access
  14:    c331          stmia    r3!, {r0, r4, r5}
  16:    ca13          ldmia    r2!, {r0, r1, r4}
  18:    c313          stmia    r3!, {r0, r1, r4}
  1a:    6812          ldr    r2, [r2, #0]
  1c:    601a          str    r2, [r3, #0]

I have confirmed this both on a TI OMAP 3530 (BeagleBoard) and Samsung Exynos
3110 (Samsung Epic 4G). I'm not sure if this is the same as bug #47754.

[Bug target/53016] memcpy optimization can cause unaligned access on ARM

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53016

--- Comment #1 from Mikael Pettersson <mikpe at it dot uu.se> 2012-04-17 08:14:49 UTC ---
Created attachment 27175
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27175
reduced test case

Gets alignment faults with both -mthumb and -marm (+ -march=armv5te -O1) using
gcc 4.4.7 and 4.5.3, but not with 4.6.3 or 4.7.0.

With -marm I get alignment faults that the kernel fixes up.  With -mthumb I get
a fatal SIGBUS and a kernel message like the following:

Alignment trap: a.out (10263) PC=0x0000834e Instr=0xc331ca31 Address=0xbea0b335
FSR 0x001
Alignment trap: not handling instruction c331ca31 at [<0000834e>]
Unhandled fault: alignment exception (0x001) at 0xbea0b335

I'm not sure if that's due to a kernel bug or because I've neglected to enable
some Thumb1-specific support in my kernel (I normally only run pure ARM code).

[Bug target/53016] memcpy optimization can cause unaligned access on ARM

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53016

Mikael Pettersson <mikpe at it dot uu.se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikpe at it dot uu.se

--- Comment #2 from Mikael Pettersson <mikpe at it dot uu.se> 2012-04-17 08:17:49 UTC ---
(In reply to comment #1)
> Gets alignment faults with both -mthumb and -marm (+ -march=armv5te -O1) using
> gcc 4.4.7 and 4.5.3, but not with 4.6.3 or 4.7.0.

Forgot to mention, this is on armv5tel-linux-gnueabi.

[Bug target/53016] memcpy optimization can cause unaligned access on ARM

[rearnsha at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53016

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

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

--- Comment #3 from Richard Earnshaw <rearnsha at gcc dot gnu.org> 2012-04-17 14:37:13 UTC ---
child_t is a struct that requires 32-bit alignment, so the compiler is entitled
to assume that any pointer to an object containing a child_t will be correctly
aligned.  The line:

const parent_t* parentptr = (const parent_t*)(data + 1);

violates that assumption, so your code has undefined behaviour.  In this case
that turns out to be an alignment fault at run-time.

[Bug target/53016] memcpy optimization can cause unaligned access on ARM

[jquesnelle at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53016

Jeffrey Quesnelle <jquesnelle at gmail dot com> changed:

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

--- Comment #4 from Jeffrey Quesnelle <jquesnelle at gmail dot com> 2012-04-17 16:34:12 UTC ---
That may be the case for something like an operator=, but memcpy takes a void*
(an opaque stream of bytes). In fact, I originally had an used operator= here
which caused a SIGBUS, which was reasonable. Given such a problem, the solution
is clearly "well it's unaligned, use a memcpy", which is what I did. In -O2
however the behavior is essentially equivalent to an operator=, but memcpy was
exactly the solution needed to get away from the problem created by using an
operator=!

In the memcpy line in the test case, I even have casts to (void*) and (const
void *). I would argue that the compiler is not entitled to treat a memcpy as
if it were an operator= when manual pointer arithmetic and direct casts to the
opaque byte type imply that we don't want a member-by-member copy but rather a
byte-by-byte copy. Crucially, memcpy is likely to be used exactly at times when
this behavior is needed.

Reverting to unconfirmed. If you still disagree with my argument revert back to
invalid, but I wanted to explain how this code can (and was) written from a
reasonable thought process and as such could reasonably be expected to work.

As a side note, this problem doesn't in G++ if reinterpret_cast<> is used on
the arguments to memcpy.

[Bug target/53016] memcpy optimization can cause unaligned access on ARM

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53016

Mikael Pettersson <mikpe at it dot uu.se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|mikpe at it dot uu.se       |

--- Comment #5 from Mikael Pettersson <mikpe at it dot uu.se> 2012-04-17 16:55:34 UTC ---
If you have misaligned data either use void*/char* and explicit address
arithmetic, or use __attribute__((packed)) on the misaligned types.

[Bug target/53016] memcpy optimization can cause unaligned access on ARM

[jquesnelle at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53016

--- Comment #6 from Jeffrey Quesnelle <jquesnelle at gmail dot com> 2012-04-17 17:08:22 UTC ---
Hmm, even explicit casts to new void*/char* types doesn't fix it:

const child_t * child3 = (const child_t*)( (const char*)(parentptr) + 4 + size
+ size);
const void* src = (const void*)child3;
void* dest = (void*)&parent.children[2];
memcpy(dest,src,size);

const child_t * child3 = (const child_t*)( (const char*)(parentptr) + 4 + size
+ size);
const unsigned char* src = (const unsigned char*)child3;
unsigned char* dest = (unsigned char*)&parent.children[2];
memcpy(dest,src,size);

Both of these still cause the alignment fault.

[Bug target/53016] memcpy optimization can cause unaligned access on ARM

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53016

--- Comment #7 from Mikael Pettersson <mikpe at it dot uu.se> 2012-04-17 17:27:20 UTC ---
The problem is the fact that you attach explicit child_t/parent_t types to
pointers not aligned accordingly.  Casting those typed pointers to void*
doesn't eliminate the implied alignment.  Marking those types packed fixes the
test case.

[Bug target/53016] memcpy optimization can cause unaligned access on ARM

[rearnsha at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53016

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

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

--- Comment #8 from Richard Earnshaw <rearnsha at gcc dot gnu.org> 2012-04-18 14:23:04 UTC ---
Casts do not change the fact that the pointer initialized in statement I
highlighted creates undefined behaviour.  The fact that you are using memcpy is
a red herring.

>From c99:

6.3.2.3 Pointers

7) A pointer to an object or incomplete type may be converted to a pointer to a
different object or incomplete type. If the resulting pointer is not correctly
aligned for the pointed-to type, the behavior is undefined. ...