Merge pull request #84321 from Xazax-hun/unbalanced-refcount-in-pods

[cxx-interop] Fix over-releasing reference members of trival C++ types

Author: Xazax-hun

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<LoadableClangRecordTypeInfo, LoadableTypeInfo,
                                   ClangFieldInfo> {
     const clang::RecordDecl *ClangDecl;
+    bool HasReferenceField;
 
   public:
     LoadableClangRecordTypeInfo(ArrayRef<ClangFieldInfo> 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<LoadableClangRecordTypeInfo>().hasReferenceField())
+          hasReferenceField = true;
       return;
     }
 

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()}};
+}

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
+// 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