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

Unowned result conventions do not work well with OSSA. Retain the result
right after the call when we come out of OSSA so we can treat the
returned value as if it was owned when we do optimizations.

This fix a miscompilation due to the DestroyAddrHoisting pass hoisting
destroys above copies with unowned sources. When the destroyed object
was the last reference to the pointed memory the copy is happening too
late resulting in a use after free.

rdar://160462854

Author: Gabor Horvath

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

@@ -6188,13 +6188,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/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,16 @@ bool OwnershipModelEliminatorVisitor::visitApplyInst(ApplyInst *ai) {
     changed = true;
   }
 
+  // Insert a retain for unowned results.
+  SILBuilderWithScope builder(ai->getNextInstruction(), builderCtx);
+  builder.emitDestructureValueOperation(
+      ai->getLoc(), ai, [&](unsigned idx, SILValue v) {
+        auto resultsIt = fnConv.getDirectSILResults().begin();
+        if (resultsIt->getConvention() == ResultConvention::Unowned)
+          builder.emitCopyValueOperation(ai->getLoc(), v);
+        ++resultsIt;
+      });
+
   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