Merge pull request #84612 from swiftlang/fix-overrelease-destroy-hoist

[cxx-interop] Delay lowering unowned convention until ownership elimination

lib/SIL/IR/SILType.cpp

@@ -655,6 +655,11 @@ SILResultInfo::getOwnershipKind(SILFunction &F,
   case ResultConvention::Owned:
     return OwnershipKind::Owned;
   case ResultConvention::Unowned:
+    // We insert a retain right after the call returning an unowned value in
+    // OwnershipModelEliminator. So treat the result as owned.
+    if (IsTrivial)
+      return OwnershipKind::None;
+    return OwnershipKind::Owned;
   case ResultConvention::UnownedInnerPointer:
     if (IsTrivial)
       return OwnershipKind::None;

lib/SILGen/SILGenApply.cpp

@@ -6215,13 +6215,12 @@ RValue SILGenFunction::emitApply(
                                  B.getDefaultAtomicity());
         hasAlreadyLifetimeExtendedSelf = true;
       }
-      LLVM_FALLTHROUGH;
-
-    case ResultConvention::Unowned:
-      // Unretained. Retain the value.
       result = resultTL.emitCopyValue(B, loc, result);
       break;
 
+    case ResultConvention::Unowned:
+      // Handled in OwnershipModelEliminator.
+      break;
     case ResultConvention::GuaranteedAddress:
     case ResultConvention::Guaranteed:
       llvm_unreachable("borrow accessor is not yet implemented");

lib/SILGen/SILGenPoly.cpp

@@ -5290,6 +5290,7 @@ void ResultPlanner::execute(SmallVectorImpl<SILValue> &innerDirectResultStack,
       LLVM_FALLTHROUGH;
     case ResultConvention::Owned:
     case ResultConvention::Autoreleased:
+    case ResultConvention::Unowned: // Handled in OwnershipModelEliminator.
       return SGF.emitManagedRValueWithCleanup(resultValue, resultTL);
     case ResultConvention::Pack:
       llvm_unreachable("shouldn't have direct result with pack results");
@@ -5299,8 +5300,6 @@ void ResultPlanner::execute(SmallVectorImpl<SILValue> &innerDirectResultStack,
       // originally 'self'.
       SGF.SGM.diagnose(Loc.getSourceLoc(), diag::not_implemented,
                        "reabstraction of returns_inner_pointer function");
-      LLVM_FALLTHROUGH;
-    case ResultConvention::Unowned:
       return SGF.emitManagedCopy(Loc, resultValue, resultTL);
     case ResultConvention::GuaranteedAddress:
     case ResultConvention::Guaranteed:

lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp

@@ -22,6 +22,8 @@
 ///
 //===----------------------------------------------------------------------===//
 
+#include "swift/AST/Types.h"
+#include "swift/SIL/SILValue.h"
 #define DEBUG_TYPE "sil-ownership-model-eliminator"
 
 #include "swift/Basic/Assertions.h"
@@ -404,6 +406,20 @@ bool OwnershipModelEliminatorVisitor::visitApplyInst(ApplyInst *ai) {
     changed = true;
   }
 
+  // Insert a retain for unowned results.
+  SILBuilderWithScope builder(ai->getNextInstruction(), builderCtx);
+  auto resultIt = fnConv.getDirectSILResults().begin();
+  auto copyValue = [&](unsigned idx, SILValue v) {
+    auto result = *resultIt;
+    if (result.getConvention() == ResultConvention::Unowned)
+      builder.emitCopyValueOperation(ai->getLoc(), v);
+    ++resultIt;
+  };
+  if (fnConv.getNumDirectSILResults() == 1)
+    copyValue(0, ai);
+  else
+    builder.emitDestructureValueOperation(ai->getLoc(), ai, copyValue);
+
   return changed;
 }
 

test/Interop/Cxx/foreign-reference/Inputs/logging-frts.h

@@ -7,12 +7,14 @@ class SharedFRT {
 public:
   SharedFRT() : _refCount(1) { logMsg("Ctor"); }
 
+protected:
+  virtual ~SharedFRT() { logMsg("Dtor"); }
+
 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;
@@ -62,3 +64,24 @@ inline LargeStructWithRefCountedField getStruct() {
 inline LargeStructWithRefCountedFieldNested getNestedStruct() {
   return {0, {0, 0, 0, 0, new SharedFRT()}};
 }
+
+template <class T>
+struct Ref {
+  T *_Nonnull ptr() const { return ref; }
+  T *ref;
+};
+
+class Payload final : public SharedFRT {
+public:
+  static Ref<Payload> create(int value) {
+    Ref<Payload> ref;
+    ref.ref = new Payload(value);
+    return ref;
+  }
+
+  int value() const { return m_value; }
+
+private:
+  explicit Payload(int value) : m_value(value) {}
+  int m_value;
+};

test/Interop/Cxx/foreign-reference/get-frt-from-smart-ptr.swift

@@ -0,0 +1,26 @@
+// RUN: %target-run-simple-swift(-I %swift_src_root/lib/ClangImporter/SwiftBridging -I %S/Inputs -cxx-interoperability-mode=default -Xfrontend -disable-availability-checking -O) | %FileCheck %s
+
+// REQUIRES: executable_test
+
+import LoggingFrts
+
+@inline(never)
+func use(_ x: CInt) {
+    print("Value is \(x).")
+}
+
+func testRefIssues() {
+    var a2 = Optional.some(Payload.create(0));
+    let b2 = a2!.ptr();
+    a2 = Optional.none;
+    let v2 = b2.value();
+    use(v2)
+}
+testRefIssues()
+
+// CHECK:      RefCount: 1, message: Ctor
+// CHECK-NEXT: RefCount: 2, message: retain
+// CHECK-NEXT: RefCount: 1, message: release
+// CHECK-NEXT: Value is 0.
+// CHECK-NEXT: RefCount: 0, message: release
+// CHECK-NEXT: RefCount: 0, message: Dtor

test/SIL/OwnershipVerifier/use_verifier.sil

@@ -828,30 +828,6 @@ bb5(%5 : @owned $ThreeDifferingPayloadEnum):
   return %5 : $ThreeDifferingPayloadEnum
 }
 
-sil [ossa] @enum_cases_with_trivial_unowned_cases_arg_into_phi : $@convention(thin) (Builtin.NativeObject) -> ThreeDifferingPayloadEnum {
-bb0(%0 : @unowned $Builtin.NativeObject):
-  cond_br undef, bb1, bb2
-
-bb1:
-  cond_br undef, bb3, bb4
-
-bb2:
-  %1 = enum $ThreeDifferingPayloadEnum, #ThreeDifferingPayloadEnum.nopayload!enumelt
-  br bb5(%1 : $ThreeDifferingPayloadEnum)
-
-bb3:
-  %2 = enum $ThreeDifferingPayloadEnum, #ThreeDifferingPayloadEnum.nontrivial_payload!enumelt, %0 : $Builtin.NativeObject
-  br bb5(%2 : $ThreeDifferingPayloadEnum)
-
-bb4:
-  %3 = integer_literal $Builtin.Int32, 0
-  %4 = enum $ThreeDifferingPayloadEnum, #ThreeDifferingPayloadEnum.trivial_payload!enumelt, %3 : $Builtin.Int32
-  br bb5(%4 : $ThreeDifferingPayloadEnum)
-
-bb5(%5 : @unowned $ThreeDifferingPayloadEnum):
-  return %5 : $ThreeDifferingPayloadEnum
-}
-
 sil [ossa] @enum_cases_with_trivial_guaranteed_cases_arg_into_phi : $@convention(thin) (@guaranteed Builtin.NativeObject) -> @owned ThreeDifferingPayloadEnum {
 bb0(%0 : @guaranteed $Builtin.NativeObject):
   cond_br undef, bb1, bb2
@@ -1383,20 +1359,6 @@ bb3(%fUnknown : @owned $@callee_owned () -> ()):
   return %9999 : $()
 }
 
-sil [ossa] @unowned_to_ref_is_unowned_instant_use : $@convention(thin) (@guaranteed Builtin.NativeObject) -> Builtin.NativeObject {
-bb0(%0 : @guaranteed $Builtin.NativeObject):
-  %1 = ref_to_unowned %0 : $Builtin.NativeObject to $@sil_unowned Builtin.NativeObject
-  %2 = unowned_to_ref %1 : $@sil_unowned Builtin.NativeObject to $Builtin.NativeObject
-  return %2 : $Builtin.NativeObject
-}
-
-sil [ossa] @unmanaged_to_ref_is_unowned_instant_use : $@convention(thin) (@guaranteed Builtin.NativeObject) -> Builtin.NativeObject {
-bb0(%0 : @guaranteed $Builtin.NativeObject):
-  %1 = ref_to_unmanaged %0 : $Builtin.NativeObject to $@sil_unmanaged Builtin.NativeObject
-  %2 = unmanaged_to_ref %1 : $@sil_unmanaged Builtin.NativeObject to $Builtin.NativeObject
-  return %2 : $Builtin.NativeObject
-}
-
 sil [ossa] @nontrivial_enum_unchecked_enum_data_trivial_payload_owned : $@convention(thin) (@owned ThreeDifferingPayloadEnum) -> Builtin.Int32 {
 bb0(%0 : @owned $ThreeDifferingPayloadEnum):
   // NOTE: It may be surprising that %0 is consumed by this unchecked_enum_data

test/SILOptimizer/cse_objc_ossa.sil

@@ -14,12 +14,13 @@ import Foundation
 // CHECK-NOT: objc_protocol
 // CHECK: tuple (%0 : $Protocol, %0 : $Protocol)
 // CHECK-LABEL: } // end sil function 'cse_objc_protocol'
-sil [ossa] @cse_objc_protocol : $@convention(thin) () -> (Protocol, Protocol) {
+sil [ossa] @cse_objc_protocol : $@convention(thin) () -> @owned (Protocol, Protocol) {
 bb0:
   %0 = objc_protocol #XX : $Protocol
   %1 = objc_protocol #XX : $Protocol
   %2 = tuple (%0: $Protocol, %1: $Protocol)
-  return %2 : $(Protocol, Protocol)
+  %3 = copy_value %2
+  return %3
 }
 
 @objc protocol Walkable {

test/SILOptimizer/cse_ossa.sil

@@ -700,12 +700,13 @@ bb0(%0 : $*Builtin.Int8):
 // CHECK-NOT: raw_pointer_to_ref
 // CHECK: tuple
 // CHECK-LABEL: } // end sil function 'cse_raw_pointer_to_ref'
-sil [ossa] @cse_raw_pointer_to_ref : $@convention(thin) (Builtin.RawPointer) -> (C, C) {
+sil [ossa] @cse_raw_pointer_to_ref : $@convention(thin) (Builtin.RawPointer) -> @owned (C, C) {
 bb0(%0 : $Builtin.RawPointer):
   %1 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $C
   %2 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $C
   %6 = tuple(%1: $C, %2: $C)
-  return %6 : $(C, C)
+  %7 = copy_value %6
+  return %7
 }
 
 // CHECK-LABEL: sil [ossa] @cse_unchecked_addr_cast :

test/SILOptimizer/cse_ossa_nontrivial.sil

@@ -255,19 +255,6 @@ bb0(%0 : @owned $Klass):
   return %6 : $()
 }
 
-// CHECK-LABEL: sil [ossa] @cse_raw_pointer_to_ref :
-// CHECK: raw_pointer_to_ref
-// CHECK-NOT: raw_pointer_to_ref
-// CHECK: tuple
-// CHECK-LABEL: } // end sil function 'cse_raw_pointer_to_ref'
-sil [ossa] @cse_raw_pointer_to_ref : $@convention(thin) (Builtin.RawPointer) -> (Klass, Klass) {
-bb0(%0 : $Builtin.RawPointer):
-  %1 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $Klass
-  %2 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $Klass
-  %6 = tuple(%1: $Klass, %2: $Klass)
-  return %6 : $(Klass, Klass)
-}
-
 enum Enum1 {
     case Case1
     case Case2

test/SILOptimizer/ossa_rauw_tests.sil

@@ -302,52 +302,57 @@ bbExitBlock(%result : @owned $FakeOptional<Klass>):
   return %result : $FakeOptional<Klass>
 }
 
-// CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_2 : $@convention(thin) (@guaranteed Klass) -> (Klass, Klass) {
+// CHECK-LABEL: sil [ossa] @owned_to_guaranteed_rauw_2 : $@convention(thin) (@guaranteed Klass) -> @owned (Klass, Klass) {
 // CHECK: bb0(
 // CHECK-NEXT: tuple
+// CHECK-NEXT: copy_value
 // CHECK-NEXT: return
-// CHECK: } // end sil function 'unowned_to_guaranteed_rauw_2'
-sil [ossa] @unowned_to_guaranteed_rauw_2 : $@convention(thin) (@guaranteed Klass) -> (Klass, Klass) {
+// CHECK: } // end sil function 'owned_to_guaranteed_rauw_2'
+sil [ossa] @owned_to_guaranteed_rauw_2 : $@convention(thin) (@guaranteed Klass) -> @owned (Klass, Klass) {
 bb0(%0 : @guaranteed $Klass):
   %1 = unchecked_bitwise_cast %0 : $Klass to $SubKlass
   %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass
   %3 = tuple(%2 : $Klass, %2 : $Klass)
-  return %3 : $(Klass, Klass)
+  %4 = copy_value %3
+  return %4 : $(Klass, Klass)
 }
 
-// CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_2a : $@convention(thin) (@guaranteed Builtin.NativeObject) -> (Klass, Klass) {
+// CHECK-LABEL: sil [ossa] @owned_to_guaranteed_rauw_2a : $@convention(thin) (@guaranteed Builtin.NativeObject) -> @owned (Klass, Klass) {
 // CHECK: bb0(
 // CHECK-NEXT: unchecked_ref_cast
 // CHECK-NEXT: tuple
+// CHECK-NEXT: copy_value
 // CHECK-NEXT: return
-// CHECK: } // end sil function 'unowned_to_guaranteed_rauw_2a'
-sil [ossa] @unowned_to_guaranteed_rauw_2a : $@convention(thin) (@guaranteed Builtin.NativeObject) -> (Klass, Klass) {
+// CHECK: } // end sil function 'owned_to_guaranteed_rauw_2a'
+sil [ossa] @owned_to_guaranteed_rauw_2a : $@convention(thin) (@guaranteed Builtin.NativeObject) -> @owned (Klass, Klass) {
 bb0(%0 : @guaranteed $Builtin.NativeObject):
   %0a = unchecked_ref_cast %0 : $Builtin.NativeObject to $Klass
   %1 = unchecked_bitwise_cast %0a : $Klass to $SubKlass
   %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass
   %3 = tuple(%2 : $Klass, %2 : $Klass)
-  return %3 : $(Klass, Klass)
+  %4 = copy_value %3
+  return %4 : $(Klass, Klass)
 }
 
 // We need the unchecked_ownership_conversion since our base value is
 // guaranteed, not a function argument, and our user is a function exiting
 // terminator.
 //
-// CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_2b : $@convention(thin) (@guaranteed Builtin.NativeObject) -> Klass {
+// CHECK-LABEL: sil [ossa] @owned_to_guaranteed_rauw_2b : $@convention(thin) (@guaranteed Builtin.NativeObject) -> @owned Klass {
 // CHECK: bb0(
 // CHECK-NEXT: unchecked_ref_cast
+// CHECK-NEXT: copy_value
 // CHECK-NEXT: return
-// CHECK: } // end sil function 'unowned_to_guaranteed_rauw_2b'
-sil [ossa] @unowned_to_guaranteed_rauw_2b : $@convention(thin) (@guaranteed Builtin.NativeObject) -> Klass {
+// CHECK: } // end sil function 'owned_to_guaranteed_rauw_2b'
+sil [ossa] @owned_to_guaranteed_rauw_2b : $@convention(thin) (@guaranteed Builtin.NativeObject) -> @owned Klass {
 bb0(%0 : @guaranteed $Builtin.NativeObject):
   %0a = unchecked_ref_cast %0 : $Builtin.NativeObject to $Klass
   %1 = unchecked_bitwise_cast %0a : $Klass to $SubKlass
   %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass
-  return %2 : $Klass
+  %3 = copy_value %2
+  return %3 : $Klass
 }
 
-
 // CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_2_loop : $@convention(thin) (@guaranteed Klass) -> @owned FakeOptional<(Klass, Klass)> {
 // CHECK: bb0([[ARG:%.*]] : @guaranteed $Klass):
 // CHECK-NOT: unchecked_bitwise_cast
@@ -404,15 +409,17 @@ bbExitBlock(%result : @owned $FakeOptional<(Klass, Klass)>):
   return %result : $FakeOptional<(Klass, Klass)>
 }
 
-// CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_3 : $@convention(thin) (@guaranteed Klass) -> Klass {
+// CHECK-LABEL: sil [ossa] @owned_to_guaranteed_rauw_3 : $@convention(thin) (@guaranteed Klass) -> @owned Klass {
 // CHECK: bb0(
+// CHECK-NEXT: copy_value
 // CHECK-NEXT: return
-// CHECK: } // end sil function 'unowned_to_guaranteed_rauw_3'
-sil [ossa] @unowned_to_guaranteed_rauw_3 : $@convention(thin) (@guaranteed Klass) -> Klass {
+// CHECK: } // end sil function 'owned_to_guaranteed_rauw_3'
+sil [ossa] @owned_to_guaranteed_rauw_3 : $@convention(thin) (@guaranteed Klass) -> @owned Klass {
 bb0(%0 : @guaranteed $Klass):
   %1 = unchecked_bitwise_cast %0 : $Klass to $SubKlass
   %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass
-  return %2 : $Klass
+  %3 = copy_value %2
+  return %3 : $Klass
 }
 
 //===---

test/SILOptimizer/sil_combine_ossa.sil

@@ -1604,18 +1604,6 @@ bb0(%0 : @guaranteed $B):
   return %3 : $F
 }
 
-// CHECK-LABEL: sil [ossa] @unchecked_ref_cast_formation_unowned : $@convention(thin) (B) -> F {
-// CHECK: bb0([[INPUT_REF:%[0-9]+]] :
-// CHECK: ref_to_raw_pointer
-// CHECK: raw_pointer_to_ref
-// CHECK: } // end sil function 'unchecked_ref_cast_formation_unowned'
-sil [ossa] @unchecked_ref_cast_formation_unowned : $@convention(thin) (B) -> F {
-bb0(%0 : @unowned $B):
-  %1 = ref_to_raw_pointer %0 : $B to $Builtin.RawPointer
-  %2 = raw_pointer_to_ref %1 : $Builtin.RawPointer to $F
-  return %2 : $F
-}
-
 // CHECK-LABEL: sil [ossa] @upcast_unchecked_ref_cast_roundtrip : $@convention(thin) (@owned B) -> @owned B {
 // CHECK-NOT: unchecked_ref_cast
 // CHECK-NOT: upcast
@@ -2745,27 +2733,6 @@ class XXImpl : XX {
   init()
 }
 
-// CHECK-LABEL: sil [ossa] @unowned_round_trips : $@convention(thin) (@guaranteed B, @guaranteed @sil_unowned B, @guaranteed AnyObject, @sil_unmanaged AnyObject) -> (B, @sil_unowned B, AnyObject, @sil_unmanaged AnyObject) {
-// CHECK: bb0(
-// CHECK-NEXT: tuple
-// CHECK-NEXT: return
-// CHECK: } // end sil function 'unowned_round_trips'
-sil [ossa] @unowned_round_trips : $@convention(thin) (@guaranteed B, @guaranteed @sil_unowned B, @guaranteed AnyObject, @sil_unmanaged AnyObject) -> (B, @sil_unowned B, AnyObject, @sil_unmanaged AnyObject) {
-bb0(%0 : @guaranteed $B, %1 : @guaranteed $@sil_unowned B, %2 : @guaranteed $AnyObject, %3 : $@sil_unmanaged AnyObject):
-  %4 = ref_to_unowned %0 : $B to $@sil_unowned B
-  %5 = unowned_to_ref %4 : $@sil_unowned B to $B
-  %6 = unowned_to_ref %1 : $@sil_unowned B to $B
-  %7 = ref_to_unowned %6 : $B to $@sil_unowned B
-
-  %8 = ref_to_unmanaged %2 : $AnyObject to $@sil_unmanaged AnyObject
-  %9 = unmanaged_to_ref %8 : $@sil_unmanaged AnyObject to $AnyObject
-  %10 = unmanaged_to_ref %3 : $@sil_unmanaged AnyObject to $AnyObject
-  %11 = ref_to_unmanaged %10 : $AnyObject to $@sil_unmanaged AnyObject
-
-  %9999 = tuple(%5 : $B, %7 : $@sil_unowned B, %9 : $AnyObject, %11 : $@sil_unmanaged AnyObject)
-  return %9999 : $(B, @sil_unowned B, AnyObject, @sil_unmanaged AnyObject)
-}
-
 // CHECK-LABEL: sil [ossa] @collapse_existential_pack_unpack_unchecked_ref_cast :
 // CHECK:       bb0([[Ref:%.*]] : @guaranteed $MyClass):
 // CHECK-NOT:     init_existential_ref

test/SILOptimizer/simplify_unconditional_check_cast.sil

@@ -37,15 +37,16 @@ bb0(%0 : $@thick T.Type):
   return %1
 }
 
-
 // CHECK-LABEL: sil [ossa] @test_non_metatype :
 // CHECK:         %1 = unconditional_checked_cast %0
-// CHECK-NEXT:    return %1
+// CHECK-NEXT:    %2 = copy_value %1
+// CHECK-NEXT:    return %2
 // CHECK:       } // end sil function 'test_non_metatype'
-sil [ossa] @test_non_metatype : $@convention(thin) (@guaranteed C) -> any PC {
+sil [ossa] @test_non_metatype : $@convention(thin) (@guaranteed C) -> @owned any PC {
 bb0(%0 : @guaranteed $C):
   %1 = unconditional_checked_cast %0 to any PC
-  return %1
+  %2 = copy_value %1
+  return %2
 }
 
 // CHECK-LABEL: sil [ossa] @test_non_existential_target :

test/SILOptimizer/simplify_value_to_bridge_object.sil

@@ -22,15 +22,16 @@ bb0(%0 : $Builtin.Int64):
 // CHECK:         %1 = value_to_bridge_object %0
 // CHECK:         %2 = value_to_bridge_object %0
 // CHECK:         %3 = tuple (%1 : $Builtin.BridgeObject, %2 : $Builtin.BridgeObject)
-// CHECK:         return %3
+// CHECK:         %4 = copy_value %3
+// CHECK:         return %4
 // CHECK:       } // end sil function 'keep_both_vtbo_instructions'
-sil [ossa] @keep_both_vtbo_instructions : $@convention(thin) (Builtin.Int64) -> (Builtin.BridgeObject, Builtin.BridgeObject) {
+sil [ossa] @keep_both_vtbo_instructions : $@convention(thin) (Builtin.Int64) -> @owned (Builtin.BridgeObject, Builtin.BridgeObject) {
 bb0(%0 : $Builtin.Int64):
   %1 = value_to_bridge_object %0 : $Builtin.Int64
   %2 = unchecked_trivial_bit_cast %1 : $Builtin.BridgeObject to $UInt64
   %3 = struct_extract %2 : $UInt64, #UInt64._value
   %4 = value_to_bridge_object %3 : $Builtin.Int64
   %5 = tuple (%1 : $Builtin.BridgeObject, %4 : $Builtin.BridgeObject)
-  return %5 : $(Builtin.BridgeObject, Builtin.BridgeObject)
+  %6 = copy_value %5
+  return %6
 }
-