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)) + { + /* 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); -- 2.28.0.89.g85b4e0a6dc