[Bug c/46483] New: Built-in memcpy() does not handle unaligned int for ARM

[Bug c/46483] New: Built-in memcpy() does not handle unaligned int for ARM

[fredrik.hederstierna@securitas-direct.com]

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

           Summary: Built-in memcpy() does not handle unaligned int for
                    ARM
           Product: gcc
           Version: 4.5.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: fredrik.hederstierna@securitas-direct.com


I had some problems with memcpy() when copying unaligned integers for ARM.

For intel target it worked fine, and I concluded that if I overloaded gcclib
weak memcpy-reference I had the same problem, but if I just overloaded memcpy
with a #define to my own function, it worked fine.

At first glance I though it was just a possible strict-aliasing violation, but
when having a closer look I'm not so sure, so I submit this to Bugzilla anyway
for validation.

I submit some lines of source-code that gives an example of the problem.

----------------------
#include <stdio.h>
----------------------
struct unaligned_int {
  char dummy1;
  char dummy2;
  char dummy3;
  unsigned int number;
} __attribute__((packed));
//---------------------
void *my_memcpy(void *dst, const void *src, size_t n)
{
  char *s = (char*)src;
  char *d = (char*)dst;
  while (n--) {
    *d++ = *s++;
  }
  return dst;
}
//---------------------
static void copy_x_into_struct(char *buf, unsigned int x)
{
  unsigned int i;
  struct unaligned_int* testp = (struct unaligned_int*)buf;

  memset(buf, 0xFF, sizeof(unsigned int));
  memcpy((void*)&(testp->number), &x, sizeof(unsigned int));

  printf("BUILT-IN MEMCPY TO %08x: ", &(testp->number));
  for (i = 0; i < sizeof(struct unaligned_int); i++) {
    printf(" %02x", buf[i]);
  }
  printf("\n");

  memset(buf, 0xFF, sizeof(unsigned int));
  my_memcpy((void*)&(testp->number), &x, sizeof(int));

  printf("USER-DEF MEMCPY TO %08x: ", &(testp->number));
  for (i = 0; i < sizeof(struct unaligned_int); i++) {
    printf(" %02x", buf[i]);
  }
  printf("\n");
}
//---------------------
int main(void)
{
  char buf[100];
  unsigned int x = 0x12345678;
  copy_x_into_struct(buf, x);
  return 0;
}

-------------------------

PROGRAM OUTPUT:

BUILT-IN MEMCPY TO 04016bd7:  78 56 34 12 00 00 00
USER-DEF MEMCPY TO 04016bd7:  ff ff ff 78 56 34 12

-------------------------

GCC-COMMAND-LINE:

arm-elf-gcc -g3 -ggdb3 -gdwarf-2 -mthumb -c -Wall -W -Wextra
-Wno-unused-parameter -mcpu=arm966e-s -Os -fno-omit-frame-pointer -fno-web
-mhard-float -mfpu=fpa -ffunction-sections -fdata-sections 
-o test.o test.c

-------------------------

TOOLCHAIN:  Attaching toolchain build-script.

[Bug c/46483] Built-in memcpy() does not handle unaligned int for ARM

[fredrik.hederstierna@securitas-direct.com]

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

--- Comment #1 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> 2010-11-15 14:42:48 UTC ---
Created attachment 22399
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22399
Script to build arm-elf toolchain

[Bug target/46483] Built-in memcpy() does not handle unaligned int for ARM

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |arm-elf
          Component|c                           |target

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-11-15 14:59:27 UTC ---
Works on x86_64 (I can't see any error with the testcase).  I suspect
autoinc may hose the starting address from the output.

[Bug target/46483] Built-in memcpy() does not handle unaligned int for ARM

[mikpe at it dot uu.se]

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

--- Comment #3 from Mikael Pettersson <mikpe at it dot uu.se> 2010-11-15 21:02:56 UTC ---
Created attachment 22408
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22408
test case showing how __builtin_memcpy ignores known mis-alignment

[Bug target/46483] Built-in memcpy() does not handle unaligned int for ARM

[mikpe at it dot uu.se]

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

--- Comment #4 from Mikael Pettersson <mikpe at it dot uu.se> 2010-11-15 21:04:11 UTC ---
Created attachment 22409
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22409
gcc-4.5.2 output for pr46483.c

[Bug target/46483] Built-in memcpy() does not handle unaligned int for ARM

[mikpe at it dot uu.se]

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

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> 2010-11-15 21:28:12 UTC ---
When I run the original test case on armv5tel-linux-gnueabi it works, but
causes the Linux kernel to log an unaligned access in the binary just executed,
which it fixed up.

The original observation
> BUILT-IN MEMCPY TO 04016bd7:  78 56 34 12 00 00 00
> USER-DEF MEMCPY TO 04016bd7:  ff ff ff 78 56 34 12
is then probably explained by the arm-elf environment (bare metal?) disabling
and ignoring alignment faults, which causes the processor to access different
locations than normally expected.

In the simpler test case I attached we can see how gcc totally ignores
knowledge about mis-alignment when expanding __builtin_memcpy:

> /* pr46483.c */
> 
> struct unaligned_int {
>     char dummy;
>     unsigned int number;
> } __attribute__((packed));
> 
> void set_by_memcpy(struct unaligned_int *p, unsigned int x)
> {
>     __builtin_memcpy(&p->number, &x, sizeof(unsigned int));
> }

for this gcc-4.5 generates

> set_by_memcpy:
>         @ args = 0, pretend = 0, frame = 0
>         @ frame_needed = 0, uses_anonymous_args = 0
>         @ link register save eliminated.
>         str     r1, [r0, #1]
>         bx      lr

which does an int-sized write to &p->number (r0+1), which isn't int-aligned.

For the equivalent plain assignment:

> void set_by_assignment(struct unaligned_int *p, unsigned int x)
> {
>     p->number = x;
> }

gcc-4.5 generates

> set_by_assignment:
>         @ args = 0, pretend = 0, frame = 0
>         @ frame_needed = 0, uses_anonymous_args = 0
>         @ link register save eliminated.
>         mov     ip, r1, lsr #8
>         mov     r2, r1, lsr #16
>         mov     r3, r1, lsr #24
>         strb    r1, [r0, #1]
>         strb    ip, [r0, #2]
>         strb    r2, [r0, #3]
>         strb    r3, [r0, #4]
>         bx      lr

so here it does know about the misalignment and avoids misaligned int-sized
writes.

Finally let's ask gcc about the known alignment of an unaligned_int:

> unsigned int alignof_p(struct unaligned_int *p)
> {
>     return __alignof__(p);
> }
> 
> unsigned int alignof_p_number(struct unaligned_int *p)
> {
>     return __alignof__(p->number);
> }

for this it generates

> alignof_p:
>         @ args = 0, pretend = 0, frame = 0
>         @ frame_needed = 0, uses_anonymous_args = 0
>         @ link register save eliminated.
>         mov     r0, #4
>         bx      lr
...
> alignof_p_number:
>         @ args = 0, pretend = 0, frame = 0
>         @ frame_needed = 0, uses_anonymous_args = 0
>         @ link register save eliminated.
>         mov     r0, #1
>         bx      lr

so clearly gcc knows that the number field is only byte-aligned.

gcc-4.4.5 generates similar bogus code for __builtin_memcpy.  Current 4.6
however converts the __builtin_memcpy to a call to the memcpy library routine.

[Bug target/46483] Built-in memcpy() does not handle unaligned int for ARM

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> 2010-11-15 21:32:45 UTC ---
I think the biggest issue is what is the type of &p->number ? is it an pointer
to an int or a pointer to an int with an alignment of 1.

[Bug target/46483] Built-in memcpy() does not handle unaligned int for ARM

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-11-15 22:49:21 UTC ---
Sounds like the very old bad alignment issues for string-alignment targets,
should be fixed (conservatively) in 4.6.

[Bug target/46483] Built-in memcpy() does not handle unaligned int for ARM

[mikpe at it dot uu.se]

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

--- Comment #8 from Mikael Pettersson <mikpe at it dot uu.se> 2010-11-16 10:35:41 UTC ---
The misaligned __builtin_memcpy was fixed for 4.6 by r163189, Richard
Guenther's conservative alignment tracking (2nd try) patch:
http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00831.html
http://gcc.gnu.org/ml/gcc-cvs/2010-08/msg00400.html

[Bug target/46483] Built-in memcpy() does not handle unaligned int for ARM

[mikpe at it dot uu.se]

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

--- Comment #9 from Mikael Pettersson <mikpe at it dot uu.se> 2010-11-16 12:21:20 UTC ---
With gcc-4.5.1, the plain assignment is preserved until 141r.expand, which
expands it to a bitfield assignment due to the misalignment check in
expr.c:store_field lines 5836-5840.

The __builtin_memcpy is turned into a plain indirect assignment in 004t.gimple:

set_by_memcpy (struct unaligned_int * p, unsigned int x)
{
  unsigned int * D.2026;
  unsigned int * {ref-all} D.2027;
  unsigned int x.0;

  D.2026 = &p->number;
  D.2027 = (unsigned int * {ref-all}) D.2026;
  x.0 = x;
  *D.2027 = x.0;
}

and this misaligned assignment is never corrected by later passes.

[Bug target/46483] Built-in memcpy() does not handle unaligned int for ARM

[mikpe at it dot uu.se]

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

--- Comment #10 from Mikael Pettersson <mikpe at it dot uu.se> 2010-11-16 13:08:53 UTC ---
The test cases also fail with gcc-4.5.1 on sparc64-linux.  On that platform
alignment errors are fatal and the program dies with a SIGBUS on the misaligned
int-sized write generated for the __builtin_memcpy.

[Bug target/46483] Built-in memcpy() does not handle unaligned int for ARM

[mikpe at it dot uu.se]

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

--- Comment #11 from Mikael Pettersson <mikpe at it dot uu.se> 2010-11-16 13:40:32 UTC ---
The test cases work with gcc-4.3.5.  For arm it generates the following for the
__builtin_memcpy case:

set_by_memcpy:
        @ args = 0, pretend = 0, frame = 8
        @ frame_needed = 0, uses_anonymous_args = 0
        str     lr, [sp, #-4]!
        sub     sp, sp, #12
        add     r3, sp, #8
        str     r1, [r3, #-4]!
        mov     r1, r3
        mov     r2, #4
        add     r0, r0, #1
        bl      memcpy
        add     sp, sp, #12
        ldmfd   sp!, {pc}

and for sparc it generates:

set_by_memcpy:
        st      %o1, [%sp+72]
        ldub    [%sp+72], %g1
        add     %sp, 72, %g3
        stb     %g1, [%o0+1]
        ldub    [%g3+3], %g1
        add     %o0, 1, %o0
        stb     %g1, [%o0+3]
        ldub    [%g3+1], %g2
        stb     %g2, [%o0+1]
        ldub    [%g3+2], %g1
        jmp     %o7+8
         stb    %g1, [%o0+2]

[Bug target/46483] Built-in memcpy() does not handle unaligned int for ARM

[mikpe at it dot uu.se]

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

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

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

--- Comment #12 from Mikael Pettersson <mikpe at it dot uu.se> 2010-11-16 19:11:03 UTC ---
The bug was introduced between 4.3 and 4.4 in r142061, Jakub's "More aggressive
__builtin_memcpy optimizations (PR middle-end/29215)" patch:
http://gcc.gnu.org/ml/gcc-cvs/2008-11/msg00562.html
http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00637.html

[Bug target/46483] Built-in memcpy() does not handle unaligned int for ARM

[ramana at gcc dot gnu.org]

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

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2010.12.03 10:02:39
                 CC|                            |ramana at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #13 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> 2010-12-03 10:02:39 UTC ---
Trunk appears to be fixed but very conservatively as Richi says but 4.5
probably exhibits the issue from Mikael's last comment.

[Bug middle-end/46483] [4.4/4.5 regression] built-in memcpy() does not handle unaligned int

[ebotcazou at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|arm-elf                     |
                 CC|                            |ebotcazou at gcc dot
                   |                            |gnu.org
          Component|target                      |middle-end
   Target Milestone|---                         |4.5.4
            Summary|Built-in memcpy() does not  |[4.4/4.5 regression]
                   |handle unaligned int for    |built-in memcpy() does not
                   |ARM                         |handle unaligned int

--- Comment #14 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-12-10 21:22:14 UTC ---
IMO there is no other solution than disabling the folding for strict-alignment
targets, i.e. reverting to the 4.3 behavior.  Wh

[Bug middle-end/46483] [4.5 regression] built-in memcpy() does not handle unaligned int

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
           Priority|P3                          |P2

[Bug middle-end/46483] [4.5 regression] built-in memcpy() does not handle unaligned int

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

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

--- Comment #15 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-07-02 10:38:23 UTC ---
Fixed in 4.6.0, the 4.5 branch is being closed.