[Bug other/48981] New: bootstrap-lto -O3 produces miscompiled, broken gcc

[Bug other/48981] New: bootstrap-lto -O3 produces miscompiled, broken gcc

[d.g.gorbachev at gmail dot com]

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

           Summary: bootstrap-lto -O3 produces miscompiled, broken gcc
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: other
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: d.g.gorbachev@gmail.com
                CC: rguenth@gcc.gnu.org
              Host: i686-pc-linux-gnu
            Target: i686-pc-linux-gnu
             Build: i686-pc-linux-gnu


GCC crashes because of violation of strict aliasing rules. The same memory
location is accessed in vec_gc_o_reserve_1 through (struct vec_prefix *)
pointer, then in VEC_tree_base_quick_push through (struct VEC_tree_base *).
Optimization moves the code around in a wrong way.

This patch fixes it:

--- gcc/vec.c
+++ gcc/vec.c
@@ -37,11 +37,11 @@
 struct vec_prefix
 {
   unsigned num;
   unsigned alloc;
   void *vec[1];
-};
+} __attribute__ ((may_alias));


 #ifdef GATHER_STATISTICS

 /* Store information about each particular vector.  */

[Bug other/48981] bootstrap-lto -O3 produces miscompiled, broken gcc

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-05-12 17:59:56 UTC ---
Created attachment 24239
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24239
Patch which changes how vec base is done

Can you try this patch?  I have not tested it and have not even tried compiling
it.

[Bug other/48981] bootstrap-lto -O3 produces miscompiled, broken gcc

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.05.13 08:36:35
     Ever Confirmed|0                           |1

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-05-13 08:36:35 UTC ---
Confirmed.  A patch similar to Andrews should work.

[Bug other/48981] bootstrap-lto -O3 produces miscompiled, broken gcc

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-05-13 21:22:18 UTC ---
(In reply to comment #2)
> Confirmed.  A patch similar to Andrews should work.

I will attach a patch which actually compiles :).

[Bug other/48981] bootstrap-lto -O3 produces miscompiled, broken gcc

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #24239|0                           |1
        is obsolete|                            |

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-05-13 21:23:56 UTC ---
Created attachment 24244
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24244
new patch which actually compiles but not has been fully tested

[Bug other/48981] bootstrap-lto -O3 produces miscompiled, broken gcc

[d.g.gorbachev at gmail dot com]

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

--- Comment #5 from Dmitry Gorbachev <d.g.gorbachev at gmail dot com> 2011-05-14 00:51:44 UTC ---
(In reply to comment #4)

Then this comment in gengtype.c becomes no longer valid, and confusing:

   /* Record the definition of a generic VEC structure, as if we had expanded
      the macros in vec.h:

>     typedef struct VEC_<type>_base GTY(()) {
>     unsigned num;
>     unsigned alloc;
> -   <type> GTY((length ("%h.num"))) vec[1];
> +   <type> GTY((length ("%h.prefix.num"))) vec[1];
>     } VEC_<type>_base
>
>     where the GTY(()) tags are only present if is_scalar is _false_.  */

In vec.h:

> +#ifndef GTY
> +#define GTY(x)  /* nothing - marker for gengtype */
> +#endif

It could be fixed by changing the #include order in vec.c:

 #include "system.h"
+#include "coretypes.h"
 #include "ggc.h"
 #include "vec.h"
-#include "coretypes.h"
 #include "diagnostic-core.h"
 #include "hashtab.h"

[Bug other/48981] bootstrap-lto -O3 produces miscompiled, broken gcc

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #24244|0                           |1
        is obsolete|                            |
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |pinskia at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-05-25 00:57:24 UTC ---
Created attachment 24352
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24352
new patch which I am testing right now

Here is the new patch which fixes up gengtype issue.  I am going to submit it
tomorrow after my testresults finish.

[Bug other/48981] bootstrap-lto -O3 produces miscompiled, broken gcc

[d.g.gorbachev at gmail dot com]

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

--- Comment #7 from Dmitry Gorbachev <d.g.gorbachev at gmail dot com> 2011-05-27 15:00:04 UTC ---
(In reply to comment #6)

I think there should be a space after "}" in vec.h:

> +}vec_prefix;

Thanks!

[Bug other/48981] bootstrap-lto -O3 produces miscompiled, broken gcc

[pinskia at gcc dot gnu.org]

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-05-27 20:19:16 UTC ---
Author: pinskia
Date: Fri May 27 20:19:13 2011
New Revision: 174359

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174359
Log:
2011-05-27  Andrew Pinski  <pinskia@gmail.com>

    PR middle-end/48981
    * gengtype.c (vec_prefix_type): New function.
    (note_def_vec): Use vec_prefix_type and change the length
    attribute to be based on the prefix.
    * vec.c: Include coretypes.h before vec.h.
    (struct vec_prefix): Remove.
    (vec_gc_p_reserve): Change the offsetof to sizeof.
    (vec_gc_p_reserve_exact): Likewise.
    (vec_heap_p_reserve): Likewise.
    (vec_heap_p_reserve_exact): Likewise.
    (vec_stack_o_reserve_1): Copy from +1 instead of from vec.
    (vec_stack_p_reserve): Change the offsetof to sizeof.
    (vec_stack_p_reserve_exact): Likewise.
    * vec.h (struct vec_prefix): New struct definition.
    (VEC_T(T,B)): Use vec_prefix instead of having num/alloc fields.
    (VEC_T_GTY(T,B)): Likewise.
    (DEF_VEC_FUNC_P(T)): Use prefix field.
    (DEF_VEC_NONALLOC_FUNCS_O(T,A)): Likewise.
    (DEF_VEC_NONALLOC_FUNCS_I(T,A)): Likewise.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gengtype.c
    trunk/gcc/vec.c
    trunk/gcc/vec.h

[Bug other/48981] bootstrap-lto -O3 produces miscompiled, broken gcc

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.7.0

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-05-27 20:20:19 UTC ---
Fixed.