From: Marek Polacek <polacek@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: C++ PATCH for c++/80095, ICE with this pointer in NSDMI
Date: Wed, 05 Apr 2017 09:49:00 -0000 [thread overview]
Message-ID: <20170405094900.GC3172@redhat.com> (raw)
In-Reply-To: <20170329213239.GU3172@redhat.com>
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
next prev parent reply other threads:[~2017-04-05 9:49 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-29 17:08 Marek Polacek
2017-03-29 19:11 ` Jason Merrill
2017-03-29 21:51 ` Marek Polacek
2017-04-05 9:49 ` Marek Polacek [this message]
2017-04-07 17:03 ` Jason Merrill
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170405094900.GC3172@redhat.com \
--to=polacek@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).