| Message ID | 20170329213239.GU3172@redhat.com |
|---|---|
| State | New |
| Headers | show |
Ping. On Wed, Mar 29, 2017 at 11:32:39PM +0200, Marek Polacek wrote: > On Wed, Mar 29, 2017 at 02:56:51PM -0400, Jason Merrill wrote: > > On Wed, Mar 29, 2017 at 12:38 PM, Marek Polacek <polacek@redhat.com> wrote: > > > Here we have a reference initialization with NSDMI and *this. We are crashing > > > because a PLACEHOLDER_EXPR crept into the gimplifier. > > > > > > This happens since r218653 where set_up_extended_ref_temp was changed to > > > use split_nonconstant_init. As a consequence, cp_gimplify_init_expr might > > > now be receiving > > > > > > D.2051.p = (void *) &<PLACEHOLDER_EXPR struct A> > > > > > > instead of > > > > > > D.2051 = {.p = (void *) &<PLACEHOLDER_EXPR struct A>} > > > > > > where the RHS was a CONSTRUCTOR. It no longer is, so replace_placeholders is > > > not called anymore. It occurred to me that we should use the same check as in > > > store_init_value (i.e. check that the object to be used in the substitution is > > > a class), but given what split_nonconstant_init might produce, handle > > > COMPONENT_REFs specially. > > > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk and 6? > > > > > > 2017-03-29 Marek Polacek <polacek@redhat.com> > > > > > > PR c++/80095 - ICE with this pointer in NSDMI. > > > * cp-gimplify.c (cp_gimplify_init_expr): Call replace_placeholders > > > when TO is a class. > > > > > > * g++.dg/cpp1y/nsdmi-aggr8.C: New test. > > > > > > diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c > > > index 354ae1a..e530daf 100644 > > > --- gcc/cp/cp-gimplify.c > > > +++ gcc/cp/cp-gimplify.c > > > @@ -496,7 +496,16 @@ cp_gimplify_init_expr (tree *expr_p) > > > TREE_TYPE (from) = void_type_node; > > > } > > > > > > - if (cxx_dialect >= cxx14 && TREE_CODE (sub) == CONSTRUCTOR) > > > + /* split_nonconstant_init might've produced something like > > > + D.2051.p = (void *) &<PLACEHOLDER_EXPR struct A> > > > + in which case we want to substitute the placeholder with > > > + D.2051. */ > > > + tree op0 = to; > > > + while (TREE_CODE (op0) == COMPONENT_REF) > > > + op0 = TREE_OPERAND (op0, 0); > > > + tree type = TREE_TYPE (op0); > > > + > > > + if (cxx_dialect >= cxx14 && CLASS_TYPE_P (strip_array_types (type))) > > > > How about doing this checking in replace_placeholders, instead? That > > is, if the object isn't a (member of a) class, just return. > > Sure. I also moved the C++14 check into replace_placeholders, although maybe > I shouldn't have. > > Bootstrapped/regtested on x86_64-linux, ok for trunk and 6? > > 2017-03-29 Marek Polacek <polacek@redhat.com> > > PR c++/80095 > * call.c (build_over_call): Don't check cxx_dialect. > * cp-gimplify.c (cp_gimplify_init_expr): Don't check cxx_dialect nor > whether SUB is a CONSTRUCTOR. > * init.c (build_new_1): Don't check cxx_dialect. > * tree.c (replace_placeholders): Add a function comment. Return if > not in C++14, or if the object isn't a (member of a) class. > * typeck2.c (store_init_value): Don't check cxx_dialect nor whether > TYPE is CLASS_TYPE_P. > > * g++.dg/cpp1y/nsdmi-aggr8.C: New test. > > diff --git gcc/cp/call.c gcc/cp/call.c > index 803fbd4..c15b8e4 100644 > --- gcc/cp/call.c > +++ gcc/cp/call.c > @@ -8047,9 +8047,8 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) > { > arg = cp_build_indirect_ref (arg, RO_NULL, complain); > val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); > - if (cxx_dialect >= cxx14) > - /* Handle NSDMI that refer to the object being initialized. */ > - replace_placeholders (arg, to); > + /* Handle NSDMI that refer to the object being initialized. */ > + replace_placeholders (arg, to); > } > else > { > diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c > index 354ae1a..3f91c35 100644 > --- gcc/cp/cp-gimplify.c > +++ gcc/cp/cp-gimplify.c > @@ -496,9 +496,8 @@ cp_gimplify_init_expr (tree *expr_p) > TREE_TYPE (from) = void_type_node; > } > > - if (cxx_dialect >= cxx14 && TREE_CODE (sub) == CONSTRUCTOR) > - /* Handle aggregate NSDMI. */ > - replace_placeholders (sub, to); > + /* Handle aggregate NSDMI. */ > + replace_placeholders (sub, to); > > if (t == sub) > break; > diff --git gcc/cp/init.c gcc/cp/init.c > index 7732795..6a70955 100644 > --- gcc/cp/init.c > +++ gcc/cp/init.c > @@ -3373,8 +3373,7 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, > object being initialized, replace them now and don't try to > preevaluate. */ > bool had_placeholder = false; > - if (cxx_dialect >= cxx14 > - && !processing_template_decl > + if (!processing_template_decl > && TREE_CODE (init_expr) == INIT_EXPR) > TREE_OPERAND (init_expr, 1) > = replace_placeholders (TREE_OPERAND (init_expr, 1), > diff --git gcc/cp/tree.c gcc/cp/tree.c > index 2757af6..9939135 100644 > --- gcc/cp/tree.c > +++ gcc/cp/tree.c > @@ -2813,9 +2813,23 @@ replace_placeholders_r (tree* t, int* walk_subtrees, void* data_) > return NULL_TREE; > } > > +/* Replace PLACEHOLDER_EXPRs in EXP with object OBJ. SEEN_P is set if > + a PLACEHOLDER_EXPR has been encountered. */ > + > tree > replace_placeholders (tree exp, tree obj, bool *seen_p) > { > + /* This is only relevant for C++14. */ > + if (cxx_dialect < cxx14) > + return exp; > + > + /* If the object isn't a (member of a) class, do nothing. */ > + tree op0 = obj; > + while (TREE_CODE (op0) == COMPONENT_REF) > + op0 = TREE_OPERAND (op0, 0); > + if (!CLASS_TYPE_P (strip_array_types (TREE_TYPE (op0)))) > + return exp; > + > tree *tp = &exp; > replace_placeholders_t data = { obj, false }; > if (TREE_CODE (exp) == TARGET_EXPR) > diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c > index 58a01c9..67fbaea 100644 > --- gcc/cp/typeck2.c > +++ gcc/cp/typeck2.c > @@ -836,9 +836,8 @@ store_init_value (tree decl, tree init, vec<tree, va_gc>** cleanups, int flags) > } > value = cp_fully_fold (value); > > - if (cxx_dialect >= cxx14 && CLASS_TYPE_P (strip_array_types (type))) > - /* Handle aggregate NSDMI in non-constant initializers, too. */ > - value = replace_placeholders (value, decl); > + /* Handle aggregate NSDMI in non-constant initializers, too. */ > + value = replace_placeholders (value, decl); > > /* DECL may change value; purge caches. */ > clear_cv_and_fold_caches (); > diff --git gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C > index e69de29..8c99ffb 100644 > --- gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C > +++ gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C > @@ -0,0 +1,16 @@ > +// PR c++/80095 > +// { dg-do compile { target c++14 } } > + > +struct A > +{ > + void* p = this; > +}; > + > +void > +foo () > +{ > + const A& a = A{}; > + A&& a2 = A{}; > + const A& a3{}; > + A&& a4{}; > +} > > Marek Marek
OK, thanks. On Wed, Apr 5, 2017 at 5:49 AM, Marek Polacek <polacek@redhat.com> wrote: > Ping. > > On Wed, Mar 29, 2017 at 11:32:39PM +0200, Marek Polacek wrote: >> On Wed, Mar 29, 2017 at 02:56:51PM -0400, Jason Merrill wrote: >> > On Wed, Mar 29, 2017 at 12:38 PM, Marek Polacek <polacek@redhat.com> wrote: >> > > Here we have a reference initialization with NSDMI and *this. We are crashing >> > > because a PLACEHOLDER_EXPR crept into the gimplifier. >> > > >> > > This happens since r218653 where set_up_extended_ref_temp was changed to >> > > use split_nonconstant_init. As a consequence, cp_gimplify_init_expr might >> > > now be receiving >> > > >> > > D.2051.p = (void *) &<PLACEHOLDER_EXPR struct A> >> > > >> > > instead of >> > > >> > > D.2051 = {.p = (void *) &<PLACEHOLDER_EXPR struct A>} >> > > >> > > where the RHS was a CONSTRUCTOR. It no longer is, so replace_placeholders is >> > > not called anymore. It occurred to me that we should use the same check as in >> > > store_init_value (i.e. check that the object to be used in the substitution is >> > > a class), but given what split_nonconstant_init might produce, handle >> > > COMPONENT_REFs specially. >> > > >> > > Bootstrapped/regtested on x86_64-linux, ok for trunk and 6? >> > > >> > > 2017-03-29 Marek Polacek <polacek@redhat.com> >> > > >> > > PR c++/80095 - ICE with this pointer in NSDMI. >> > > * cp-gimplify.c (cp_gimplify_init_expr): Call replace_placeholders >> > > when TO is a class. >> > > >> > > * g++.dg/cpp1y/nsdmi-aggr8.C: New test. >> > > >> > > diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c >> > > index 354ae1a..e530daf 100644 >> > > --- gcc/cp/cp-gimplify.c >> > > +++ gcc/cp/cp-gimplify.c >> > > @@ -496,7 +496,16 @@ cp_gimplify_init_expr (tree *expr_p) >> > > TREE_TYPE (from) = void_type_node; >> > > } >> > > >> > > - if (cxx_dialect >= cxx14 && TREE_CODE (sub) == CONSTRUCTOR) >> > > + /* split_nonconstant_init might've produced something like >> > > + D.2051.p = (void *) &<PLACEHOLDER_EXPR struct A> >> > > + in which case we want to substitute the placeholder with >> > > + D.2051. */ >> > > + tree op0 = to; >> > > + while (TREE_CODE (op0) == COMPONENT_REF) >> > > + op0 = TREE_OPERAND (op0, 0); >> > > + tree type = TREE_TYPE (op0); >> > > + >> > > + if (cxx_dialect >= cxx14 && CLASS_TYPE_P (strip_array_types (type))) >> > >> > How about doing this checking in replace_placeholders, instead? That >> > is, if the object isn't a (member of a) class, just return. >> >> Sure. I also moved the C++14 check into replace_placeholders, although maybe >> I shouldn't have. >> >> Bootstrapped/regtested on x86_64-linux, ok for trunk and 6? >> >> 2017-03-29 Marek Polacek <polacek@redhat.com> >> >> PR c++/80095 >> * call.c (build_over_call): Don't check cxx_dialect. >> * cp-gimplify.c (cp_gimplify_init_expr): Don't check cxx_dialect nor >> whether SUB is a CONSTRUCTOR. >> * init.c (build_new_1): Don't check cxx_dialect. >> * tree.c (replace_placeholders): Add a function comment. Return if >> not in C++14, or if the object isn't a (member of a) class. >> * typeck2.c (store_init_value): Don't check cxx_dialect nor whether >> TYPE is CLASS_TYPE_P. >> >> * g++.dg/cpp1y/nsdmi-aggr8.C: New test. >> >> diff --git gcc/cp/call.c gcc/cp/call.c >> index 803fbd4..c15b8e4 100644 >> --- gcc/cp/call.c >> +++ gcc/cp/call.c >> @@ -8047,9 +8047,8 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) >> { >> arg = cp_build_indirect_ref (arg, RO_NULL, complain); >> val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); >> - if (cxx_dialect >= cxx14) >> - /* Handle NSDMI that refer to the object being initialized. */ >> - replace_placeholders (arg, to); >> + /* Handle NSDMI that refer to the object being initialized. */ >> + replace_placeholders (arg, to); >> } >> else >> { >> diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c >> index 354ae1a..3f91c35 100644 >> --- gcc/cp/cp-gimplify.c >> +++ gcc/cp/cp-gimplify.c >> @@ -496,9 +496,8 @@ cp_gimplify_init_expr (tree *expr_p) >> TREE_TYPE (from) = void_type_node; >> } >> >> - if (cxx_dialect >= cxx14 && TREE_CODE (sub) == CONSTRUCTOR) >> - /* Handle aggregate NSDMI. */ >> - replace_placeholders (sub, to); >> + /* Handle aggregate NSDMI. */ >> + replace_placeholders (sub, to); >> >> if (t == sub) >> break; >> diff --git gcc/cp/init.c gcc/cp/init.c >> index 7732795..6a70955 100644 >> --- gcc/cp/init.c >> +++ gcc/cp/init.c >> @@ -3373,8 +3373,7 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, >> object being initialized, replace them now and don't try to >> preevaluate. */ >> bool had_placeholder = false; >> - if (cxx_dialect >= cxx14 >> - && !processing_template_decl >> + if (!processing_template_decl >> && TREE_CODE (init_expr) == INIT_EXPR) >> TREE_OPERAND (init_expr, 1) >> = replace_placeholders (TREE_OPERAND (init_expr, 1), >> diff --git gcc/cp/tree.c gcc/cp/tree.c >> index 2757af6..9939135 100644 >> --- gcc/cp/tree.c >> +++ gcc/cp/tree.c >> @@ -2813,9 +2813,23 @@ replace_placeholders_r (tree* t, int* walk_subtrees, void* data_) >> return NULL_TREE; >> } >> >> +/* Replace PLACEHOLDER_EXPRs in EXP with object OBJ. SEEN_P is set if >> + a PLACEHOLDER_EXPR has been encountered. */ >> + >> tree >> replace_placeholders (tree exp, tree obj, bool *seen_p) >> { >> + /* This is only relevant for C++14. */ >> + if (cxx_dialect < cxx14) >> + return exp; >> + >> + /* If the object isn't a (member of a) class, do nothing. */ >> + tree op0 = obj; >> + while (TREE_CODE (op0) == COMPONENT_REF) >> + op0 = TREE_OPERAND (op0, 0); >> + if (!CLASS_TYPE_P (strip_array_types (TREE_TYPE (op0)))) >> + return exp; >> + >> tree *tp = &exp; >> replace_placeholders_t data = { obj, false }; >> if (TREE_CODE (exp) == TARGET_EXPR) >> diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c >> index 58a01c9..67fbaea 100644 >> --- gcc/cp/typeck2.c >> +++ gcc/cp/typeck2.c >> @@ -836,9 +836,8 @@ store_init_value (tree decl, tree init, vec<tree, va_gc>** cleanups, int flags) >> } >> value = cp_fully_fold (value); >> >> - if (cxx_dialect >= cxx14 && CLASS_TYPE_P (strip_array_types (type))) >> - /* Handle aggregate NSDMI in non-constant initializers, too. */ >> - value = replace_placeholders (value, decl); >> + /* Handle aggregate NSDMI in non-constant initializers, too. */ >> + value = replace_placeholders (value, decl); >> >> /* DECL may change value; purge caches. */ >> clear_cv_and_fold_caches (); >> diff --git gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C >> index e69de29..8c99ffb 100644 >> --- gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C >> +++ gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C >> @@ -0,0 +1,16 @@ >> +// PR c++/80095 >> +// { dg-do compile { target c++14 } } >> + >> +struct A >> +{ >> + void* p = this; >> +}; >> + >> +void >> +foo () >> +{ >> + const A& a = A{}; >> + A&& a2 = A{}; >> + const A& a3{}; >> + A&& a4{}; >> +} >> >> Marek > > Marek
diff --git gcc/cp/call.c gcc/cp/call.c index 803fbd4..c15b8e4 100644 --- gcc/cp/call.c +++ gcc/cp/call.c @@ -8047,9 +8047,8 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) { arg = cp_build_indirect_ref (arg, RO_NULL, complain); val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); - if (cxx_dialect >= cxx14) - /* Handle NSDMI that refer to the object being initialized. */ - replace_placeholders (arg, to); + /* Handle NSDMI that refer to the object being initialized. */ + replace_placeholders (arg, to); } else { diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c index 354ae1a..3f91c35 100644 --- gcc/cp/cp-gimplify.c +++ gcc/cp/cp-gimplify.c @@ -496,9 +496,8 @@ cp_gimplify_init_expr (tree *expr_p) TREE_TYPE (from) = void_type_node; } - if (cxx_dialect >= cxx14 && TREE_CODE (sub) == CONSTRUCTOR) - /* Handle aggregate NSDMI. */ - replace_placeholders (sub, to); + /* Handle aggregate NSDMI. */ + replace_placeholders (sub, to); if (t == sub) break; diff --git gcc/cp/init.c gcc/cp/init.c index 7732795..6a70955 100644 --- gcc/cp/init.c +++ gcc/cp/init.c @@ -3373,8 +3373,7 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, object being initialized, replace them now and don't try to preevaluate. */ bool had_placeholder = false; - if (cxx_dialect >= cxx14 - && !processing_template_decl + if (!processing_template_decl && TREE_CODE (init_expr) == INIT_EXPR) TREE_OPERAND (init_expr, 1) = replace_placeholders (TREE_OPERAND (init_expr, 1), diff --git gcc/cp/tree.c gcc/cp/tree.c index 2757af6..9939135 100644 --- gcc/cp/tree.c +++ gcc/cp/tree.c @@ -2813,9 +2813,23 @@ replace_placeholders_r (tree* t, int* walk_subtrees, void* data_) return NULL_TREE; } +/* Replace PLACEHOLDER_EXPRs in EXP with object OBJ. SEEN_P is set if + a PLACEHOLDER_EXPR has been encountered. */ + tree replace_placeholders (tree exp, tree obj, bool *seen_p) { + /* This is only relevant for C++14. */ + if (cxx_dialect < cxx14) + return exp; + + /* If the object isn't a (member of a) class, do nothing. */ + tree op0 = obj; + while (TREE_CODE (op0) == COMPONENT_REF) + op0 = TREE_OPERAND (op0, 0); + if (!CLASS_TYPE_P (strip_array_types (TREE_TYPE (op0)))) + return exp; + tree *tp = &exp; replace_placeholders_t data = { obj, false }; if (TREE_CODE (exp) == TARGET_EXPR) diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c index 58a01c9..67fbaea 100644 --- gcc/cp/typeck2.c +++ gcc/cp/typeck2.c @@ -836,9 +836,8 @@ store_init_value (tree decl, tree init, vec<tree, va_gc>** cleanups, int flags) } value = cp_fully_fold (value); - if (cxx_dialect >= cxx14 && CLASS_TYPE_P (strip_array_types (type))) - /* Handle aggregate NSDMI in non-constant initializers, too. */ - value = replace_placeholders (value, decl); + /* Handle aggregate NSDMI in non-constant initializers, too. */ + value = replace_placeholders (value, decl); /* DECL may change value; purge caches. */ clear_cv_and_fold_caches (); diff --git gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C index e69de29..8c99ffb 100644 --- gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C +++ gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C @@ -0,0 +1,16 @@ +// PR c++/80095 +// { dg-do compile { target c++14 } } + +struct A +{ + void* p = this; +}; + +void +foo () +{ + const A& a = A{}; + A&& a2 = A{}; + const A& a3{}; + A&& a4{}; +}