[Bug c/49413] New: over-optimization that causes valid code to segfault

[Bug c/49413] New: over-optimization that causes valid code to segfault

[gattis at gmail dot com]

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

           Summary: over-optimization that causes valid code to segfault
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: gattis@gmail.com


Created attachment 24529
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24529
source code

The attached short .c code compiles without warnings and segfaults when -O3 is
used.  The code runs fine with -O1 or if I declare v as "volatile double *"
instead of just "double *".  This was triggered in a much larger code base. 
I've managed to strip everything else out and reduce it to this.  I'm pretty
sure the code left is correct, so I think it's a GCC bug.

GCC Version: 4.6.0 w/ gmp-5.0.2, mpc-0.9, mpfr-3.0.1
System: CentOS 5 x86_64 (AMD Istanbull)
GCC ./configure flags: --enable-languages=c,c++,fortran
Source compilation command: gcc -O3 volatile.c -o volatile

[Bug c/49413] over-optimization that causes valid code to segfault

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-06-15 00:34:37 UTC ---
I think this code is undefined as the alignment requirements of double is 8
bytes but the original (t->xyz->va/t->xyz->vb) is packed so it has a alignment
of 4 bytes.

[Bug c/49413] over-optimization that causes valid code to segfault

[gattis at gmail dot com]

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

--- Comment #2 from Matt Gattis <gattis at gmail dot com> 2011-06-15 00:38:29 UTC ---
Created attachment 24530
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24530
verbose gcc output

[Bug c/49413] over-optimization that causes valid code to segfault

[gattis at gmail dot com]

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

--- Comment #3 from Matt Gattis <gattis at gmail dot com> 2011-06-15 00:39:06 UTC ---
Created attachment 24531
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24531
verbose gcc output

[Bug tree-optimization/49413] over-optimization that causes valid code to segfault

[paolo.carlini at oracle dot com]

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

--- Comment #4 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-06-15 01:00:34 UTC ---
Personally, I find the expression "over-optimization" misleading: either we
have a compiler *bug*, which therefore is performing an incorrect
transformation, or we don't, thus the code is triggering undefined behavior
(which happens to show up at high optimization level). In either case, I don't
see "over-optimization".

[Bug tree-optimization/49413] over-optimization that causes valid code to segfault

[gattis at gmail dot com]

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

--- Comment #5 from Matt Gattis <gattis at gmail dot com> 2011-06-15 01:10:35 UTC ---
(In reply to comment #1)
> I think this code is undefined as the alignment requirements of double is 8
> bytes but the original (t->xyz->va/t->xyz->vb) is packed so it has a alignment
> of 4 bytes.

Isn't the only reason to use a packed struct to be able to squeeze misaligned
data together and have it all work?  My understanding is that GCC is supposed
to automatically take care of the alignment using extra instructions.  And it
does, except when I turn more optimizations on.

For some added context, I am mmap'ing a large file with the data packed in that
struct format.  Are you saying the only way I would get "defined" behavior from
GCC 
is if I referenced xyz->va[i] every access, instead of assigning a temp
variable to xyz->va?

[Bug tree-optimization/49413] over-optimization that causes valid code to segfault

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-06-15 01:13:14 UTC ---
The problem is:
  double *v = (qp == &(t->q)) ? t->xyz->va : t->xyz->vb;

the pointer v is a pointer to a 8 byte aligned data but you supply it with
something which is only 4 byte aligned.

[Bug tree-optimization/49413] over-optimization that causes valid code to segfault

[gattis at gmail dot com]

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

--- Comment #7 from Matt Gattis <gattis at gmail dot com> 2011-06-15 01:19:57 UTC ---
(In reply to comment #6)
> The problem is:
>   double *v = (qp == &(t->q)) ? t->xyz->va : t->xyz->vb;
> 
> the pointer v is a pointer to a 8 byte aligned data but you supply it with
> something which is only 4 byte aligned.

Silly me, I thought the whole point of a compiler was to make it so you didn't
have to code to machine-specific byte alignments and instructions.

[Bug tree-optimization/49413] over-optimization that causes valid code to segfault

[gabriel at teuton dot org]

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

Gabriel M. Beddingfield <gabriel at teuton dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gabriel at teuton dot org

--- Comment #8 from Gabriel M. Beddingfield <gabriel at teuton dot org> 2011-08-04 03:59:01 UTC ---
I ran into the same problem (gcc 4.5.2, Ubuntu 11.04, x86 Core Duo). 
Unfortunately, it looks like Mr. Pinski is right.

I don't have a copy of the C99 spec, but I do have the C++ 2003 spec.  It says
that alignment requirements are implementation-defined.  In 3.9.2 it says
"Pointers to cv-qualified and cv-unqualified versions (3.9.3) of
layout-compatible types shall have the same value representation and alignment
requirements(3.9)."  I'm sure C99 has a similar requirement.

I.e. since a `double` is required to be 8-byte aligned, so is a valid
`double*`.  Unfortunately, this means that:

   double *ra = malloc(512*sizeof(double));

...can return an invalid (unaligned) array.

However, on most archs I'm pretty sure the optimizer already checks alignment
in order to do things like SIMD instructions.  So this is probably a valid
FEATURE REQUEST... that the optimizer supports (but not necc. optimize)
pointers with arbitrary alignment.

[Bug tree-optimization/49413] over-optimization that causes valid code to segfault

[gattis at gmail dot com]

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

--- Comment #9 from Matt Gattis <gattis at gmail dot com> 2011-08-04 04:21:20 UTC ---
> However, on most archs I'm pretty sure the optimizer already checks alignment
> in order to do things like SIMD instructions.  So this is probably a valid
> FEATURE REQUEST... that the optimizer supports (but not necc. optimize)
> pointers with arbitrary alignment.

The thing is, it works on every previous version of gcc I've tried, even with
-ftree-vectorize on, and works with 4.6 with every optimization except
-ftree-vectorize.  I'm sure it breaks a lot of stable code.  I was going to
just fix it myself and submit a patch, but the fact that it's even a question
whether or not this is a bug discourages me from spending time on it because I
know it'll just end up lost in some flame war.  I've jumped off this sinking
ship anyway... code works fine on llvm/clang.

[Bug tree-optimization/49413] over-optimization that causes valid code to segfault

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-04 09:35:13 UTC ---
There are two camps of users, some want the compiler to treat *(double *)
as aligned because, damn, they said so (especially on targets where
accessing unaligned data is costly)!  Others want the compiler to figure
out misalignment properly, thus, treat *(double *) as aligned only if
it knows (for sure?).

We can't serve both camps.

Consider

void foo (double *p, double *q, int n)
{
  int i;
  for (i = 0; i < n; ++i)
    p[i] += q[i];
}

do you want the compiler to vectorize the above or not?  The C standard
says we can assume *p and *q are properly aligned for what the ABI
says about doubles.  But you say, well, I may als well call this function
with some misaligned data where I think the compiler should figure this
out for me.

Now, I say not so.