Re: C++ PATCH for c++/80095, ICE with this pointer in NSDMI

[Jason Merrill <jason at redhat dot com>]

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