c++: empty union member activation during constexpr [PR102163]

Message ID 20210913195454.3679513-1-ppalka@redhat.com
State New
Headers show
Series
  • c++: empty union member activation during constexpr [PR102163]
Related show

Commit Message

apinski--- via Gcc-patches Sept. 13, 2021, 7:54 p.m.
Here, the union's constructor is defined to activate its empty data
member _M_rest, but during constexpr evaluation of this constructor the
subobject constructor call to O::O(&_M_rest, 42) produces no side
effects that actually activates the member, so the union still appears
uninitialized after the fact.  This patch fixes this by faking up a
dummy MODIFY_EXPR in this situation, whose evaluation ensures the member
gets activated.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
for trunk?

	PR c++/102163

gcc/cp/ChangeLog:

	* constexpr.c (cxx_eval_call_expression): After evaluating a
	constructor call for an empty union member, produce a side
	effect that makes sure the member is activated.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/constexpr-empty17.C: New test.
---
 gcc/cp/constexpr.c                            | 34 +++++++++++++++----
 .../g++.dg/cpp0x/constexpr-empty17.C          | 21 ++++++++++++
 2 files changed, 49 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-empty17.C

-- 
2.33.0.328.g8b7c11b866

Comments

apinski--- via Gcc-patches Sept. 14, 2021, 2:54 p.m. | #1
On 9/13/21 3:54 PM, Patrick Palka wrote:
> Here, the union's constructor is defined to activate its empty data

> member _M_rest, but during constexpr evaluation of this constructor the

> subobject constructor call to O::O(&_M_rest, 42) produces no side

> effects that actually activates the member, so the union still appears

> uninitialized after the fact.  This patch fixes this by faking up a

> dummy MODIFY_EXPR in this situation, whose evaluation ensures the member

> gets activated.

> 

> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK

> for trunk?


OK.

> 	PR c++/102163

> 

> gcc/cp/ChangeLog:

> 

> 	* constexpr.c (cxx_eval_call_expression): After evaluating a

> 	constructor call for an empty union member, produce a side

> 	effect that makes sure the member is activated.

> 

> gcc/testsuite/ChangeLog:

> 

> 	* g++.dg/cpp0x/constexpr-empty17.C: New test.

> ---

>   gcc/cp/constexpr.c                            | 34 +++++++++++++++----

>   .../g++.dg/cpp0x/constexpr-empty17.C          | 21 ++++++++++++

>   2 files changed, 49 insertions(+), 6 deletions(-)

>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-empty17.C

> 

> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c

> index 7772fe62d95..40b0b80b438 100644

> --- a/gcc/cp/constexpr.c

> +++ b/gcc/cp/constexpr.c

> @@ -2787,12 +2787,34 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,

>   					&jump_target);

>   

>   	  if (DECL_CONSTRUCTOR_P (fun))

> -	    /* This can be null for a subobject constructor call, in

> -	       which case what we care about is the initialization

> -	       side-effects rather than the value.  We could get at the

> -	       value by evaluating *this, but we don't bother; there's

> -	       no need to put such a call in the hash table.  */

> -	    result = lval ? ctx->object : ctx->ctor;

> +	    {

> +	      /* This can be null for a subobject constructor call, in

> +		 which case what we care about is the initialization

> +		 side-effects rather than the value.  We could get at the

> +		 value by evaluating *this, but we don't bother; there's

> +		 no need to put such a call in the hash table.  */

> +	      result = lval ? ctx->object : ctx->ctor;

> +

> +	      if (!result && new_obj

> +		  && TREE_CODE (new_obj) == COMPONENT_REF

> +		  && TREE_CODE (TREE_TYPE

> +				(TREE_OPERAND (new_obj, 0))) == UNION_TYPE

> +		  && is_really_empty_class (TREE_TYPE (new_obj),

> +					    /*ignore_vptr*/false))

> +		{

> +		  /* This constructor call for an empty union member might not

> +		     have produced a side effect that actually activated the

> +		     member.  So produce such a side effect now to ensure the

> +		     union appears initialized.  */

> +		  tree activate = build2 (MODIFY_EXPR, TREE_TYPE (new_obj),

> +					  new_obj,

> +					  build_constructor (TREE_TYPE (new_obj),

> +							     NULL));

> +		  cxx_eval_constant_expression (ctx, activate, lval,

> +						non_constant_p, overflow_p);

> +		  ggc_free (activate);

> +		}

> +	    }

>   	  else if (VOID_TYPE_P (TREE_TYPE (res)))

>   	    result = void_node;

>   	  else

> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-empty17.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-empty17.C

> new file mode 100644

> index 00000000000..9d753a3bb69

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-empty17.C

> @@ -0,0 +1,21 @@

> +// PR c++/102163

> +// { dg-do compile { target c++11 } }

> +

> +struct O {

> +  constexpr O(int) { }

> +};

> +

> +union _Variadic_union {

> +  constexpr _Variadic_union(int __arg) : _M_rest(__arg) { }

> +

> +  int _M_first;

> +  O _M_rest;

> +};

> +

> +

> +struct _Variant_storage {

> +  constexpr _Variant_storage() : _M_u(42) {}

> +  _Variadic_union _M_u;

> +};

> +

> +constexpr _Variant_storage w;

>

Patch

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 7772fe62d95..40b0b80b438 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -2787,12 +2787,34 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 					&jump_target);
 
 	  if (DECL_CONSTRUCTOR_P (fun))
-	    /* This can be null for a subobject constructor call, in
-	       which case what we care about is the initialization
-	       side-effects rather than the value.  We could get at the
-	       value by evaluating *this, but we don't bother; there's
-	       no need to put such a call in the hash table.  */
-	    result = lval ? ctx->object : ctx->ctor;
+	    {
+	      /* This can be null for a subobject constructor call, in
+		 which case what we care about is the initialization
+		 side-effects rather than the value.  We could get at the
+		 value by evaluating *this, but we don't bother; there's
+		 no need to put such a call in the hash table.  */
+	      result = lval ? ctx->object : ctx->ctor;
+
+	      if (!result && new_obj
+		  && TREE_CODE (new_obj) == COMPONENT_REF
+		  && TREE_CODE (TREE_TYPE
+				(TREE_OPERAND (new_obj, 0))) == UNION_TYPE
+		  && is_really_empty_class (TREE_TYPE (new_obj),
+					    /*ignore_vptr*/false))
+		{
+		  /* This constructor call for an empty union member might not
+		     have produced a side effect that actually activated the
+		     member.  So produce such a side effect now to ensure the
+		     union appears initialized.  */
+		  tree activate = build2 (MODIFY_EXPR, TREE_TYPE (new_obj),
+					  new_obj,
+					  build_constructor (TREE_TYPE (new_obj),
+							     NULL));
+		  cxx_eval_constant_expression (ctx, activate, lval,
+						non_constant_p, overflow_p);
+		  ggc_free (activate);
+		}
+	    }
 	  else if (VOID_TYPE_P (TREE_TYPE (res)))
 	    result = void_node;
 	  else
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-empty17.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-empty17.C
new file mode 100644
index 00000000000..9d753a3bb69
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-empty17.C
@@ -0,0 +1,21 @@ 
+// PR c++/102163
+// { dg-do compile { target c++11 } }
+
+struct O {
+  constexpr O(int) { }
+};
+
+union _Variadic_union {
+  constexpr _Variadic_union(int __arg) : _M_rest(__arg) { }
+
+  int _M_first;
+  O _M_rest;
+};
+
+
+struct _Variant_storage {
+  constexpr _Variant_storage() : _M_u(42) {}
+  _Variadic_union _M_u;
+};
+
+constexpr _Variant_storage w;