On Thu, Jun 5, 2014 at 2:59 PM, Paolo Carlini <paolo.carl...@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>>>> > > >