c++: Copy elision and [[no_unique_address]]. [PR93711]

We don't elide a copy from a function returning a class by value into a base
because that can overwrite data laid out in the tail padding of the base
class; we need to handle [[no_unique_address]] fields the same way, or we
ICE when the middle-end wants to create a temporary object of a
TYPE_NEEDS_CONSTRUCTING type.

This means that we can't always express initialization of a field with
INIT_EXPR from a TARGET_EXPR the way we usually do, so I needed
to change several places that were assuming that was sufficient.

This also fixes 90254, the same problem with C++17 aggregate initialization
of a base.

gcc/cp/ChangeLog:

	PR c++/90254
	PR c++/93711
	* cp-tree.h (unsafe_return_slot_p): Declare.
	* call.c (is_base_field_ref): Rename to unsafe_return_slot_p.
	(build_over_call): Check unsafe_return_slot_p.
	(build_special_member_call): Likewise.
	* init.c (expand_default_init): Likewise.
	* typeck2.c (split_nonconstant_init_1): Likewise.

gcc/testsuite/ChangeLog:

	PR c++/90254
	PR c++/93711
	* g++.dg/cpp1z/aggr-base10.C: New test.
	* g++.dg/cpp2a/no_unique_address7.C: New test.
	* g++.dg/cpp2a/no_unique_address7a.C: New test.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index d4935bd0ce13578eb3edd970fda60a0c395a3040..94aaf65b3f4459a62a2b22b073f3e5fc2675d11d 100644 (file)
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8347,24 +8347,34 @@ call_copy_ctor (tree a, tsubst_flags_t complain)
   return r;
 }
 
-/* Return true iff T refers to a base field.  */
+/* Return true iff T refers to a base or potentially-overlapping field, which
+   cannot be used for return by invisible reference.  We avoid doing C++17
+   mandatory copy elision when this is true.
 
-static bool
-is_base_field_ref (tree t)
+   This returns true even if the type of T has no tail padding that other data
+   could be allocated into, because that depends on the particular ABI.
+   unsafe_copy_elision_p, below, does consider whether there is padding.  */
+
+bool
+unsafe_return_slot_p (tree t)
 {
   STRIP_NOPS (t);
   if (TREE_CODE (t) == ADDR_EXPR)
     t = TREE_OPERAND (t, 0);
   if (TREE_CODE (t) == COMPONENT_REF)
     t = TREE_OPERAND (t, 1);
-  if (TREE_CODE (t) == FIELD_DECL)
-    return DECL_FIELD_IS_BASE (t);
-  return false;
+  if (TREE_CODE (t) != FIELD_DECL)
+    return false;
+  if (!CLASS_TYPE_P (TREE_TYPE (t)))
+    /* The middle-end will do the right thing for scalar types.  */
+    return false;
+  return (DECL_FIELD_IS_BASE (t)
+         || lookup_attribute ("no_unique_address", DECL_ATTRIBUTES (t)));
 }
 
-/* We can't elide a copy from a function returning by value to a base
-   subobject, as the callee might clobber tail padding.  Return true iff this
-   could be that case.  */
+/* We can't elide a copy from a function returning by value to a
+   potentially-overlapping subobject, as the callee might clobber tail padding.
+   Return true iff this could be that case.  */
 
 static bool
 unsafe_copy_elision_p (tree target, tree exp)
@@ -8374,10 +8384,11 @@ unsafe_copy_elision_p (tree target, tree exp)
     return false;
   tree type = TYPE_MAIN_VARIANT (TREE_TYPE (exp));
   /* It's safe to elide the copy for a class with no tail padding.  */
-  if (tree_int_cst_equal (TYPE_SIZE (type), CLASSTYPE_SIZE (type)))
+  if (!is_empty_class (type)
+      && tree_int_cst_equal (TYPE_SIZE (type), CLASSTYPE_SIZE (type)))
     return false;
   /* It's safe to elide the copy if we aren't initializing a base object.  */
-  if (!is_base_field_ref (target))
+  if (!unsafe_return_slot_p (target))
     return false;
   tree init = TARGET_EXPR_INITIAL (exp);
   /* build_compound_expr pushes COMPOUND_EXPR inside TARGET_EXPR.  */
@@ -8569,6 +8580,7 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
       && DECL_COMPLETE_CONSTRUCTOR_P (fn)
       && (DECL_COPY_CONSTRUCTOR_P (fn)
          || DECL_MOVE_CONSTRUCTOR_P (fn))
+      && !unsafe_return_slot_p (first_arg)
       && conv_binds_ref_to_prvalue (convs[0]))
     {
       force_elide = true;
@@ -8953,7 +8965,7 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
     {
       tree targ;
       tree arg = argarray[num_artificial_parms_for (fn)];
-      tree fa;
+      tree fa = argarray[0];
       bool trivial = trivial_fn_p (fn);
 
       /* Pull out the real argument, disregarding const-correctness.  */
@@ -8983,8 +8995,8 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
       else
        arg = cp_build_fold_indirect_ref (arg);
 
-      /* In C++17 we shouldn't be copying a TARGET_EXPR except into a base
-        subobject.  */
+      /* In C++17 we shouldn't be copying a TARGET_EXPR except into a
+        potentially-overlapping subobject.  */
       if (CHECKING_P && cxx_dialect >= cxx17)
        gcc_assert (TREE_CODE (arg) != TARGET_EXPR
                    || force_elide
@@ -8992,9 +9004,8 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
                    || convs[0]->need_temporary_p
                    || seen_error ()
                    /* See unsafe_copy_elision_p.  */
-                   || DECL_BASE_CONSTRUCTOR_P (fn));
+                   || unsafe_return_slot_p (fa));
 
-      fa = argarray[0];
       bool unsafe = unsafe_copy_elision_p (fa, arg);
       bool eliding_temp = (TREE_CODE (arg) == TARGET_EXPR && !unsafe);
 
@@ -9806,7 +9817,7 @@ build_special_member_call (tree instance, tree name, vec<tree, va_gc> **args,
      resolution.  */
   if (cxx_dialect >= cxx17
       && args && vec_safe_length (*args) == 1
-      && name == complete_ctor_identifier)
+      && !unsafe_return_slot_p (instance))
     {
       tree arg = (**args)[0];
 

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 82834e65a53fab654cb1166b71bb8daccd41140e..59ab2309d18c6c6e3c91984b3d99e2475cf58551 100644 (file)
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6359,6 +6359,7 @@ extern bool is_std_init_list                      (tree);
 extern bool is_list_ctor                       (tree);
 extern void validate_conversion_obstack                (void);
 extern void mark_versions_used                 (tree);
+extern bool unsafe_return_slot_p               (tree);
 extern bool cp_warn_deprecated_use             (tree, tsubst_flags_t = tf_warning_or_error);
 extern void cp_warn_deprecated_use_scopes      (tree);
 extern tree get_function_version_dispatcher    (tree);

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 3f089404cf261b567cd61e67b95c52b44292dbb5..872c23453fd35a407d6c8f12034ac495b898f1fd 100644 (file)
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -1895,7 +1895,8 @@ expand_default_init (tree binfo, tree true_exp, tree exp, tree init, int flags,
     }
 
   if (init && TREE_CODE (init) != TREE_LIST
-      && (flags & LOOKUP_ONLYCONVERTING))
+      && (flags & LOOKUP_ONLYCONVERTING)
+      && !unsafe_return_slot_p (exp))
     {
       /* Base subobjects should only get direct-initialization.  */
       gcc_assert (true_exp == exp);

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index dac135a2e1144e58e5f894f4c5ef5d24867a80e2..b95f112a7449c8fd59fd7bb3cfbaaa2a7fa48a60 100644 (file)
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -711,7 +711,17 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested)
                    sub = build3 (COMPONENT_REF, inner_type, dest, field_index,
                                  NULL_TREE);
 
-                 code = build2 (INIT_EXPR, inner_type, sub, value);
+                 if (unsafe_return_slot_p (sub))
+                   {
+                     /* We may need to add a copy constructor call if
+                        the field has [[no_unique_address]].  */
+                     releasing_vec args = make_tree_vector_single (value);
+                     code = build_special_member_call
+                       (sub, complete_ctor_identifier, &args, inner_type,
+                        LOOKUP_NORMAL, tf_warning_or_error);
+                   }
+                 else
+                   code = build2 (INIT_EXPR, inner_type, sub, value);
                  code = build_stmt (input_location, EXPR_STMT, code);
                  code = maybe_cleanup_point_expr_void (code);
                  add_stmt (code);

diff --git a/gcc/testsuite/g++.dg/cpp1z/aggr-base10.C b/gcc/testsuite/g++.dg/cpp1z/aggr-base10.C
new file mode 100644 (file)
index 0000000..10370da
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/aggr-base10.C
@@ -0,0 +1,16 @@
+// PR c++/90254
+// { dg-do compile { target c++17 } }
+
+struct A {
+  A();
+  A(const A &);
+};
+struct B : A { };
+
+A foo ();
+
+int
+main ()
+{
+  B b{foo()};
+}

diff --git a/gcc/testsuite/g++.dg/cpp2a/no_unique_address7.C b/gcc/testsuite/g++.dg/cpp2a/no_unique_address7.C
new file mode 100644 (file)
index 0000000..edd82d1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/no_unique_address7.C
@@ -0,0 +1,13 @@
+// PR c++/93711
+// { dg-do compile { target c++11 } }
+
+struct A { A(const A&) = delete; };
+
+A f();
+
+struct C
+{
+  [[no_unique_address]] A a;
+};
+
+C c{f()};                      // { dg-error "deleted" }

diff --git a/gcc/testsuite/g++.dg/cpp2a/no_unique_address7a.C b/gcc/testsuite/g++.dg/cpp2a/no_unique_address7a.C
new file mode 100644 (file)
index 0000000..453bace
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/no_unique_address7a.C
@@ -0,0 +1,14 @@
+// PR c++/93711
+// { dg-do compile { target c++11 } }
+
+struct B { };
+struct A: B { A(const A&) = delete; };
+
+A f();
+
+struct C
+{
+  [[no_unique_address]] A a;
+};
+
+C c{f()};                      // { dg-error "deleted" }