On 8/3/20 2:45 PM, Patrick Palka wrote:
On Mon, 3 Aug 2020, Jason Merrill wrote:
On 8/3/20 8:53 AM, Patrick Palka wrote:
On Mon, 3 Aug 2020, Patrick Palka wrote:
In the first testcase below, expand_aggr_init_1 sets up t's default
constructor such that the ctor first zero-initializes the entire base b,
followed by calling b's default constructor, the latter of which just
default-initializes the array member b::m via a VEC_INIT_EXPR.
So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is
nonempty due to the prior zero-initialization, and we proceed in
cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor
without first checking if a matching constructor_elt already exists.
This leads to ctx->ctor having two matching constructor_elts for each
index.
This patch partially fixes this issue by making the RANGE_EXPR
optimization in cxx_eval_vec_init truncate ctx->ctor before adding the
single RANGE_EXPR constructor_elt. This isn't a complete fix because
the RANGE_EXPR optimization applies only when the constant initializer
is relocatable, so whenever it's not relocatable we can still build up
an invalid CONSTRUCTOR, e.g. if in the first testcase we add an NSDMI
such as 'e *p = this;' to struct e, then the ICE still occurs even with
this patch.
A complete but more risky one-line fix would be to always truncate
ctx->ctor beforehand, not just when the RANGE_EXPR optimization applies.
If it's true that the initializer of a VEC_INIT_EXPR can't observe the
previous elements of the target array, then it should be safe to always
truncate I think?
What if default-initialization of the array element type doesn't fully
initialize the elements, e.g. if 'e' had another member without a default
initializer? Does truncation first mean we lose the zero-initialization of
such a member?
Hmm, it looks like we would lose the zero-initialization of such a
member with or without truncation first (so with any one of the three
proposed fixes). I think it's because the evaluation loop in
cxx_eval_vec_init disregards each element's prior (zero-initialized)
state.
We could probably still do the truncation, but clear the
CONSTRUCTOR_NO_CLEARING flag on the element initializer.
Ah, this seems to work well. Like this?
-- >8 --
Subject: [PATCH] c++: cxx_eval_vec_init after zero initialization [PR96282]
In the first testcase below, expand_aggr_init_1 sets up t's default
constructor such that the ctor first zero-initializes the entire base b,
followed by calling b's default constructor, the latter of which just
default-initializes the array member b::m via a VEC_INIT_EXPR.
So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is
nonempty due to the prior zero-initialization, and we proceed in
cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor
without first checking if a matching constructor_elt already exists.
This leads to ctx->ctor having two matching constructor_elts for each
index.
This patch fixes this issue by truncating a zero-initialized array
object in cxx_eval_vec_init_1 before we begin appending default-initialized
array elements to it. Since default-initialization may leave parts of
the element type unitialized, we also preserve the array's prior
zero-initialized state by clearing CONSTRUCTOR_NO_CLEARING on each
appended element initializers.
gcc/cp/ChangeLog:
PR c++/96282
* constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and
then clear CONSTRUCTOR_NO_CLEARING on each appended element
initializer if we're default-initializing a previously
zero-initialized array object.
gcc/testsuite/ChangeLog:
PR c++/96282
* g++.dg/cpp0x/constexpr-array26.C: New test.
* g++.dg/cpp0x/constexpr-array27.C: New test.
* g++.dg/cpp2a/constexpr-init18.C: New test.
---
gcc/cp/constexpr.c | 17 ++++++++++++++++-
gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++
gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++
gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 ++++++++++++++++
4 files changed, 58 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index b1c1d249c6e..706bef323b2 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -4171,6 +4171,17 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree
atype, tree init,
pre_init = true;
}
+ bool zero_initialized_p = false;
+ if ((pre_init || value_init || !init) && initializer_zerop (ctx->ctor))
Does initializer_zerop capture the difference between a
default-initialized or value-initialized containing object? I would
expect it to be true for both. Maybe check CONSTRUCTOR_NO_CLEARING
(ctx->ctor) instead?
This all seems parallel to the no_zero_init code in
cxx_eval_store_expression.
+ {
+ /* We're default-initializing an array object that had been
+ zero-initialized earlier. We'll preserve this prior state
+ by clearing CONSTRUCTOR_NO_CLEARING on each of its element
+ initializers. */
+ zero_initialized_p = true;
+ vec_safe_truncate (*p, 0);
+ }
+
tree nelts = get_array_or_vector_nelts (ctx, atype, non_constant_p,
overflow_p);
unsigned HOST_WIDE_INT max = tree_to_uhwi (nelts);
@@ -4182,7 +4193,11 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree
atype, tree init,
constexpr_ctx new_ctx;
init_subob_ctx (ctx, new_ctx, idx, pre_init ? init : elttype);
if (new_ctx.ctor != ctx->ctor)
- CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor);
+ {
+ if (zero_initialized_p)
+ CONSTRUCTOR_NO_CLEARING (new_ctx.ctor) = false;
+ CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor);
+ }
if (TREE_CODE (elttype) == ARRAY_TYPE)
{
/* A multidimensional array; recurse. */
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C
new file mode 100644
index 00000000000..274f55a88bf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C
@@ -0,0 +1,13 @@
+// PR c++/96282
+// { dg-do compile { target c++11 } }
+
+struct e { bool v = true; };
+
+template<int N>
+struct b { e m[N]; };
+
+template<int N>
+struct t : b<N> { constexpr t() : b<N>() {} };
+
+constexpr t<1> h1;
+constexpr t<42> h2;
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C
new file mode 100644
index 00000000000..bd5dd58d1ab
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C
@@ -0,0 +1,13 @@
+// PR c++/96282
+// { dg-do compile { target c++11 } }
+
+struct e { bool v = true; e *p = this; };
+
+template<int N>
+struct b { e m[N]; };
+
+template<int N>
+struct t : b<N> { constexpr t() : b<N>() {} };
+
+constexpr t<1> h1;
+constexpr t<42> h2;
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C
b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C
new file mode 100644
index 00000000000..47c15edfc73
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C
@@ -0,0 +1,16 @@
+// PR c++/96282
+// { dg-do compile { target c++20 } }
+
+struct e { bool v = true; bool w; };
+
+template<int N>
+struct b { e m[N]; };
+
+template<int N>
+struct t : b<N> { constexpr t() : b<N>() {} };
+
+constexpr t<1> h1;
+static_assert(h1.m[0].w == false);
+
+constexpr t<42> h2;
+static_assert(h2.m[17].w == false);
