[Bug c/55614] New: vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[Bug c/55614] New: vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[soren.sandmann at gmail dot com]

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

             Bug #: 55614
           Summary: vector extensions cause movdqa to be generated for
                    memcpy on unaligned buffer
    Classification: Unclassified
           Product: gcc
           Version: 4.7.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: soren.sandmann@gmail.com


Created attachment 28891
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28891
Program in which memcpy() is miscompiled

When the attached program is compiled with 4.7.2 with -O2, the memcpy() is
expanded to a movdqa instruction, but the buffer passed is unaligned, so we get
a crash.



localhost:~% gcc -v
Using built-in specs.
COLLECT_GCC=/usr/bin/gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/4.7.2/lto-wrapper
Target: x86_64-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
--infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla
--enable-bootstrap --enable-shared --enable-threads=posix
--enable-checking=release --disable-build-with-cxx
--disable-build-poststage1-with-cxx --with-system-zlib --enable-__cxa_atexit
--disable-libunwind-exceptions --enable-gnu-unique-object
--enable-linker-build-id --with-linker-hash-style=gnu
--enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto --enable-plugin
--enable-initfini-array --enable-java-awt=gtk --disable-dssi
--with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre
--enable-libgcj-multifile --enable-java-maintainer-mode
--with-ecj-jar=/usr/share/java/eclipse-ecj.jar --disable-libjava-multilib
--with-ppl --with-cloog --with-tune=generic --with-arch_32=i686
--build=x86_64-redhat-linux
Thread model: posix
gcc version 4.7.2 20120921 (Red Hat 4.7.2-2) (GCC)

[Bug c/55614] vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[mikpe at it dot uu.se]

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

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

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

--- Comment #1 from Mikael Pettersson <mikpe at it dot uu.se> 2012-12-07 13:59:05 UTC ---
SEGVs when compiled by 4.7 or 4.6.3, works when compiled by 4.8 or 4.5.4 and
older.

[Bug c/55614] [4.6/4.7 Regression] vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-12-07
                 CC|                            |ebotcazou at gcc dot
                   |                            |gnu.org, jakub at gcc dot
                   |                            |gnu.org, jamborm at gcc dot
                   |                            |gnu.org
   Target Milestone|---                         |4.6.4
            Summary|vector extensions cause     |[4.6/4.7 Regression] vector
                   |movdqa to be generated for  |extensions cause movdqa to
                   |memcpy on unaligned buffer  |be generated for memcpy on
                   |                            |unaligned buffer
     Ever Confirmed|0                           |1

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-12-07 14:05:29 UTC ---
Reduced testcase:

typedef unsigned int T __attribute__ ((vector_size (16)));
extern int rand ();

void
foo (unsigned char *x, unsigned long y)
{
  union { T v; } r;
  T e = { 0, rand (), 0, 0 };
  r.v = e;
  while (y >= 16)
    {
      __builtin_memcpy (x, &r, 16);
      x += 16;
      y -= 16;
    }
}

int
main ()
{
  unsigned char b[32];
  foo (b + 1, 16);
  return b[16];
}

Works on trunk, likely (haven't bisected it to the last few revs, so it is
range r161958 (last known good) and r161976 (first broken)) introduced by
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161974
and fixed by
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186501
(or just made latent?).
CCing authors of corresponding changes.

[Bug tree-optimization/55614] [4.6/4.7 Regression] vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |tree-optimization
      Known to work|                            |4.5.0, 4.8.0
      Known to fail|                            |4.6.3, 4.7.2

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-12-07 14:30:22 UTC ---
The difference from broken to working starts at the esra pass:
 <bb 3>:
-  MEM[(char * {ref-all})x_1].v = r$v_15;
+  MEM[(char * {ref-all})x_1] = r$v_15;
   x_8 = x_1 + 16;
   y_9 = y_2 + 18446744073709551600;

Note that the MEM[(char * {ref-all})x_1] is
 <mem_ref 0x7ffff1a93d48
    type <union_type 0x7ffff1a9cb28 sizes-gimplified type_0 TI
        size <integer_cst 0x7ffff1982dc0 constant 128>
        unit size <integer_cst 0x7ffff1982de0 constant 16>
        align 8 symtab 0 alias set -1 canonical type 0x7ffff1a9c738
        fields <field_decl 0x7ffff19a45f0 v type <vector_type 0x7ffff1a9c498 T>
            unsigned V4SI file rh885082.c line 7 col 13 size <integer_cst
0x7ffff1982dc0 128> unit size <integer_cst 0x7ffff1982de0 16>
            align 128 offset_align 128
            offset <integer_cst 0x7ffff1982d80 constant 0>
            bit offset <integer_cst 0x7ffff1982e00 constant 0> context
<union_type 0x7ffff1a9c738>> context <function_decl 0x7ffff1a9ba00 foo>>

    arg 0 <ssa_name 0x7ffff1985bd0
        type <pointer_type 0x7ffff1a9c5e8 type <integer_type 0x7ffff1997348
unsigned char>
            unsigned DI
            size <integer_cst 0x7ffff1982d40 constant 64>
            unit size <integer_cst 0x7ffff1982d60 constant 8>
            align 64 symtab 0 alias set -1 canonical type 0x7ffff1a9c5e8>
        visited var <parm_decl 0x7ffff1ab6000 x>def_stmt x_1 = PHI <x_10(D)(2),
x_13(3)>

        version 1
        ptr-info 0x7ffff1aa48a0>
    arg 1 <integer_cst 0x7ffff1aa63e0 type <pointer_type 0x7ffff1a9ca80>
constant 0>>

thus it has the right TYPE_ALIGN.  But when it is wrapped into COMPONENT_REF,
while the inner MEM_REF still has TYPE_ALIGN 8, the FIELD_REF has DECL_ALIGN
128, which is probably why the expansion expands it as aligned.
If the r186501 patch is too risky for the release branches (did it need any
follow-ups so far?), an alternative would be to start doing what trunk does
only
if the MEM_REF has lower alignment than the field.

[Bug tree-optimization/55614] [4.6/4.7 Regression] vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[jamborm at gcc dot gnu.org]

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

--- Comment #4 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-12-07 14:56:15 UTC ---
As far as I can remember, my changes in revision 186501 depended on
previous work in expander and cannot be simply backported without
those prerequisites (or breaking strict-alignment requirements on many
platforms in quite a few cases).

[Bug tree-optimization/55614] [4.6/4.7 Regression] vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[ebotcazou at gcc dot gnu.org]

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

--- Comment #5 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-12-07 14:57:28 UTC ---
> The difference from broken to working starts at the esra pass:
>  <bb 3>:
> -  MEM[(char * {ref-all})x_1].v = r$v_15;
> +  MEM[(char * {ref-all})x_1] = r$v_15;
>    x_8 = x_1 + 16;
>    y_9 = y_2 + 18446744073709551600;
> 
> Note that the MEM[(char * {ref-all})x_1] is
>  <mem_ref 0x7ffff1a93d48
>     type <union_type 0x7ffff1a9cb28 sizes-gimplified type_0 TI
>         size <integer_cst 0x7ffff1982dc0 constant 128>
>         unit size <integer_cst 0x7ffff1982de0 constant 16>
>         align 8 symtab 0 alias set -1 canonical type 0x7ffff1a9c738
>         fields <field_decl 0x7ffff19a45f0 v type <vector_type 0x7ffff1a9c498 T>
>             unsigned V4SI file rh885082.c line 7 col 13 size <integer_cst
> 0x7ffff1982dc0 128> unit size <integer_cst 0x7ffff1982de0 16>
>             align 128 offset_align 128
>             offset <integer_cst 0x7ffff1982d80 constant 0>
>             bit offset <integer_cst 0x7ffff1982e00 constant 0> context
> <union_type 0x7ffff1a9c738>> context <function_decl 0x7ffff1a9ba00 foo>>

How can a union have alignment 8 if it contains a field with alignment 128? 
That should only happen if the field is a bit-field...

> thus it has the right TYPE_ALIGN.  But when it is wrapped into COMPONENT_REF,
> while the inner MEM_REF still has TYPE_ALIGN 8, the FIELD_REF has DECL_ALIGN
> 128, which is probably why the expansion expands it as aligned.

...in which case the expansion should expand it as a bit-field.

> If the r186501 patch is too risky for the release branches (did it need any
> follow-ups so far?), an alternative would be to start doing what trunk does
> only if the MEM_REF has lower alignment than the field.

I don't think that r186501 can be backported, unless you want to break again
the strict-alignment platforms.  r161958 just fixed the disaster introduced on
them by an ealier patch, see PR middle-end/44843.

[Bug tree-optimization/55614] [4.6/4.7 Regression] vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-12-07 15:28:30 UTC ---
(In reply to comment #5)
> How can a union have alignment 8 if it contains a field with alignment 128? 
> That should only happen if the field is a bit-field...

That is what the memcpy folding folds it to.  We could perhaps stop for 4.6/4.7
doing such replacements in memcpy folding if it would try to decrease alignment
of some aggregate, but then we risk regressing generated code quality.

[Bug tree-optimization/55614] [4.6/4.7 Regression] vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[ebotcazou at gcc dot gnu.org]

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

--- Comment #7 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-12-07 15:40:08 UTC ---
> That is what the memcpy folding folds it to.  We could perhaps stop for 4.6/4.7
> doing such replacements in memcpy folding if it would try to decrease alignment
> of some aggregate, but then we risk regressing generated code quality.

It's the build_aligned_type paths in fold_builtin_memory_op, right?  No wonder
you get problems by breaking such a fundamental invariant as the type
alignment.

I guess you can kludge in SRA and disable the COMPONENT_REF thing if the
platform isn't STRICT_ALIGNMENT.

[Bug tree-optimization/55614] [4.6/4.7 Regression] vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[siarhei.siamashka at gmail dot com]

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

--- Comment #8 from Siarhei Siamashka <siarhei.siamashka at gmail dot com> 2012-12-09 20:59:42 UTC ---
FWIW, the current gentoo patchset for gcc-4.7.2 contains
10_all_default-fortify-source.patch intended to "Enable -D_FORTIFY_SOURCE=2 by
default". It makes this bug non-reproducible there (or just latent?).

[Bug tree-optimization/55614] [4.6/4.7 Regression] vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[siarhei.siamashka at gmail dot com]

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

--- Comment #9 from Siarhei Siamashka <siarhei.siamashka at gmail dot com> 2012-12-09 22:25:17 UTC ---
*** Bug 55454 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/55614] [4.6/4.7 Regression] vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[rguenth at gcc dot gnu.org]

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

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

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

[Bug tree-optimization/55614] [4.7 Regression] vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.6.4                       |4.7.4

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-12 15:15:45 UTC ---
GCC 4.6.4 has been released and the branch has been closed.

[Bug tree-optimization/55614] [4.7 Regression] vector extensions cause movdqa to be generated for memcpy on unaligned buffer

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|4.7.4                       |4.8.0
      Known to fail|                            |4.7.4

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed for 4.8.0.