https://github.com/glebov-andrey updated
https://github.com/llvm/llvm-project/pull/114075
>From 2b3865e5f11235c88b24940745e65e0feed1863b Mon Sep 17 00:00:00 2001
From: Andrey Glebov <andrey458641...@gmail.com>
Date: Mon, 28 Oct 2024 00:02:51 +0300
Subject: [PATCH] [clang] MicrosoftCXXABI: restore the RecordToCopyCtor table
when loading AST (#53486)
- Includes a regression test for the issue
---
.../clang/AST/CXXRecordDeclDefinitionBits.def | 5 +++++
clang/include/clang/AST/DeclCXX.h | 8 ++++++++
clang/lib/AST/DeclCXX.cpp | 7 ++++---
clang/lib/Sema/SemaExprCXX.cpp | 4 ++++
clang/lib/Serialization/ASTReaderDecl.cpp | 14 ++++++++++++++
.../PCH/cxx-exception-copy-ctor-crash.cpp | 19 +++++++++++++++++++
.../test/PCH/cxx-exception-copy-ctor-crash.h | 14 ++++++++++++++
7 files changed, 68 insertions(+), 3 deletions(-)
create mode 100644 clang/test/PCH/cxx-exception-copy-ctor-crash.cpp
create mode 100644 clang/test/PCH/cxx-exception-copy-ctor-crash.h
diff --git a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
index 6620840df0ced2..7560691a0e4172 100644
--- a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
+++ b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
@@ -249,6 +249,11 @@ FIELD(HasDeclaredCopyAssignmentWithConstParam, 1, MERGE_OR)
/// base classes or fields have a no-return destructor
FIELD(IsAnyDestructorNoReturn, 1, NO_MERGE)
+/// Microsoft CXX ABI specific:
+/// Whether the copy constructor is used by a `throw` expression.
+/// Used by ASTReader to restore the sidecar RecordToCopyCtor LUT.
+FIELD(HasCopyConstructorForExceptionObject, 1, MERGE_OR)
+
/// Whether the record type is intangible (if any base classes or fields have
/// type that is intangible). HLSL only.
FIELD(IsHLSLIntangible, 1, NO_MERGE)
diff --git a/clang/include/clang/AST/DeclCXX.h
b/clang/include/clang/AST/DeclCXX.h
index fa3f4ec98eb369..c143787327975e 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1558,6 +1558,14 @@ class CXXRecordDecl : public RecordDecl {
/// a field or in base class.
bool isHLSLIntangible() const { return data().IsHLSLIntangible; }
+ bool hasCopyConstructorForExceptionObject() const {
+ return data().HasCopyConstructorForExceptionObject;
+ }
+
+ void setHasCopyConstructorForExceptionObject() {
+ data().HasCopyConstructorForExceptionObject = true;
+ }
+
/// If the class is a local class [class.local], returns
/// the enclosing function declaration.
const FunctionDecl *isLocalClass() const {
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 44f45898fb483d..e1afac75f3fc84 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -109,9 +109,10 @@
CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D)
ImplicitCopyAssignmentHasConstParam(true),
HasDeclaredCopyConstructorWithConstParam(false),
HasDeclaredCopyAssignmentWithConstParam(false),
- IsAnyDestructorNoReturn(false), IsHLSLIntangible(false), IsLambda(false),
- IsParsingBaseSpecifiers(false), ComputedVisibleConversions(false),
- HasODRHash(false), Definition(D) {}
+ IsAnyDestructorNoReturn(false),
+ HasCopyConstructorForExceptionObject(false), IsHLSLIntangible(false),
+ IsLambda(false), IsParsingBaseSpecifiers(false),
+ ComputedVisibleConversions(false), HasODRHash(false), Definition(D) {}
CXXBaseSpecifier *CXXRecordDecl::DefinitionData::getBasesSlowCase() const {
return Bases.get(Definition->getASTContext().getExternalSource());
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 1e39d69e8b230f..8f6b760ad2842c 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1082,6 +1082,10 @@ bool Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc,
// friendship or any other means).
Context.addCopyConstructorForExceptionObject(Subobject, CD);
+ // Store the bit in CXXRecordDecl so that ASTReader can restore this
+ // mapping later.
+ Subobject->setHasCopyConstructorForExceptionObject();
+
// We don't keep the instantiated default argument expressions around so
// we must rebuild them here.
for (unsigned I = 1, E = CD->getNumParams(); I != E; ++I) {
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 0b75468a94103f..b5ce91eff614cb 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2327,6 +2327,20 @@ void
ASTDeclReader::VisitCXXConstructorDecl(CXXConstructorDecl *D) {
}
VisitCXXMethodDecl(D);
+
+ // Microsoft CXX ABI specific:
+ // Restore the RecordToCopyCtor sidecar LUT entry so that `throw` expressions
+ // find the correct copy constructor for exceptions during codegen.
+ // There is no need to check the target info because the
+ // HasCopyConstructorForExceptionObject bit only gets set for the MS ABI.
+ if (D->isCopyConstructor()) {
+ // TODO What if this is not the same copy constructor which was chosen by
+ // LookupCopyingConstructor() in SemaExprCXX? Is there a better way?
+ auto *R = cast<CXXRecordDecl>(D->getDeclContext());
+ if (R->hasCopyConstructorForExceptionObject()) {
+ Reader.getContext().addCopyConstructorForExceptionObject(R, D);
+ }
+ }
}
void ASTDeclReader::VisitCXXDestructorDecl(CXXDestructorDecl *D) {
diff --git a/clang/test/PCH/cxx-exception-copy-ctor-crash.cpp
b/clang/test/PCH/cxx-exception-copy-ctor-crash.cpp
new file mode 100644
index 00000000000000..29bcb114f20f0a
--- /dev/null
+++ b/clang/test/PCH/cxx-exception-copy-ctor-crash.cpp
@@ -0,0 +1,19 @@
+// REQUIRES: system-windows, target={{.*-windows-msvc}}
+// RUN: %clang_cc1 -x c++ -std=c++17 -fcxx-exceptions -fexceptions
-triple=%ms_abi_triple -emit-pch -building-pch-with-obj -fmodules-codegen -o
%t.pch %S/cxx-exception-copy-ctor-crash.h
+// RUN: %clang_cc1 -x c++ -std=c++17 -fcxx-exceptions -fexceptions
-triple=%ms_abi_triple -include-pch %t.pch -emit-obj -building-pch-with-obj
-fmodules-codegen -o %t.pch.obj
+// RUN: %clang_cc1 -x c++ -std=c++17 -fcxx-exceptions -fexceptions
-triple=%ms_abi_triple -include-pch %t.pch -emit-obj -o %t.obj %s
+// RUN: lld-link -subsystem:console -out:%t.exe %t.pch.obj %t.obj libucrt.lib
libvcruntime.lib libcmt.lib
+// RUN: %t.exe
+
+// Regression test for https://github.com/llvm/llvm-project/issues/53486
+
+int main() {
+ try {
+ throw Exception();
+ } catch (const Exception ex) { // catch by value to trigger copy constructor
+ }
+ if (ctor_count != dtor_count) {
+ return 1;
+ }
+ return 0;
+}
diff --git a/clang/test/PCH/cxx-exception-copy-ctor-crash.h
b/clang/test/PCH/cxx-exception-copy-ctor-crash.h
new file mode 100644
index 00000000000000..9645df56c786e3
--- /dev/null
+++ b/clang/test/PCH/cxx-exception-copy-ctor-crash.h
@@ -0,0 +1,14 @@
+// Header for PCH test cxx-exception-copy-ctor-crash.cpp
+
+inline int ctor_count = 0;
+inline int dtor_count = 0;
+
+struct Exception {
+ Exception() { ++ctor_count; }
+ ~Exception() { ++dtor_count; }
+ Exception(const Exception &) noexcept { ++ctor_count; }
+};
+
+inline void throw_exception() {
+ throw Exception();
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
