[C++ RFH] PR 56961

[Richard Biener]

On Thu, Jun 5, 2014 at 2:59 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> in this minor issue, after a permerror about "passing ‘volatile foo’ as
> ‘this’ argument discards qualifiers" we crash with an infinite recursion in
> the gimplifier. The testcase:
>
> struct foo { };
>
> typedef struct
> {
> volatile foo fields;
> } CSPHandleState;
>
> CSPHandleState a;
>
> void fn1 ()
> {
> CSPHandleState b;
> b.fields = foo();
> }
>
> involves the empty struct foo, and I noticed that the crash doesn't happen
> otherwise. Therefore this comment in cp-gimplify.c seems relevant:
>
> 624 /* Remove any copies of empty classes. We check that the RHS
> 625 has a simple form so that TARGET_EXPRs and non-empty
> 626 CONSTRUCTORs get reduced properly, and we leave the return
> 627 slot optimization alone because it isn't a copy (FIXME so it
> 628 shouldn't be represented as one).
> 629
> 630 Also drop volatile variables on the RHS to avoid infinite
> 631 recursion from gimplify_expr trying to load the value. */
> 632 if (!TREE_SIDE_EFFECTS (op1)
> 633 || (DECL_P (op1) && TREE_THIS_VOLATILE (op1)))
> 634 *expr_p = op0;
> 635 else if (TREE_CODE (op1) == MEM_REF
> 636 && TREE_THIS_VOLATILE (op1))
> 637 {
> 638 /* Similarly for volatile MEM_REFs on the RHS. */
>
> and in fact we get there, op1 is volatile, but we don't adjust things
> because op1 is a COMPONENT_REF, not a decl, not a MEM_REF. Then I'm
> wondering if we should handle in the same place the COMPONENT_REF case?!?
> Eg, brutally hacking the above to handle a COMPONENT_REF like a DECL_P
> avoids the infinite recursion. The below is *expr_p (its arg0 is op0 and its
> arg1 is op1). Hints?

See my comment in the PR.  You need to handle all references/decls
here but preserve side-effects in operands of references.  For example
by gimplifying as unused address (just an idea):

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),

Richard.


> Thanks!
> Paolo.
>
> ////////////////////////
>
> <modify_expr 0x7ffff682d668
> type <record_type 0x7ffff6835150 foo sizes-gimplified type_5 type_6 QI
> size <integer_cst 0x7ffff66d5918 constant 8>
> unit size <integer_cst 0x7ffff66d5930 constant 1>
> align 8 symtab 0 alias set -1 canonical type 0x7ffff6835150
> fields <type_decl 0x7ffff682e958 foo type <record_type 0x7ffff68351f8 foo>
> nonlocal decl_4 VOID file 56961.C line 1 col 12
> align 1 context <record_type 0x7ffff6835150 foo> result <record_type
> 0x7ffff6835150 foo>
>> context <translation_unit_decl 0x7ffff66e0170 D.1>
> full-name "struct foo"
> X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown
> pointer_to_this <pointer_type 0x7ffff6835930> reference_to_this
> <reference_type 0x7ffff6835690> chain <type_decl 0x7ffff682e8a0 foo>>
> side-effects
> arg 0 <var_decl 0x7ffff683a7b8 vol.0 type <record_type 0x7ffff6835150 foo>
> used ignored QI file 56961.C line 13 col 19 size <integer_cst 0x7ffff66d5918
> 8> unit size <integer_cst 0x7ffff66d5930 1>
> align 8 context <function_decl 0x7ffff6834b00 fn1>>
> arg 1 <component_ref 0x7ffff66d8870
> type <record_type 0x7ffff6835498 foo sizes-gimplified volatile type_5 type_6
> QI size <integer_cst 0x7ffff66d5918 8> unit size <integer_cst 0x7ffff66d5930
> 1>
> align 8 symtab 0 alias set -1 canonical type 0x7ffff6835498 fields
> <type_decl 0x7ffff682e958 foo> context <translation_unit_decl 0x7ffff66e0170
> D.1>
> full-name "volatile struct foo"
> X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown
> pointer_to_this <pointer_type 0x7ffff6835b28>>
> side-effects volatile
> arg 0 <var_decl 0x7ffff683a558 b type <record_type 0x7ffff6835348
> CSPHandleState>
> addressable used tree_1 decl_5 QI file 56961.C line 12 col 18 size
> <integer_cst 0x7ffff66d5918 8> unit size <integer_cst 0x7ffff66d5930 1>
> align 8 context <function_decl 0x7ffff6834b00 fn1>>
> arg 1 <field_decl 0x7ffff683a390 fields type <record_type 0x7ffff6835498
> foo>
> side-effects volatile used nonlocal decl_3 QI file 56961.C line 5 col 16
> size <integer_cst 0x7ffff66d5918 8> unit size <integer_cst 0x7ffff66d5930 1>
> align 8 offset_align 128
> offset <integer_cst 0x7ffff66d5858 constant 0>
> bit offset <integer_cst 0x7ffff66d58a0 constant 0> context <record_type
> 0x7ffff6835348 CSPHandleState> chain <type_decl 0x7ffff682eac8 ._0>>>>
>
>
>