Skip to content

Commit

Permalink
[clang][CodeGen] Fix MSVC ABI for classes with a deleted copy assignm…
Browse files Browse the repository at this point in the history
…ent operator (llvm#90547)

For global functions and static methods the MSVC ABI returns
structs/classes with a deleted copy assignment operator indirectly.
From local testing this ABI holds true for all currently supported
architectures including ARM64EC.
  • Loading branch information
MaxEW707 authored May 7, 2024
1 parent c0d9efd commit 3f37397
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 1 deletion.
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ ABI Changes in This Version
returning a class in a register. This affects some uses of std::pair.
(#GH86384).

- Fixed Microsoft calling convention when returning classes that have a deleted
copy assignment operator. Such a class should be returned indirectly.

AST Dumping Potentially Breaking Changes
----------------------------------------

Expand Down
22 changes: 21 additions & 1 deletion clang/lib/CodeGen/MicrosoftCXXABI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1122,7 +1122,22 @@ static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
// No base classes
// No virtual functions
// Additionally, we need to ensure that there is a trivial copy assignment
// operator, a trivial destructor and no user-provided constructors.
// operator, a trivial destructor, no user-provided constructors and no
// deleted copy assignment operator.

// We need to cover two cases when checking for a deleted copy assignment
// operator.
//
// struct S { int& r; };
// The above will have an implicit copy assignment operator that is deleted
// and there will not be a `CXXMethodDecl` for the copy assignment operator.
// This is handled by the `needsImplicitCopyAssignment()` check below.
//
// struct S { S& operator=(const S&) = delete; int i; };
// The above will not have an implicit copy assignment operator that is
// deleted but there is a deleted `CXXMethodDecl` for the declared copy
// assignment operator. This is handled by the `isDeleted()` check below.

if (RD->hasProtectedFields() || RD->hasPrivateFields())
return false;
if (RD->getNumBases() > 0)
Expand All @@ -1131,13 +1146,18 @@ static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
return false;
if (RD->hasNonTrivialCopyAssignment())
return false;
if (RD->needsImplicitCopyAssignment() && !RD->hasSimpleCopyAssignment())
return false;
for (const Decl *D : RD->decls()) {
if (auto *Ctor = dyn_cast<CXXConstructorDecl>(D)) {
if (Ctor->isUserProvided())
return false;
} else if (auto *Template = dyn_cast<FunctionTemplateDecl>(D)) {
if (isa<CXXConstructorDecl>(Template->getTemplatedDecl()))
return false;
} else if (auto *MethodDecl = dyn_cast<CXXMethodDecl>(D)) {
if (MethodDecl->isCopyAssignmentOperator() && MethodDecl->isDeleted())
return false;
}
}
if (RD->hasNonTrivialDestructor())
Expand Down
92 changes: 92 additions & 0 deletions clang/test/CodeGen/x64-microsoft-arguments.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// RUN: %clang_cc1 -triple x86_64-windows-msvc -ffreestanding -emit-llvm -O0 \
// RUN: -x c++ -o - %s | FileCheck %s

int global_i = 0;

// Pass and return object with a reference type (pass directly, return indirectly).
// CHECK: define dso_local void @"?f1@@YA?AUS1@@XZ"(ptr dead_on_unwind noalias writable sret(%struct.S1) align 8 {{.*}})
// CHECK: call void @"?func1@@YA?AUS1@@U1@@Z"(ptr dead_on_unwind writable sret(%struct.S1) align 8 {{.*}}, i64 {{.*}})
struct S1 {
int& r;
};

S1 func1(S1 x);
S1 f1() {
S1 x{ global_i };
return func1(x);
}

// Pass and return object with a reference type within an inner struct (pass directly, return indirectly).
// CHECK: define dso_local void @"?f2@@YA?AUS2@@XZ"(ptr dead_on_unwind noalias writable sret(%struct.S2) align 8 {{.*}})
// CHECK: call void @"?func2@@YA?AUS2@@U1@@Z"(ptr dead_on_unwind writable sret(%struct.S2) align 8 {{.*}}, i64 {{.*}})
struct Inner {
int& r;
};

struct S2 {
Inner i;
};

S2 func2(S2 x);
S2 f2() {
S2 x{ { global_i } };
return func2(x);
}

// Pass and return object with a reference type (pass directly, return indirectly).
// CHECK: define dso_local void @"?f3@@YA?AUS3@@XZ"(ptr dead_on_unwind noalias writable sret(%struct.S3) align 8 {{.*}})
// CHECK: call void @"?func3@@YA?AUS3@@U1@@Z"(ptr dead_on_unwind writable sret(%struct.S3) align 8 {{.*}}, i64 {{.*}})
struct S3 {
const int& r;
};

S3 func3(S3 x);
S3 f3() {
S3 x{ global_i };
return func3(x);
}

// Pass and return object with a reference type within an inner struct (pass directly, return indirectly).
// CHECK: define dso_local void @"?f4@@YA?AUS4@@XZ"(ptr dead_on_unwind noalias writable sret(%struct.S4) align 8 {{.*}})
// CHECK: call void @"?func4@@YA?AUS4@@U1@@Z"(ptr dead_on_unwind writable sret(%struct.S4) align 8 {{.*}}, i64 {{.*}})
struct InnerConst {
const int& r;
};

struct S4 {
InnerConst i;
};

S4 func4(S4 x);
S4 f4() {
S4 x{ { global_i } };
return func4(x);
}

// Pass and return an object with an explicitly deleted copy assignment operator (pass directly, return indirectly).
// CHECK: define dso_local void @"?f5@@YA?AUS5@@XZ"(ptr dead_on_unwind noalias writable sret(%struct.S5) align 4 {{.*}})
// CHECK: call void @"?func5@@YA?AUS5@@U1@@Z"(ptr dead_on_unwind writable sret(%struct.S5) align 4 {{.*}}, i32 {{.*}})
struct S5 {
S5& operator=(const S5&) = delete;
int i;
};

S5 func5(S5 x);
S5 f5() {
S5 x{ 1 };
return func5(x);
}

// Pass and return an object with an explicitly defaulted copy assignment operator that is implicitly deleted (pass directly, return indirectly).
// CHECK: define dso_local void @"?f6@@YA?AUS6@@XZ"(ptr dead_on_unwind noalias writable sret(%struct.S6) align 8 {{.*}})
// CHECK: call void @"?func6@@YA?AUS6@@U1@@Z"(ptr dead_on_unwind writable sret(%struct.S6) align 8 {{.*}}, i64 {{.*}})
struct S6 {
S6& operator=(const S6&) = default;
int& i;
};

S6 func6(S6 x);
S6 f6() {
S6 x{ global_i };
return func6(x);
}

0 comments on commit 3f37397

Please sign in to comment.