Merge pull request #84430 from Xazax-hun/unbalanced-refcount-in-pods-on-6.2

[6.2][cxx-interop] Fix over-releasing reference members of trivial 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"
@@ -369,6 +370,7 @@ namespace {
       : public StructTypeInfoBase<LoadableClangRecordTypeInfo, LoadableTypeInfo,
                                   ClangFieldInfo> {
     const clang::RecordDecl *ClangDecl;
+    bool HasReferenceField;
 
   public:
     LoadableClangRecordTypeInfo(ArrayRef<ClangFieldInfo> fields,
@@ -377,14 +379,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,
@@ -448,6 +450,7 @@ namespace {
                                    const ClangFieldInfo &field) const {
       llvm_unreachable("non-fixed field in Clang type?");
     }
+    bool hasReferenceField() const { return HasReferenceField; }
   };
 
   class AddressOnlyPointerAuthRecordTypeInfo final
@@ -1320,6 +1323,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,
@@ -1360,11 +1368,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:
@@ -1489,6 +1498,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

@@ -0,0 +1,64 @@
+#pragma once
+
+#include <stdio.h>
+#include <swift/bridging>
+
+class SharedFRT {
+public:
+  SharedFRT() : _refCount(1) { logMsg("Ctor"); }
+
+private:
+  void logMsg(const char *s) const {
+    printf("RefCount: %d, message: %s\n", _refCount, s);
+  }
+
+  ~SharedFRT() { logMsg("Dtor"); }
+  SharedFRT(const SharedFRT &) = delete;
+  SharedFRT &operator=(const SharedFRT &) = delete;
+  SharedFRT(SharedFRT &&) = delete;
+  SharedFRT &operator=(SharedFRT &&) = delete;
+
+  int _refCount;
+
+  friend void retainSharedFRT(SharedFRT *_Nonnull);
+  friend void releaseSharedFRT(SharedFRT *_Nonnull);
+} SWIFT_SHARED_REFERENCE(retainSharedFRT, releaseSharedFRT);
+
+class MyToken {
+public:
+  MyToken() = default;
+  MyToken(MyToken const &) {}
+};
+
+inline void retainSharedFRT(SharedFRT *_Nonnull x) {
+  ++x->_refCount;
+  x->logMsg("retain");
+}
+
+inline void releaseSharedFRT(SharedFRT *_Nonnull x) {
+  --x->_refCount;
+  x->logMsg("release");
+  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/Inputs/module.modulemap

@@ -83,3 +83,8 @@ module VirtMethodWithRvalRef {
   header "virtual-methods-with-rvalue-reference.h"
   requires cplusplus
 }
+
+module LoggingFrts {
+    header "logging-frts.h"
+    requires cplusplus
+}

test/Interop/Cxx/foreign-reference/frts-as-fields.swift

@@ -0,0 +1,27 @@
+// RUN: %target-run-simple-swift(-I %swift_src_root/lib/ClangImporter/SwiftBridging -I %S/Inputs -cxx-interoperability-mode=default -Xfrontend -disable-availability-checking -Onone) | %FileCheck %s
+
+// REQUIRES: executable_test
+
+import LoggingFrts
+
+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