[Bug middle-end/50716] New: Segmentation fault caused by misaligned vector access

[Bug middle-end/50716] New: Segmentation fault caused by misaligned vector access

[tema at gcc dot gnu.org]

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

             Bug #: 50716
           Summary: Segmentation fault caused by misaligned vector access
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: tema@gcc.gnu.org


The following code segfaults on i386 + sse targets:

typedef int vec __attribute__((vector_size(16)));
int main ()
{
 int * arr = __builtin_malloc (1024);
 vec *p = (vec *) &arr[1];
 *p = (vec){1, 2, 3, 4};
 return *(char *)p;
}

The problem is that *p = (vec){1,2,3,4} produces aligned move instead of
unaligned.  Most likely this could be reproduced on any target with SIMD
extensions, where aligned move differs from unaligned.

[Bug middle-end/50716] Segmentation fault caused by misaligned vector access

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-10-13 19:44:32 UTC ---
I think vec has an alignment requirement which makes the testcase invalid.

[Bug middle-end/50716] Segmentation fault caused by misaligned vector access

[artyom.shinkaroff at gmail dot com]

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

Artem Shinkarov <artyom.shinkaroff at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |artyom.shinkaroff at gmail
                   |                            |dot com

--- Comment #2 from Artem Shinkarov <artyom.shinkaroff at gmail dot com> 2011-10-13 19:51:36 UTC ---
(In reply to comment #1)
> I think vec has an alignment requirement which makes the testcase invalid.

Ok, but in that case the compiler should say something to the user.  Rejecting
such a code could be a first step.

However, in general I think that the compiler should analyse the code and
figure our which instruction suits in the given situation.  If alignment can be
evaluated -- aligned move, unaligned move otherwise.

[Bug middle-end/50716] Segmentation fault caused by misaligned vector access

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-10-14
                 CC|richard.guenther at gmail   |rguenth at gcc dot gnu.org
                   |dot com                     |
     Ever Confirmed|0                           |1

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-14 09:30:37 UTC ---
In this case the compiler knows that p is aligned to 8, misaligned 1, so
it has all the information to ensure to use an unaligned vector move.
So I belive it should use that information.

Somebody needs to step through RTL expansion and see where we ignore
the information.

[Bug middle-end/50716] Segmentation fault caused by misaligned vector access

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-14 12:09:27 UTC ---
It's because we do

        align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), get_object_alignment (exp));

which discards the knowledge we have (exp is aligned to 4 bytes,
get_object_alignment returns 4).

Which can be fixed for example by

        align = get_object_alignment_1 (exp, &misalign);
        align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
        if (misalign != 0)
          align = (misalign & -misalign);

so always honor an explicit knowledge about misalignment.  Or less
aggressively,

        align = get_object_alignment_1 (exp, &misalign);
        if (TYPE_ALIGN (TREE_TYPE (exp)) <= align)
          {
            if (misalign != 0)
              align = (misalign & -misalign);
          } 
        else
          align = TYPE_ALIGN (TREE_TYPE (exp));

thus only when the base alignment is at least that of the types alignment.
Which means we'd treat a vector load from a misaligned int * pointer as
aligned, but from a misaligned vector int * pointer not - maybe too surprising,
I'd definitely go with the first variant.  Any idea which reasonable
case we'd miss here?  Even *(vector int *)(int-ptr + 2) would be handled
as aligned, get_object_alignment_1 would return 32 (int aligned), the
misalign is truncated to the base alignment.

[Bug middle-end/50716] Segmentation fault caused by misaligned vector access

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-14 12:10:52 UTC ---
(In reply to comment #4)
> It's because we do
> 
>         align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), get_object_alignment (exp));
> 
> which discards the knowledge we have (exp is aligned to 4 bytes,
> get_object_alignment returns 4).
> 
> Which can be fixed for example by
> 
>         align = get_object_alignment_1 (exp, &misalign);
>         align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
>         if (misalign != 0)
>           align = (misalign & -misalign);
> 
> so always honor an explicit knowledge about misalignment.  Or less
> aggressively,
> 
>         align = get_object_alignment_1 (exp, &misalign);
>         if (TYPE_ALIGN (TREE_TYPE (exp)) <= align)
>           {
>             if (misalign != 0)
>               align = (misalign & -misalign);
>           } 
>         else
>           align = TYPE_ALIGN (TREE_TYPE (exp));
> 
> thus only when the base alignment is at least that of the types alignment.
> Which means we'd treat a vector load from a misaligned int * pointer as
> aligned, but from a misaligned vector int * pointer not - maybe too surprising,
> I'd definitely go with the first variant.  Any idea which reasonable
> case we'd miss here?  Even *(vector int *)(int-ptr + 2) would be handled
> as aligned, get_object_alignment_1 would return 32 (int aligned), the
> misalign is truncated to the base alignment.

One odd result is that with -O the code would "work" while with -O0
it would segfault (we don't know anything about the alignment at -O0 as
CCP is not run).

[Bug middle-end/50716] Segmentation fault caused by misaligned vector access

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-17 12:21:11 UTC ---
Candidate patch:

http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01508.html

[Bug middle-end/50716] Segmentation fault caused by misaligned vector access

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-18 08:46:06 UTC ---
Author: rguenth
Date: Tue Oct 18 08:46:00 2011
New Revision: 180125

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=180125
Log:
2011-10-18  Richard Guenther  <rguenther@suse.de>

    PR middle-end/50716
    * expr.c (get_object_or_type_alignment): New function.
    (expand_assignment): Use it.
    (expand_expr_real_1): Likewise.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/expr.c

[Bug middle-end/50716] Segmentation fault caused by misaligned vector access

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-18 11:45:53 UTC ---
"Fixed."