On Mon, Jun 11, 2018 at 2:38 PM, Jason Merrill <ja...@redhat.com> wrote:
> The fix for 80178 was broken, because I forgot that copy_fn_p is false
> for move constructors. As a result, the calling convention for a
> class with a trivial move constructor and deleted copy constructor
> changed inappropriately.
This patch restores the broken behavior to -fabi-version=12 and adds
-fabi-version=13 for the fix; people can use -Wabi=12 with GCC 8.2 to
check for compatibility issues against 8.1, or -Wabi=11 to check
compatibility with GCC 7.
Tested x86_64-pc-linux-gnu, applying to trunk and 8. Do we want to
accelerate 8.2 because of this issue?
commit f118d95dd7802a80387982135686be27b3e0dbb7
Author: Jason Merrill <ja...@redhat.com>
Date: Wed Jun 13 13:35:29 2018 -0400
PR c++/86094 - wrong code with defaulted move ctor.
gcc/c-family/
* c-opts.c (c_common_post_options): Bump the current ABI version to
13. Set warn_abi_version and flag_abi_compat_version to the current
version rather than 0. Fix defaulting flag_abi_compat_version from
warn_abi_version.
gcc/cp/
* class.c (classtype_has_non_deleted_move_ctor): New.
* tree.c (maybe_warn_parm_abi, type_has_nontrivial_copy_init):
Handle v12 breakage.
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index ddaaef37e1d..107359ec20d 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -915,31 +915,38 @@ c_common_post_options (const char **pfilename)
if (flag_declone_ctor_dtor == -1)
flag_declone_ctor_dtor = optimize_size;
- if (warn_abi_version == -1)
- {
- if (flag_abi_compat_version != -1)
- warn_abi_version = flag_abi_compat_version;
- else
- warn_abi_version = 0;
- }
-
if (flag_abi_compat_version == 1)
{
warning (0, "%<-fabi-compat-version=1%> is not supported, using =2");
flag_abi_compat_version = 2;
}
- else if (flag_abi_compat_version == -1)
+
+ /* Change flag_abi_version to be the actual current ABI level, for the
+ benefit of c_cpp_builtins, and to make comparison simpler. */
+ const int latest_abi_version = 13;
+ /* Generate compatibility aliases for ABI v11 (7.1) by default. */
+ const int abi_compat_default = 11;
+
+#define clamp(X) if (X == 0 || X > latest_abi_version) X = latest_abi_version
+ clamp (flag_abi_version);
+ clamp (warn_abi_version);
+ clamp (flag_abi_compat_version);
+#undef clamp
+
+ /* Default -Wabi= or -fabi-compat-version= from each other. */
+ if (warn_abi_version == -1 && flag_abi_compat_version != -1)
+ warn_abi_version = flag_abi_compat_version;
+ else if (flag_abi_compat_version == -1 && warn_abi_version != -1)
+ flag_abi_compat_version = warn_abi_version;
+ else if (warn_abi_version == -1 && flag_abi_compat_version == -1)
{
- /* Generate compatibility aliases for ABI v11 (7.1) by default. */
- flag_abi_compat_version
- = (flag_abi_version == 0 ? 11 : 0);
+ warn_abi_version = latest_abi_version;
+ if (flag_abi_version == latest_abi_version)
+ flag_abi_compat_version = abi_compat_default;
+ else
+ flag_abi_compat_version = latest_abi_version;
}
- /* Change flag_abi_version to be the actual current ABI level for the
- benefit of c_cpp_builtins. */
- if (flag_abi_version == 0)
- flag_abi_version = 12;
-
/* By default, enable the new inheriting constructor semantics along with ABI
11. New and old should coexist fine, but it is a change in what
artificial symbols are generated. */
diff --git a/gcc/common.opt b/gcc/common.opt
index 4aebcaf0656..efbeba33819 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -938,7 +938,11 @@ Driver Undocumented
;
; 12: Corrects the calling convention for classes with only deleted copy/move
; constructors and changes passing/returning of empty records.
-; Default in G++ 8.
+; Default in G++ 8.1.
+;
+; 13: Fixes the accidental change in 12 to the calling convention for classes
+; with deleted copy constructor and trivial move constructor.
+; Default in G++ 8.2.
;
; Additional positive integers will be assigned as new versions of
; the ABI become the default version of the ABI.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index b6e78c6377d..9a397e3856c 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -5175,6 +5175,19 @@ classtype_has_move_assign_or_move_ctor_p (tree t, bool user_p)
return false;
}
+/* True iff T has a move constructor that is not deleted. */
+
+bool
+classtype_has_non_deleted_move_ctor (tree t)
+{
+ if (CLASSTYPE_LAZY_MOVE_CTOR (t))
+ lazily_declare_fn (sfk_move_constructor, t);
+ for (ovl_iterator iter (CLASSTYPE_CONSTRUCTORS (t)); iter; ++iter)
+ if (move_fn_p (*iter) && !DECL_DELETED_FN (*iter))
+ return true;
+ return false;
+}
+
/* If T, a class, has a user-provided copy constructor, copy assignment
operator, or destructor, returns that function. Otherwise, null. */
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 59ad38107a8..9a50d666cad 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6163,6 +6163,7 @@ extern bool trivial_default_constructor_is_constexpr (tree);
extern bool type_has_constexpr_default_constructor (tree);
extern bool type_has_virtual_destructor (tree);
extern bool classtype_has_move_assign_or_move_ctor_p (tree, bool user_declared);
+extern bool classtype_has_non_deleted_move_ctor (tree);
extern tree classtype_has_user_copy_or_dtor (tree);
extern bool type_build_ctor_call (tree);
extern bool type_build_dtor_call (tree);
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 156d1e469c6..48a0ff37372 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -4071,8 +4071,21 @@ maybe_warn_parm_abi (tree t, location_t loc)
|| !deleted_copy_types->contains (t))
return;
+ if ((flag_abi_version == 12 || warn_abi_version == 12)
+ && classtype_has_non_deleted_move_ctor (t))
+ {
+ if (flag_abi_version > 12)
+ warning_at (loc, OPT_Wabi, "-fabi-version=13 (GCC 8.2) fixes the "
+ "calling convention for %qT, which was accidentally "
+ "changed in 8.1", t);
+ else
+ warning_at (loc, OPT_Wabi, "-fabi-version=12 (GCC 8.1) accidentally "
+ "changes the calling convention for %qT", t);
+ return;
+ }
+
warning_at (loc, OPT_Wabi, "the calling convention for %qT changes in "
- "-fabi-version=12 (GCC 8)", t);
+ "-fabi-version=13 (GCC 8.2)", t);
static bool explained = false;
if (!explained)
{
@@ -4114,6 +4127,7 @@ type_has_nontrivial_copy_init (const_tree type)
bool saw_copy = false;
bool saw_non_deleted = false;
+ bool saw_non_deleted_move = false;
if (CLASSTYPE_LAZY_MOVE_CTOR (t))
saw_copy = saw_non_deleted = true;
@@ -4135,7 +4149,7 @@ type_has_nontrivial_copy_init (const_tree type)
for (ovl_iterator iter (CLASSTYPE_CONSTRUCTORS (t)); iter; ++iter)
{
tree fn = *iter;
- if (copy_fn_p (fn) || move_fn_p (fn))
+ if (copy_fn_p (fn))
{
saw_copy = true;
if (!DECL_DELETED_FN (fn))
@@ -4145,19 +4159,27 @@ type_has_nontrivial_copy_init (const_tree type)
break;
}
}
+ else if (move_fn_p (fn))
+ if (!DECL_DELETED_FN (fn))
+ saw_non_deleted_move = true;
}
gcc_assert (saw_copy);
- if (saw_copy && !saw_non_deleted)
- {
- if (warn_abi && abi_version_crosses (12))
- remember_deleted_copy (t);
- if (abi_version_at_least (12))
- return true;
- }
+ /* ABI v12 buggily ignored move constructors. */
+ bool v11nontriv = false;
+ bool v12nontriv = !saw_non_deleted;
+ bool v13nontriv = !saw_non_deleted && !saw_non_deleted_move;
+ bool nontriv = (abi_version_at_least (13) ? v13nontriv
+ : flag_abi_version == 12 ? v12nontriv
+ : v11nontriv);
+ bool warn_nontriv = (warn_abi_version >= 13 ? v13nontriv
+ : warn_abi_version == 12 ? v12nontriv
+ : v11nontriv);
+ if (nontriv != warn_nontriv)
+ remember_deleted_copy (t);
- return false;
+ return nontriv;
}
else
return 0;
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index c55be3d95fd..99281eb4c85 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -2348,7 +2348,12 @@ implies @option{-fnew-inheriting-ctors}.
Version 12, which first appeared in G++ 8, corrects the calling
conventions for empty classes on the x86_64 target and for classes
-with only deleted copy/move constructors.
+with only deleted copy/move constructors. It accidentally changes the
+calling convention for classes with a deleted copy constructor and a
+trivial move constructor.
+
+Version 13, which first appeared in G++ 8.2, fixes the accidental
+change in version 12.
See also @option{-Wabi}.
@@ -2359,7 +2364,7 @@ works around mangling changes by creating an alias with the correct
mangled name when defining a symbol with an incorrect mangled name.
This switch specifies which ABI version to use for the alias.
-With @option{-fabi-version=0} (the default), this defaults to 8 (GCC 5
+With @option{-fabi-version=0} (the default), this defaults to 11 (GCC 7
compatibility). If another ABI version is explicitly selected, this
defaults to 0. For compatibility with GCC versions 3.2 through 4.9,
use @option{-fabi-compat-version=2}.
diff --git a/gcc/testsuite/g++.dg/abi/invisiref2a.C b/gcc/testsuite/g++.dg/abi/invisiref2a.C
new file mode 100644
index 00000000000..05330553287
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/invisiref2a.C
@@ -0,0 +1,14 @@
+// PR c++/86094
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-fabi-version=12 -Wabi -fdump-tree-gimple" }
+// { dg-final { scan-tree-dump "struct S &" "gimple" } }
+
+struct S {
+ S(S&&) = default;
+ int i;
+};
+
+S foo(S s) // { dg-warning "calling convention" }
+{
+ return s;
+}
diff --git a/gcc/testsuite/g++.dg/abi/macro0.C b/gcc/testsuite/g++.dg/abi/macro0.C
index 30eb02937c3..01a3541aa7d 100644
--- a/gcc/testsuite/g++.dg/abi/macro0.C
+++ b/gcc/testsuite/g++.dg/abi/macro0.C
@@ -1,6 +1,6 @@
// This testcase will need to be kept in sync with c_common_post_options.
// { dg-options "-fabi-version=0" }
-#if __GXX_ABI_VERSION != 1012
+#if __GXX_ABI_VERSION != 1013
#error "Incorrect value of __GXX_ABI_VERSION"
#endif
