[Bug c++/56961] New: cycle between do_spec_1() / process_brace_body() / handle_braces()

[Bug c++/56961] New: cycle between do_spec_1() / process_brace_body() / handle_braces()

[dimhen at gmail dot com]

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

             Bug #: 56961
           Summary: cycle between do_spec_1() / process_brace_body() /
                    handle_braces()
    Classification: Unclassified
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: dimhen@gmail.com


$ cat x.cpp 
struct foo { };
  typedef struct
  {
      volatile foo fields;
  }
  CSPHandleState;
  CSPHandleState a;
  void
  fn1 ()
  {
      CSPHandleState b;
      b.fields = foo();
  }


$ g++ -O0 -c x.cpp 
x.cpp: In function 'void fn1()':
x.cpp:12:16: error: passing 'volatile foo' as 'this' argument of 'foo&
foo::operator=(const foo&)' discards qualifiers [-fpermissive]
       b.fields = foo();
                ^
g++: internal compiler error: Segmentation fault (program cc1plus)
0x40950c execute
    /home/dimhen/src/gcc-current/gcc/gcc.c:2822
0x409769 do_spec_1
    /home/dimhen/src/gcc-current/gcc/gcc.c:4614
0x40c4d9 process_brace_body
    /home/dimhen/src/gcc-current/gcc/gcc.c:5871
0x40c4d9 handle_braces
    /home/dimhen/src/gcc-current/gcc/gcc.c:5785
0x40a1fa do_spec_1
    /home/dimhen/src/gcc-current/gcc/gcc.c:5268
0x40c4d9 process_brace_body
    /home/dimhen/src/gcc-current/gcc/gcc.c:5871
0x40c4d9 handle_braces
    /home/dimhen/src/gcc-current/gcc/gcc.c:5785
0x40a1fa do_spec_1
    /home/dimhen/src/gcc-current/gcc/gcc.c:5268
0x40a5eb do_spec_1
    /home/dimhen/src/gcc-current/gcc/gcc.c:5373
0x40c4d9 process_brace_body
    /home/dimhen/src/gcc-current/gcc/gcc.c:5871
0x40c4d9 handle_braces
    /home/dimhen/src/gcc-current/gcc/gcc.c:5785
0x40a1fa do_spec_1
    /home/dimhen/src/gcc-current/gcc/gcc.c:5268
0x40c4d9 process_brace_body
    /home/dimhen/src/gcc-current/gcc/gcc.c:5871
0x40c4d9 handle_braces
    /home/dimhen/src/gcc-current/gcc/gcc.c:5785
0x40a1fa do_spec_1
    /home/dimhen/src/gcc-current/gcc/gcc.c:5268
0x40c4d9 process_brace_body
    /home/dimhen/src/gcc-current/gcc/gcc.c:5871
0x40c4d9 handle_braces
    /home/dimhen/src/gcc-current/gcc/gcc.c:5785
0x40a1fa do_spec_1
    /home/dimhen/src/gcc-current/gcc/gcc.c:5268
0x40c4d9 process_brace_body
    /home/dimhen/src/gcc-current/gcc/gcc.c:5871
0x40c4d9 handle_braces
    /home/dimhen/src/gcc-current/gcc/gcc.c:5785
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.

Thanks to David Blaikie for reduced testcase
(http://lists.cs.uiuc.edu/pipermail/cfe-users/2013-April/000085.html)

[Bug c++/56961] Segfault with volatile field

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|ICE: cycle between          |Segfault with volatile
                   |do_spec_1() /               |field
                   |process_brace_body() /      |
                   |handle_braces()             |

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2013-04-15 07:45:37 UTC ---
Note the backtrace that is provided is the one from gcc where it is executing
cc1plus which is nothing special.

[Bug c++/56961] stack overflow in gimplifier with volatile field

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ice-on-invalid-code
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-04-15
            Summary|Segfault with volatile      |stack overflow in
                   |field                       |gimplifier with volatile
                   |                            |field
     Ever Confirmed|0                           |1
      Known to fail|                            |4.7.0, 4.8.0, 4.9.0

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> 2013-04-15 07:50:53 UTC ---
Confirmed.

[Bug c++/56961] stack overflow in gimplifier with volatile field

[dimhen at gmail dot com]

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

--- Comment #3 from Dmitry G. Dyachenko <dimhen at gmail dot com> 2013-04-15 08:01:24 UTC ---
gcc-4.7.2 / Fedora 18 FAIL too

gcc version 4.7.2 20121109 (Red Hat 4.7.2-8) (GCC)

[Bug c++/56961] stack overflow in gimplifier with volatile field

[dimhen at gmail dot com]

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

--- Comment #4 from Dmitry G. Dyachenko <dimhen at gmail dot com> ---
FAIL
gcc version 4.9.0 20140106 (experimental) [trunk revision 206356] (GCC)

[Bug c++/56961] stack overflow in gimplifier with volatile field

[paolo.carlini at oracle dot com]

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

--- Comment #5 from Paolo Carlini <paolo.carlini at oracle dot com> ---
I think this is another manifestation of the issue discussed in cp-gimplify.c:

        /* Remove any copies of empty classes.  We check that the RHS
           has a simple form so that TARGET_EXPRs and non-empty
           CONSTRUCTORs get reduced properly, and we leave the return
           slot optimization alone because it isn't a copy (FIXME so it
           shouldn't be represented as one).

           Also drop volatile variables on the RHS to avoid infinite
           recursion from gimplify_expr trying to load the value.  */

note that it happens only when struct foo is empty...

[Bug c++/56961] stack overflow in gimplifier with volatile field

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
The C++ gimplify langhook gimplifies the gimplifier generated

  vol.1 = b.fields

as

  vol.1, b.fields;

so it isn't a manifestation of the issue but a fallout of the attempt to
fix it in the C++ gimplifier...  it drops the assignment but leaves the
load from the volatile in place (as side-effect to be preserved?).  But
then we gimplify that again ... producing vol.2 = b.fields ... and on we go.

[Bug c++/56961] stack overflow in gimplifier with volatile field

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
I suggest

Index: gcc/cp/cp-gimplify.c
===================================================================
--- gcc/cp/cp-gimplify.c        (revision 211262)
+++ gcc/cp/cp-gimplify.c        (working copy)
@@ -630,18 +630,8 @@ cp_gimplify_expr (tree *expr_p, gimple_s
               Also drop volatile variables on the RHS to avoid infinite
               recursion from gimplify_expr trying to load the value.  */
            if (!TREE_SIDE_EFFECTS (op1)
-               || (DECL_P (op1) && TREE_THIS_VOLATILE (op1)))
+               || TREE_THIS_VOLATILE (op1))
              *expr_p = op0;
-           else if (TREE_CODE (op1) == MEM_REF
-                    && TREE_THIS_VOLATILE (op1))
-             {
-               /* Similarly for volatile MEM_REFs on the RHS.  */
-               if (!TREE_SIDE_EFFECTS (TREE_OPERAND (op1, 0)))
-                 *expr_p = op0;
-               else
-                 *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p),
-                                   TREE_OPERAND (op1, 0), op0);
-             }
            else
              *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p),
                                op0, op1);

[Bug c++/56961] stack overflow in gimplifier with volatile field

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> I suggest
> 
> Index: gcc/cp/cp-gimplify.c
> ===================================================================
> --- gcc/cp/cp-gimplify.c        (revision 211262)
> +++ gcc/cp/cp-gimplify.c        (working copy)
> @@ -630,18 +630,8 @@ cp_gimplify_expr (tree *expr_p, gimple_s
>                Also drop volatile variables on the RHS to avoid infinite
>                recursion from gimplify_expr trying to load the value.  */
>             if (!TREE_SIDE_EFFECTS (op1)
> -               || (DECL_P (op1) && TREE_THIS_VOLATILE (op1)))
> +               || TREE_THIS_VOLATILE (op1))
>               *expr_p = op0;
> -           else if (TREE_CODE (op1) == MEM_REF
> -                    && TREE_THIS_VOLATILE (op1))
> -             {
> -               /* Similarly for volatile MEM_REFs on the RHS.  */
> -               if (!TREE_SIDE_EFFECTS (TREE_OPERAND (op1, 0)))
> -                 *expr_p = op0;
> -               else
> -                 *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p),
> -                                   TREE_OPERAND (op1, 0), op0);
> -             }
>             else
>               *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p),
>                                 op0, op1);

This might of course drop arbitrary side-effects of op1 such as
a volatile load from a.b[foo()] where foo () would be dropped.

So it isn't really correct.

[Bug c++/56961] stack overflow in gimplifier with volatile field

[rguenth at gcc dot gnu.org]

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

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #8)
> (In reply to Richard Biener from comment #7)
> > I suggest
> > 
> > Index: gcc/cp/cp-gimplify.c
> > ===================================================================
> > --- gcc/cp/cp-gimplify.c        (revision 211262)
> > +++ gcc/cp/cp-gimplify.c        (working copy)
> > @@ -630,18 +630,8 @@ cp_gimplify_expr (tree *expr_p, gimple_s
> >                Also drop volatile variables on the RHS to avoid infinite
> >                recursion from gimplify_expr trying to load the value.  */
> >             if (!TREE_SIDE_EFFECTS (op1)
> > -               || (DECL_P (op1) && TREE_THIS_VOLATILE (op1)))
> > +               || TREE_THIS_VOLATILE (op1))
> >               *expr_p = op0;
> > -           else if (TREE_CODE (op1) == MEM_REF
> > -                    && TREE_THIS_VOLATILE (op1))
> > -             {
> > -               /* Similarly for volatile MEM_REFs on the RHS.  */
> > -               if (!TREE_SIDE_EFFECTS (TREE_OPERAND (op1, 0)))
> > -                 *expr_p = op0;
> > -               else
> > -                 *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p),
> > -                                   TREE_OPERAND (op1, 0), op0);
> > -             }
> >             else
> >               *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p),
> >                                 op0, op1);
> 
> This might of course drop arbitrary side-effects of op1 such as
> a volatile load from a.b[foo()] where foo () would be dropped.
> 
> So it isn't really correct.

The following should preserve those.

Index: gcc/cp/cp-gimplify.c
===================================================================
--- gcc/cp/cp-gimplify.c        (revision 211262)
+++ gcc/cp/cp-gimplify.c        (working copy)
@@ -629,18 +629,14 @@ cp_gimplify_expr (tree *expr_p, gimple_s

               Also drop volatile variables on the RHS to avoid infinite
               recursion from gimplify_expr trying to load the value.  */
-           if (!TREE_SIDE_EFFECTS (op1)
-               || (DECL_P (op1) && TREE_THIS_VOLATILE (op1)))
+           if (!TREE_SIDE_EFFECTS (op1))
              *expr_p = op0;
-           else if (TREE_CODE (op1) == MEM_REF
-                    && TREE_THIS_VOLATILE (op1))
+           else if (TREE_THIS_VOLATILE (op1)
+                    && (REFERENCE_CLASS_P (op1) || DECL_P (op1)))
              {
-               /* Similarly for volatile MEM_REFs on the RHS.  */
-               if (!TREE_SIDE_EFFECTS (TREE_OPERAND (op1, 0)))
-                 *expr_p = op0;
-               else
-                 *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p),
-                                   TREE_OPERAND (op1, 0), op0);
+               *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p),
+                                 op0, build_fold_addr_expr (op1));
+
              }
            else
              *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p),

[Bug c++/56961] stack overflow in gimplifier with volatile field

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |paolo.carlini at oracle dot com
   Target Milestone|---                         |4.10.0

--- Comment #10 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Handling it.

[Bug c++/56961] stack overflow in gimplifier with volatile field

[paolo at gcc dot gnu.org]

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

--- Comment #11 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> ---
Author: paolo
Date: Thu Jun  5 17:34:48 2014
New Revision: 211286

URL: http://gcc.gnu.org/viewcvs?rev=211286&root=gcc&view=rev
Log:
/cp
2014-06-05  Richard Biener  <rguenther@suse.de>
        Paolo Carlini  <paolo.carlini@oracle.com>

    PR c++/56961
    * cp-gimplify.c (cp_gimplify_expr, [MODIFY_EXPR]): Rework
    handling of empty classes.

/testsuite
2014-06-05  Richard Biener  <rguenther@suse.de>
        Paolo Carlini  <paolo.carlini@oracle.com>

    PR c++/56961
    * g++.dg/parse/pr56961.C: New.

Added:
    trunk/gcc/testsuite/g++.dg/parse/pr56961.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/cp-gimplify.c
    trunk/gcc/testsuite/ChangeLog

[Bug c++/56961] stack overflow in gimplifier with volatile field

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
           Assignee|paolo.carlini at oracle dot com    |unassigned at gcc dot gnu.org

--- Comment #12 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Fixed for 4.10.0.