From e4e54236caf714f52f41ece1788edbf189d1ec93 Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Tue, 16 Sep 2025 14:15:20 +0100 Subject: [PATCH] [cxx-interop] Fix over-releasing reference members of trival C++ types Large trivial types were copied via memcpy instead of doing a field-wise copy. This is incorrect for types with reference fields where we also need to bump the corresponding refcounts. rdar://160315343 --- lib/IRGen/GenStruct.cpp | 40 ++++++++++++++----- .../foreign-reference/Inputs/logging-frts.h | 21 ++++++++++ .../foreign-reference/frts-as-fields.swift | 24 ++++++++++- 3 files changed, 74 insertions(+), 11 deletions(-) diff --git a/lib/IRGen/GenStruct.cpp b/lib/IRGen/GenStruct.cpp index 75dacf148d030..64465238062ae 100644 --- a/lib/IRGen/GenStruct.cpp +++ b/lib/IRGen/GenStruct.cpp @@ -22,6 +22,7 @@ #include "swift/AST/Decl.h" #include "swift/AST/IRGenOptions.h" #include "swift/AST/Pattern.h" +#include "swift/AST/ReferenceCounting.h" #include "swift/AST/SemanticAttrs.h" #include "swift/AST/SubstitutionMap.h" #include "swift/AST/Types.h" @@ -368,6 +369,7 @@ namespace { : public StructTypeInfoBase { const clang::RecordDecl *ClangDecl; + bool HasReferenceField; public: LoadableClangRecordTypeInfo(ArrayRef fields, @@ -376,14 +378,14 @@ namespace { Alignment align, IsTriviallyDestroyable_t isTriviallyDestroyable, IsCopyable_t isCopyable, - const clang::RecordDecl *clangDecl) + const clang::RecordDecl *clangDecl, + bool hasReferenceField) : StructTypeInfoBase(StructTypeInfoKind::LoadableClangRecordTypeInfo, fields, explosionSize, FieldsAreABIAccessible, storageType, size, std::move(spareBits), align, - isTriviallyDestroyable, - isCopyable, - IsFixedSize, IsABIAccessible), - ClangDecl(clangDecl) {} + isTriviallyDestroyable, isCopyable, IsFixedSize, + IsABIAccessible), + ClangDecl(clangDecl), HasReferenceField(hasReferenceField) {} TypeLayoutEntry *buildTypeLayoutEntry(IRGenModule &IGM, @@ -447,6 +449,7 @@ namespace { const ClangFieldInfo &field) const { llvm_unreachable("non-fixed field in Clang type?"); } + bool hasReferenceField() const { return HasReferenceField; } }; class AddressOnlyPointerAuthRecordTypeInfo final @@ -1319,6 +1322,11 @@ class ClangRecordLowering { Size NextOffset = Size(0); Size SubobjectAdjustment = Size(0); unsigned NextExplosionIndex = 0; + // Types that are trivial in C++ but are containing fields to reference types + // are not trivial in Swift, they cannot be copied using memcpy as we need to + // do the proper retain operations. + bool hasReferenceField = false; + public: ClangRecordLowering(IRGenModule &IGM, StructDecl *swiftDecl, const clang::RecordDecl *clangDecl, @@ -1359,11 +1367,12 @@ class ClangRecordLowering { return LoadableClangRecordTypeInfo::create( FieldInfos, NextExplosionIndex, llvmType, TotalStride, std::move(SpareBits), TotalAlignment, - (SwiftDecl && SwiftDecl->getValueTypeDestructor()) - ? IsNotTriviallyDestroyable : IsTriviallyDestroyable, - (SwiftDecl && !SwiftDecl->canBeCopyable()) - ? IsNotCopyable : IsCopyable, - ClangDecl); + (SwiftDecl && + (SwiftDecl->getValueTypeDestructor() || hasReferenceField)) + ? IsNotTriviallyDestroyable + : IsTriviallyDestroyable, + (SwiftDecl && !SwiftDecl->canBeCopyable()) ? IsNotCopyable : IsCopyable, + ClangDecl, hasReferenceField); } private: @@ -1488,6 +1497,17 @@ class ClangRecordLowering { SwiftType.getFieldType(swiftField, IGM.getSILModule(), IGM.getMaximalTypeExpansionContext()))); addField(swiftField, offset, fieldTI, isZeroSized); + auto fieldTy = + swiftField->getInterfaceType()->lookThroughSingleOptionalType(); + if (fieldTy->isAnyClassReferenceType() && + fieldTy->getReferenceCounting() != ReferenceCounting::None) + hasReferenceField = true; + else if (auto structDecl = fieldTy->getStructOrBoundGenericStruct(); + structDecl && structDecl->hasClangNode() && + getStructTypeInfoKind(fieldTI) == + StructTypeInfoKind::LoadableClangRecordTypeInfo) + if (fieldTI.as().hasReferenceField()) + hasReferenceField = true; return; } diff --git a/test/Interop/Cxx/foreign-reference/Inputs/logging-frts.h b/test/Interop/Cxx/foreign-reference/Inputs/logging-frts.h index 0b7c8b5c3019e..dcee735b5ad94 100644 --- a/test/Interop/Cxx/foreign-reference/Inputs/logging-frts.h +++ b/test/Interop/Cxx/foreign-reference/Inputs/logging-frts.h @@ -41,3 +41,24 @@ inline void releaseSharedFRT(SharedFRT *_Nonnull x) { if (x->_refCount == 0) delete x; } + +struct LargeStructWithRefCountedField { + void const *a; + void const *b; + unsigned long c; + unsigned d; + SharedFRT *e; +}; + +struct LargeStructWithRefCountedFieldNested { + int a; + LargeStructWithRefCountedField b; +}; + +inline LargeStructWithRefCountedField getStruct() { + return {0, 0, 0, 0, new SharedFRT()}; +} + +inline LargeStructWithRefCountedFieldNested getNestedStruct() { + return {0, {0, 0, 0, 0, new SharedFRT()}}; +} diff --git a/test/Interop/Cxx/foreign-reference/frts-as-fields.swift b/test/Interop/Cxx/foreign-reference/frts-as-fields.swift index 7fcd7a4564ecf..6bb4609bcfbe2 100644 --- a/test/Interop/Cxx/foreign-reference/frts-as-fields.swift +++ b/test/Interop/Cxx/foreign-reference/frts-as-fields.swift @@ -27,4 +27,26 @@ go() // CHECK-NEXT: RefCount: 2, message: retain // CHECK-NEXT: RefCount: 1, message: release // CHECK-NEXT: RefCount: 0, message: release -// CHECK-NEXT: RefCount: 0, message: Dtor \ No newline at end of file +// CHECK-NEXT: RefCount: 0, message: Dtor + +func takesLargeStructWithRefCountedField(_ x: LargeStructWithRefCountedField) { + var a = x +} + +takesLargeStructWithRefCountedField(getStruct()) +// CHECK: RefCount: 1, message: Ctor +// CHECK-NEXT: RefCount: 2, message: retain +// CHECK-NEXT: RefCount: 1, message: release +// CHECK-NEXT: RefCount: 0, message: release +// CHECK-NEXT: RefCount: 0, message: Dtor + +func takesLargeStructWithRefCountedFieldNested(_ x: LargeStructWithRefCountedFieldNested) { + var a = x +} + +takesLargeStructWithRefCountedFieldNested(getNestedStruct()) +// CHECK: RefCount: 1, message: Ctor +// CHECK-NEXT: RefCount: 2, message: retain +// CHECK-NEXT: RefCount: 1, message: release +// CHECK-NEXT: RefCount: 0, message: release +// CHECK-NEXT: RefCount: 0, message: Dtor