On Thu, 19 Mar 2020, Patrick Palka wrote:
> This patch adds a check to detect changing the active union member during
> initialization of the union. It uses the CONSTRUCTOR_NO_CLEARING flag as a
> proxy for whether the non-empty CONSTRUCTOR of UNION_TYPE we're assigning to
> in
> cxx_eval_store_expression is in the process of being initialized, which seems
> to
> work well.
If we can't rely on CONSTRUCTOR_NO_CLEARING to be set iff a CONSTRUCTOR
is in the process of being initialized, then here's an alternative patch
for consideration, that detects this UB in an indirect way and after the
fact.
-- >8 --
This patch adds checks to detect whether an active member of a union has been
changed during initialization of the union.
It add checks in three places, one in cxx_eval_bare_aggregate and two in
cxx_eval_store_expression (for preeval and !preeval). These checks are indirect
and only detect this UB after the whole initializer has been evaluated and not
at e.g. the exact assignment in the initializer through which the active union
member gets changed, so the source locations attached to the error messages are
suboptimal.
gcc/cp/ChangeLog:
PR c++/94066
* constexpr.c (cxx_eval_bare_aggregate): Check whether the active
member of the union has been changed during evaluation of its
initializer, and diagnose this if so.
(cxx_eval_store_expression): Likewise for both preeval and !preeval
cases.
gcc/testsuite/ChangeLog:
PR c++/94066
* g++.dg/cpp1y/pr94066.C: New test.
* g++.dg/cpp1y/pr94066-2.C: New test.
* g++.dg/cpp1y/pr94066-3.C: New test.
---
gcc/cp/constexpr.c | 45 +++++++++++++++++++++++++-
gcc/testsuite/g++.dg/cpp1y/pr94066-2.C | 19 +++++++++++
gcc/testsuite/g++.dg/cpp1y/pr94066-3.C | 21 ++++++++++++
gcc/testsuite/g++.dg/cpp1y/pr94066.C | 18 +++++++++++
4 files changed, 102 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 192face9a3a..bcb6b758980 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3784,7 +3784,19 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree
t,
}
else
{
- if (new_ctx.ctor != ctx->ctor)
+ if (TREE_CODE (type) == UNION_TYPE
+ && vec_safe_length (*p)
+ && (**p)[0].index != index)
+ {
+ if (!ctx->quiet)
+ error_at (cp_expr_loc_or_input_loc (t),
+ "change of the active member of a union "
+ "from %qD to %qD during initialization",
+ index,
+ (**p)[0].index);
+ *non_constant_p = true;
+ }
+ else if (new_ctx.ctor != ctx->ctor)
{
/* We appended this element above; update the value. */
gcc_assert ((*p)->last().index == index);
@@ -4647,6 +4659,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
tree t,
index);
*non_constant_p = true;
}
+ else if (preeval && TREE_CODE (t) == INIT_EXPR)
+ {
+ if (!ctx->quiet)
+ error_at (cp_expr_loc_or_input_loc (t),
+ "change of the active member of a union "
+ "from %qD to %qD during initialization",
+ index,
+ CONSTRUCTOR_ELT (*valp, 0)->index);
+ *non_constant_p = true;
+ }
/* Changing active member. */
vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
no_zero_init = true;
@@ -4761,6 +4783,27 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
tree t,
if (ctors->is_empty())
/* The hash table might have moved since the get earlier. */
valp = ctx->global->values.get (object);
+ else if (TREE_CODE (t) == INIT_EXPR)
+ {
+ tree ctor = ctors->last();
+ if (TREE_CODE (TREE_TYPE (ctor)) == UNION_TYPE)
+ {
+ tree index = TREE_OPERAND (target, 1);
+ gcc_assert (CONSTRUCTOR_NELTS (ctor));
+ gcc_assert (TREE_CODE (index) == FIELD_DECL
+ && DECL_CONTEXT (index) == TREE_TYPE (ctor));
+ if (CONSTRUCTOR_ELT (ctor, 0)->index != index)
+ {
+ if (!ctx->quiet)
+ error_at (cp_expr_loc_or_input_loc (t),
+ "change of the active member of a union "
+ "from %qD to %qD during initialization",
+ index,
+ CONSTRUCTOR_ELT (ctor, 0)->index);
+ *non_constant_p = true;
+ }
+ }
+ }
}
/* Don't share a CONSTRUCTOR that might be changed later. */
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
b/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
new file mode 100644
index 00000000000..438e2e436d8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
@@ -0,0 +1,19 @@
+// PR c++/94066
+// { dg-do compile { target c++14 } }
+
+struct A { long x; };
+
+union U;
+constexpr A foo(U *up);
+
+union U {
+ U() = default;
+ A a = foo(this); int y;
+};
+
+constexpr A foo(U *up) {
+ up->y = 11; // { dg-error "'U::a' to 'U::y'" "" { target
c++17_down } }
+ return {42};
+}
+
+extern constexpr U u = {}; // { dg-error "'U::a' to 'U::y' during
initialization" }
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
b/gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
new file mode 100644
index 00000000000..d74e222b421
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
@@ -0,0 +1,21 @@
+// PR c++/94066
+// { dg-do compile { target c++14 } }
+
+struct A { long x; };
+
+union U;
+constexpr int foo(U *up);
+
+union U {
+ U() = default;
+ // { dg-error "'U::a' to 'U::y' during initialization" "" { target c++2a }
.-1 }
+ int a = foo(this); int y;
+};
+
+constexpr int foo(U *up) {
+ up->y = 11;
+ return {42};
+}
+
+extern constexpr U u = {};
+// { dg-error "'U::a' to 'U::y' during initialization" "" { target c++17_down
} .-1 }
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr94066.C
b/gcc/testsuite/g++.dg/cpp1y/pr94066.C
new file mode 100644
index 00000000000..3d981b5540e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr94066.C
@@ -0,0 +1,18 @@
+// PR c++/94066
+// { dg-do compile { target c++14 } }
+
+struct A { long x; };
+
+union U;
+constexpr A foo(U *up);
+
+union U {
+ A a = foo(this); int y;
+};
+
+constexpr A foo(U *up) {
+ up->y = 11; // { dg-error "'U::a' to 'U::y'" "" { target
c++17_down } }
+ return {42};
+}
+
+extern constexpr U u = {}; // { dg-error "'U::a' to 'U::y' during
initialization" }
--
2.26.0.rc1.11.g30e9940356
