On 7/13/21 8:15 PM, Marek Polacek wrote:
This PR gave me a hard time: I saw multiple issues starting with
different revisions.  But ultimately the root cause seems to be
the following, and the attached patch fixes all issues I've found
here.

In cxx_eval_array_reference we create a new constexpr context for the
CP_AGGREGATE_TYPE_P case, but we also have to create it for the
non-aggregate case.

But not for the scalar case, surely? Other similar places check AGGREGATE_TYPE_P || VECTOR_TYPE_P, or !SCALAR_TYPE_P.

In this test, we are evaluating

   ((B *)this)->a = rhs->a

which means that we set ctx.object to ((B *)this)->a.  Then we proceed
to evaluate the initializer, rhs->a.  For *rhs, we eval rhs, a PARM_DECL,
for which we have (const B &) &c.arr[0] in the hash table.  Then
cxx_fold_indirect_ref gives us c.arr[0].  c is evaluated to {.arr={}} so
c.arr is {}.  Now we want c.arr[0], so we end up in cxx_eval_array_reference
and since we're initializing from {}, we call build_value_init which
gives us an AGGR_INIT_EXPR that calls 'constexpr B::B()'.  Then we
evaluate this AGGR_INIT_EXPR and since its first argument is dummy,
we take ctx.object instead.  But that is the wrong object, we're not
initializing ((B *)this)->a here.  And so we wound up with an
initializer for A, and then crash in cxx_eval_component_reference:

   gcc_assert (DECL_CONTEXT (part) == TYPE_MAIN_VARIANT (TREE_TYPE (whole)));

where DECL_CONTEXT (part) is B (as it should be) but the type of whole
was A.

With that in mind, the fix is straightforward, except that when the
value-init produced an AGGR_INIT_EXPR, we shouldn't set ctx.object so
that

2508   if (DECL_CONSTRUCTOR_P (fun) && !ctx->object
2509       && TREE_CODE (t) == AGGR_INIT_EXPR)
2510     {
2511       /* We want to have an initialization target for an AGGR_INIT_EXPR.
2512          If we don't already have one in CTX, use the AGGR_INIT_EXPR_SLOT. 
 */
2513       new_ctx.object = AGGR_INIT_EXPR_SLOT (t);

comes into play.

Hmm, setting new_ctx.object to t here looks like it should be the correct c.arr[0], not ((B*)this)->a. It was wrong in the current code because we weren't setting up new_ctx at all, but once that's fixed I don't think you need special AGGR_INIT_EXPR handling.

Bootstrapped/regtested on {x86_64,ppc64le,aarch64}-pc-linux-gnu, ok for trunk?

        PR c++/101371

gcc/cp/ChangeLog:

        * constexpr.c (cxx_eval_array_reference): Create a new .object
        and .ctor for the non-aggregate case too when value-initializing.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp1y/constexpr-101371-2.C: New test.
        * g++.dg/cpp1y/constexpr-101371.C: New test.
---
  gcc/cp/constexpr.c                            | 15 ++++++----
  .../g++.dg/cpp1y/constexpr-101371-2.C         | 23 +++++++++++++++
  gcc/testsuite/g++.dg/cpp1y/constexpr-101371.C | 29 +++++++++++++++++++
  3 files changed, 61 insertions(+), 6 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-101371-2.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-101371.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 39787f3f5d5..584ef55703c 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3844,23 +3844,26 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, 
tree t,
       initializer, it's initialized from {}.  But use build_value_init
       directly for non-aggregates to avoid creating a garbage CONSTRUCTOR.  */
    tree val;
-  constexpr_ctx new_ctx;
    if (is_really_empty_class (elem_type, /*ignore_vptr*/false))
      return build_constructor (elem_type, NULL);
    else if (CP_AGGREGATE_TYPE_P (elem_type))
      {
        tree empty_ctor = build_constructor (init_list_type_node, NULL);
        val = digest_init (elem_type, empty_ctor, tf_warning_or_error);
-      new_ctx = *ctx;
-      new_ctx.object = t;
-      new_ctx.ctor = build_constructor (elem_type, NULL);
-      ctx = &new_ctx;
      }
    else
      val = build_value_init (elem_type, tf_warning_or_error);
+
+  constexpr_ctx new_ctx = *ctx;
+  /* If we are using an AGGR_INIT_EXPR, clear OBJECT for now so that
+     cxx_eval_call_expression can make use of AGGR_INIT_EXPR_SLOT.  */
+  new_ctx.object = (TREE_CODE (val) == AGGR_INIT_EXPR
+                   ? NULL_TREE : t);
+  new_ctx.ctor = build_constructor (elem_type, NULL);
+  ctx = &new_ctx;
    t = cxx_eval_constant_expression (ctx, val, lval, non_constant_p,
                                    overflow_p);
-  if (CP_AGGREGATE_TYPE_P (elem_type) && t != ctx->ctor)
+  if (t != ctx->ctor)
      free_constructor (ctx->ctor);
    return t;
  }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-101371-2.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-101371-2.C
new file mode 100644
index 00000000000..fb67b67c265
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-101371-2.C
@@ -0,0 +1,23 @@
+// PR c++/101371
+// { dg-do compile { target c++14 } }
+
+struct A {
+  int i;
+};
+struct B {
+  A a{};
+  constexpr B() : a() {}
+  constexpr B(const B &rhs) : a(rhs.a) {}
+};
+struct C {
+  B arr[1];
+};
+
+constexpr C
+fn ()
+{
+  C c{};
+  return c;
+}
+
+constexpr C c = fn();
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-101371.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-101371.C
new file mode 100644
index 00000000000..b6351b806b9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-101371.C
@@ -0,0 +1,29 @@
+// PR c++/101371
+// { dg-do compile { target c++14 } }
+
+struct A {
+  int i;
+};
+struct B {
+  A a{};
+  constexpr B() : a() {}
+  constexpr B(const B &rhs) : a(rhs.a) {}
+};
+struct C {
+  B arr[1];
+};
+
+struct X {
+  constexpr C fn () const
+  {
+    C c{};
+    return c;
+  }
+};
+
+void
+g ()
+{
+  X x;
+  constexpr auto z = x.fn();
+}

base-commit: 8695bf78dad1a42636775843ca832a2f4dba4da3


Reply via email to