aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJason Merrill <jason@redhat.com>2020-08-12 05:45:02 -0400
committerGiuliano Belinassi <giuliano.belinassi@usp.br>2020-08-17 15:08:13 -0300
commit8d249461feb6cd707b1fcb1a936baeaeee0633de (patch)
treeb256473af8402f74e819e0456c2d611625e2551c
parent487952eb6244fbd5ba720313a8d39e5732282215 (diff)
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.
-rw-r--r--gcc/cp/call.c45
-rw-r--r--gcc/cp/cp-tree.h1
-rw-r--r--gcc/cp/init.c3
-rw-r--r--gcc/cp/typeck2.c12
-rw-r--r--gcc/testsuite/g++.dg/cpp1z/aggr-base10.C16
-rw-r--r--gcc/testsuite/g++.dg/cpp2a/no_unique_address7.C13
-rw-r--r--gcc/testsuite/g++.dg/cpp2a/no_unique_address7a.C14
7 files changed, 85 insertions, 19 deletions
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index d4935bd0ce1..94aaf65b3f4 100644
--- 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 82834e65a53..59ab2309d18 100644
--- 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 3f089404cf2..872c23453fd 100644
--- 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 dac135a2e11..b95f112a744 100644
--- 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
index 00000000000..10370daa6e6
--- /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
index 00000000000..edd82d1eccc
--- /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
index 00000000000..453baceb905
--- /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" }