Bug 86094 - [8/9 Regression] Call ABI changed for small objects with defaulted ctor
Summary: [8/9 Regression] Call ABI changed for small objects with defaulted ctor
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 8.0
: P1 blocker
Target Milestone: 8.2
Assignee: Jason Merrill
URL:
Keywords: ABI, wrong-code
Depends on:
Blocks:
 
Reported: 2018-06-08 18:01 UTC by Alexander Monakov
Modified: 2018-08-17 10:20 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-06-11 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Monakov 2018-06-08 18:01:14 UTC
When compiling the following with -O2 -std=c++11:

struct S {
    S(S&&) = default;
    int i;
};

S foo(S s)
{
    return s;
}

gcc-7 and earlier emit

_Z3foo1S:
        movl    %edi, %eax
        ret

but gcc-8 and trunk emit

_Z3foo1S:
        movl    (%rsi), %edx
        movq    %rdi, %rax
        movl    %edx, (%rdi)
        ret

i.e. the object is now passed in memory rather than on register. This appears to be a silent ABI change.

(Clang generates the same code as gcc-7)
Comment 1 Jonathan Wakely 2018-06-11 11:39:19 UTC
This is controlled by -fabi-version=12 (the default in gcc 8).
Comment 2 Jonathan Wakely 2018-06-11 11:51:07 UTC
This changed with:

            PR c++/80178 - parameter passing for uncopyable classes
    
            * tree.c (type_has_nontrivial_copy_init): True for classes with only
            deleted copy/move ctors.
            (remember_deleted_copy, maybe_warn_parm_abi): New.
            * decl.c (require_complete_types_for_parms, check_function_type):
            Call maybe_warn_parm_abi.
            * call.c (convert_for_arg_passing, build_cxx_call): Likewise.

(Which didn't document -fabi-version=12 in invoke.texi)
Comment 3 Alexander Monakov 2018-06-11 11:54:50 UTC
-fabi-version=12 is not documented, not mentioned in release notes, and not wired up in -Wabi.
Comment 4 Jason Merrill 2018-06-11 17:52:48 UTC
(In reply to Alexander Monakov from comment #3)
> -fabi-version=12 is not documented, not mentioned in release notes, and not
> wired up in -Wabi.

-Wabi=11 warns about this.  With an incorrect message, since this is a bug.  I'll also fix the documentation.
Comment 5 Jason Merrill 2018-06-11 18:39:23 UTC
Author: jason
Date: Mon Jun 11 18:38:52 2018
New Revision: 261444

URL: https://gcc.gnu.org/viewcvs?rev=261444&root=gcc&view=rev
Log:
	PR c++/86094 - wrong code with defaulted move ctor.

	* tree.c (type_has_nontrivial_copy_init): Fix move ctor handling.

Added:
    trunk/gcc/testsuite/g++.dg/abi/invisiref2.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/tree.c
Comment 6 Jason Merrill 2018-06-11 18:40:46 UTC
Author: jason
Date: Mon Jun 11 18:40:14 2018
New Revision: 261445

URL: https://gcc.gnu.org/viewcvs?rev=261445&root=gcc&view=rev
Log:
	PR c++/86094 - wrong code with defaulted move ctor.

	* tree.c (type_has_nontrivial_copy_init): Fix move ctor handling.

Added:
    branches/gcc-8-branch/gcc/testsuite/g++.dg/abi/invisiref2.C
Modified:
    branches/gcc-8-branch/gcc/cp/ChangeLog
    branches/gcc-8-branch/gcc/cp/tree.c
Comment 7 Richard Biener 2018-06-13 15:22:32 UTC
Fixed?
Comment 8 Richard Biener 2018-06-13 15:23:10 UTC
Note if there's any ABI change between 8.1 and 8.2 please adjust changes.html
to note this as a caveat (even if the 7.3 -> 8.1 change was unintended).
Comment 9 Jason Merrill 2018-06-13 19:40:09 UTC
Author: jason
Date: Wed Jun 13 19:39:36 2018
New Revision: 261562

URL: https://gcc.gnu.org/viewcvs?rev=261562&root=gcc&view=rev
Log:
	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.

Added:
    trunk/gcc/testsuite/g++.dg/abi/invisiref2a.C
Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-opts.c
    trunk/gcc/common.opt
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/class.c
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/tree.c
    trunk/gcc/doc/invoke.texi
    trunk/gcc/testsuite/g++.dg/abi/macro0.C
Comment 10 Jason Merrill 2018-06-13 19:40:25 UTC
Author: jason
Date: Wed Jun 13 19:39:53 2018
New Revision: 261563

URL: https://gcc.gnu.org/viewcvs?rev=261563&root=gcc&view=rev
Log:
	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.

Added:
    branches/gcc-8-branch/gcc/testsuite/g++.dg/abi/invisiref2a.C
Modified:
    branches/gcc-8-branch/gcc/c-family/ChangeLog
    branches/gcc-8-branch/gcc/c-family/c-opts.c
    branches/gcc-8-branch/gcc/common.opt
    branches/gcc-8-branch/gcc/cp/ChangeLog
    branches/gcc-8-branch/gcc/cp/class.c
    branches/gcc-8-branch/gcc/cp/cp-tree.h
    branches/gcc-8-branch/gcc/cp/tree.c
    branches/gcc-8-branch/gcc/doc/invoke.texi
    branches/gcc-8-branch/gcc/testsuite/g++.dg/abi/macro0.C
Comment 11 Jakub Jelinek 2018-06-15 12:42:27 UTC
Fixed.
Comment 12 Azat 2018-08-16 21:56:04 UTC
Shouldn't such case also be warned by `g++ -Wabi=12` ?

struct noncopyable
{
public:
    noncopyable(noncopyable &&) = default;
};
struct S : public noncopyable
{
    int i;
};
void foo(S s) {}
Comment 13 Jonathan Wakely 2018-08-17 10:20:32 UTC
No, because the same code is generated for -fabi-version=12 and -fabi-version=13