From 34a7a2308da1effd628f9f7959e1f5cabec918be Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Wed, 13 Jun 2018 15:39:36 -0400 Subject: [PATCH] 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. From-SVN: r261562 --- gcc/c-family/ChangeLog | 8 +++++ gcc/c-family/c-opts.c | 41 ++++++++++++++---------- gcc/common.opt | 6 +++- gcc/cp/ChangeLog | 7 ++++ gcc/cp/class.c | 13 ++++++++ gcc/cp/cp-tree.h | 1 + gcc/cp/tree.c | 44 +++++++++++++++++++------- gcc/doc/invoke.texi | 9 ++++-- gcc/testsuite/g++.dg/abi/invisiref2a.C | 14 ++++++++ gcc/testsuite/g++.dg/abi/macro0.C | 2 +- 10 files changed, 113 insertions(+), 32 deletions(-) create mode 100644 gcc/testsuite/g++.dg/abi/invisiref2a.C diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index f63af26624c..72f24fdb772 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,11 @@ +2018-06-13 Jason Merrill + + PR c++/86094 - wrong code with defaulted move ctor. + * 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. + 2018-06-12 Martin Sebor PR c/85931 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/ChangeLog b/gcc/cp/ChangeLog index 4c23d574bf1..abdccbc12cf 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,10 @@ +2018-06-13 Jason Merrill + + PR c++/86094 - wrong code with defaulted move ctor. + * class.c (classtype_has_non_deleted_move_ctor): New. + * tree.c (maybe_warn_parm_abi, type_has_nontrivial_copy_init): + Handle v12 breakage. + 2018-06-12 Jason Merrill PR c++/86098 - ICE with template placeholder for TTP. 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; - } - - return false; + /* 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 nontriv; } else return 0; diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index b06ea6e7368..eb33b569b8b 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 -- 2.30.2