Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang codegen] Fix MS ABI detection of user-provided constructors. #90151

Merged
merged 2 commits into from
Apr 29, 2024

Conversation

efriedma-quic
Copy link
Collaborator

In the context of determining whether a class counts as an "aggregate", a constructor template counts as a user-provided constructor.

Fixes #86384

In the context of determining whether a class counts as an "aggregate",
a constructor template counts as a user-provided constructor.

Fixes llvm#86384
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen labels Apr 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 26, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Eli Friedman (efriedma-quic)

Changes

In the context of determining whether a class counts as an "aggregate", a constructor template counts as a user-provided constructor.

Fixes #86384


Full diff: https://github.com/llvm/llvm-project/pull/90151.diff

2 Files Affected:

  • (modified) clang/lib/CodeGen/MicrosoftCXXABI.cpp (+9-3)
  • (modified) clang/test/CodeGen/arm64-microsoft-arguments.cpp (+14)
diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index d38a26940a3cb6..d47927745759e1 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1131,9 +1131,15 @@ static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
     return false;
   if (RD->hasNonTrivialCopyAssignment())
     return false;
-  for (const CXXConstructorDecl *Ctor : RD->ctors())
-    if (Ctor->isUserProvided())
-      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;
+    }
+  }
   if (RD->hasNonTrivialDestructor())
     return false;
   return true;
diff --git a/clang/test/CodeGen/arm64-microsoft-arguments.cpp b/clang/test/CodeGen/arm64-microsoft-arguments.cpp
index e8309888dcfe21..89299277422a29 100644
--- a/clang/test/CodeGen/arm64-microsoft-arguments.cpp
+++ b/clang/test/CodeGen/arm64-microsoft-arguments.cpp
@@ -201,3 +201,17 @@ S11 f11() {
   S11 x;
   return func11(x);
 }
+
+// Pass and return object with template constructor (pass directly,
+// return indirectly).
+// CHECK: define dso_local void @"?f12@@YA?AUS12@@XZ"(ptr dead_on_unwind inreg noalias writable sret(%struct.S12) align 4 {{.*}})
+// CHECK: call void @"?func12@@YA?AUS12@@U1@@Z"(ptr dead_on_unwind inreg writable sret(%struct.S12) align 4 {{.*}}, i64 {{.*}})
+struct S12 {
+  template<typename T> S12(T*) {}
+  int x;
+};
+S12 func12(S12 x);
+S12 f12() {
+  S12 x((int*)0);
+  return func12(x);
+}

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a changelog entry mentioning the fixed issue ?
Otherwise LGTM

@efriedma-quic
Copy link
Collaborator Author

Added release note.

Copy link
Collaborator

@rnk rnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@efriedma-quic efriedma-quic merged commit 3ab4ae9 into llvm:main Apr 29, 2024
5 checks passed
@efriedma-quic efriedma-quic deleted the ms-abi-user-provided-template branch April 29, 2024 19:00
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Apr 30, 2024
…lvm#90151)

In the context of determining whether a class counts as an "aggregate",
a constructor template counts as a user-provided constructor.

Fixes llvm#86384

(cherry picked from commit 3ab4ae9)
tstellar pushed a commit to llvmbot/llvm-project that referenced this pull request May 1, 2024
…lvm#90151)

In the context of determining whether a class counts as an "aggregate",
a constructor template counts as a user-provided constructor.

Fixes llvm#86384

(cherry picked from commit 3ab4ae9)
@pointhex pointhex mentioned this pull request May 7, 2024
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Sep 9, 2024
…lvm#90151)

In the context of determining whether a class counts as an "aggregate",
a constructor template counts as a user-provided constructor.

Fixes llvm#86384

(cherry picked from commit 3ab4ae9)
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 10, 2024
…lvm#90151)

In the context of determining whether a class counts as an "aggregate",
a constructor template counts as a user-provided constructor.

Fixes llvm#86384

(cherry picked from commit 3ab4ae9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ABI mismatch between MSVC and Clang when returning std::pair on Windows ARM64
4 participants