Skip to content

Commit eef3907

Browse files
committed
[clang] MicrosoftCXXABI: restore the RecordToCopyCtor table when loading AST (#53486)
- Includes a regression test for the issue
1 parent 3b2b7ec commit eef3907

File tree

7 files changed

+68
-3
lines changed

7 files changed

+68
-3
lines changed

clang/include/clang/AST/CXXRecordDeclDefinitionBits.def

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,11 @@ FIELD(HasDeclaredCopyAssignmentWithConstParam, 1, MERGE_OR)
249249
/// base classes or fields have a no-return destructor
250250
FIELD(IsAnyDestructorNoReturn, 1, NO_MERGE)
251251

252+
/// Microsoft CXX ABI specific:
253+
/// Whether the copy constructor is used by a `throw` expression.
254+
/// Used by ASTReader to restore the sidecar RecordToCopyCtor LUT.
255+
FIELD(HasCopyConstructorForExceptionObject, 1, MERGE_OR)
256+
252257
/// Whether the record type is intangible (if any base classes or fields have
253258
/// type that is intangible). HLSL only.
254259
FIELD(IsHLSLIntangible, 1, NO_MERGE)

clang/include/clang/AST/DeclCXX.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,6 +1558,14 @@ class CXXRecordDecl : public RecordDecl {
15581558
/// a field or in base class.
15591559
bool isHLSLIntangible() const { return data().IsHLSLIntangible; }
15601560

1561+
bool hasCopyConstructorForExceptionObject() const {
1562+
return data().HasCopyConstructorForExceptionObject;
1563+
}
1564+
1565+
void setHasCopyConstructorForExceptionObject() {
1566+
data().HasCopyConstructorForExceptionObject = true;
1567+
}
1568+
15611569
/// If the class is a local class [class.local], returns
15621570
/// the enclosing function declaration.
15631571
const FunctionDecl *isLocalClass() const {

clang/lib/AST/DeclCXX.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,10 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D)
109109
ImplicitCopyAssignmentHasConstParam(true),
110110
HasDeclaredCopyConstructorWithConstParam(false),
111111
HasDeclaredCopyAssignmentWithConstParam(false),
112-
IsAnyDestructorNoReturn(false), IsHLSLIntangible(false), IsLambda(false),
113-
IsParsingBaseSpecifiers(false), ComputedVisibleConversions(false),
114-
HasODRHash(false), Definition(D) {}
112+
IsAnyDestructorNoReturn(false),
113+
HasCopyConstructorForExceptionObject(false), IsHLSLIntangible(false),
114+
IsLambda(false), IsParsingBaseSpecifiers(false),
115+
ComputedVisibleConversions(false), HasODRHash(false), Definition(D) {}
115116

116117
CXXBaseSpecifier *CXXRecordDecl::DefinitionData::getBasesSlowCase() const {
117118
return Bases.get(Definition->getASTContext().getExternalSource());

clang/lib/Sema/SemaExprCXX.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,6 +1082,10 @@ bool Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc,
10821082
// friendship or any other means).
10831083
Context.addCopyConstructorForExceptionObject(Subobject, CD);
10841084

1085+
// Store the bit in CXXRecordDecl so that ASTReader can restore this
1086+
// mapping later.
1087+
Subobject->setHasCopyConstructorForExceptionObject();
1088+
10851089
// We don't keep the instantiated default argument expressions around so
10861090
// we must rebuild them here.
10871091
for (unsigned I = 1, E = CD->getNumParams(); I != E; ++I) {

clang/lib/Serialization/ASTReaderDecl.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2327,6 +2327,20 @@ void ASTDeclReader::VisitCXXConstructorDecl(CXXConstructorDecl *D) {
23272327
}
23282328

23292329
VisitCXXMethodDecl(D);
2330+
2331+
// Microsoft CXX ABI specific:
2332+
// Restore the RecordToCopyCtor sidecar LUT entry so that `throw` expressions
2333+
// find the correct copy constructor for exceptions during codegen.
2334+
// There is no need to check the target info because the
2335+
// HasCopyConstructorForExceptionObject bit only gets set for the MS ABI.
2336+
if (D->isCopyConstructor()) {
2337+
// TODO What if this is not the same copy constructor which was chosen by
2338+
// LookupCopyingConstructor() in SemaExprCXX? Is there a better way?
2339+
auto *R = cast<CXXRecordDecl>(D->getDeclContext());
2340+
if (R->hasCopyConstructorForExceptionObject()) {
2341+
Reader.getContext().addCopyConstructorForExceptionObject(R, D);
2342+
}
2343+
}
23302344
}
23312345

23322346
void ASTDeclReader::VisitCXXDestructorDecl(CXXDestructorDecl *D) {
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// REQUIRES: system-windows, target={{.*-windows-msvc}}
2+
// 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
3+
// 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
4+
// 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
5+
// RUN: lld-link -subsystem:console -out:%t.exe %t.pch.obj %t.obj libucrt.lib libvcruntime.lib libcmt.lib
6+
// RUN: %t.exe
7+
8+
// Regression test for https://github.com/llvm/llvm-project/issues/53486
9+
10+
int main() {
11+
try {
12+
throw Exception();
13+
} catch (const Exception ex) { // catch by value to trigger copy constructor
14+
}
15+
if (ctor_count != dtor_count) {
16+
return 1;
17+
}
18+
return 0;
19+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Header for PCH test cxx-exception-copy-ctor-crash.cpp
2+
3+
inline int ctor_count = 0;
4+
inline int dtor_count = 0;
5+
6+
struct Exception {
7+
Exception() { ++ctor_count; }
8+
~Exception() { ++dtor_count; }
9+
Exception(const Exception &) noexcept { ++ctor_count; }
10+
};
11+
12+
inline void throw_exception() {
13+
throw Exception();
14+
}

0 commit comments

Comments
 (0)