Merge pull request #61651 from gottesmm/pr-727f8b30b3e32b1eb29003ac26642f5cf54b78e6

[silgen] Eliminate lifetime gap caused by AutoreleasingUnsafeMutablePointer's LValue component set.

Author: gottesmm

lib/SILGen/SILGenExpr.cpp

@@ -5552,8 +5552,14 @@ class AutoreleasingWritebackComponent : public LogicalPathComponent {
               unowned->getType().castTo<UnmanagedStorageType>().getReferentType());
     auto owned = SGF.B.createUnmanagedToRef(loc, unowned, strongType);
     auto ownedMV = SGF.emitManagedRetain(loc, owned);
-    
-    // Reassign the +1 storage with it.
+
+    // Then create a mark dependence in between the base and the ownedMV. This
+    // is important to ensure that the destroy of the assign is not hoisted
+    // above the retain. We are doing unmanaged things here so we need to be
+    // extra careful.
+    ownedMV = SGF.B.createMarkDependence(loc, ownedMV, base);
+
+    // Then reassign the mark dependence into the +1 storage.
     ownedMV.assignInto(SGF, loc, base.getUnmanagedValue());
   }
 

test/SILGen/foreign_errors.swift

@@ -28,10 +28,17 @@ func test0() throws {
   // CHECK: [[RESULT:%.*]] = apply [[METHOD]]({{.*}}, [[SELF]])
 
   //   Writeback to the first temporary.
+  //
+  //   NOTE: We need to a mark dependence here to ensure that the destroy
+  //   associated with the assign to ERR_TEMP0 is not hoisted above the copy_value
+  //   of T1_COPY. If we were to allow that, we could introduce a lifetime gap
+  //   causing potentially use after frees.
+  //
   // CHECK: [[T0:%.*]] = load [trivial] [[ERR_TEMP1]]
   // CHECK: [[T1:%.*]] = unmanaged_to_ref [[T0]]
   // CHECK: [[T1_COPY:%.*]] = copy_value [[T1]]
-  // CHECK: assign [[T1_COPY]] to [[ERR_TEMP0]]
+  // CHECK: [[T1_COPY_DEP:%.*]] = mark_dependence [[T1_COPY]] : $Optional<NSError> on [[ERR_TEMP0]]
+  // CHECK: assign [[T1_COPY_DEP]] to [[ERR_TEMP0]]
 
   //   Pull out the boolean value and compare it to zero.
   // CHECK: [[BOOL_OR_INT:%.*]] = struct_extract [[RESULT]]

test/SILGen/pointer_conversion.swift

@@ -250,7 +250,13 @@ func classInoutToPointer() {
   // CHECK: [[UNOWNED_OUT:%.*]] = load [trivial] [[WRITEBACK]]
   // CHECK: [[OWNED_OUT:%.*]] = unmanaged_to_ref [[UNOWNED_OUT]]
   // CHECK: [[OWNED_OUT_COPY:%.*]] = copy_value [[OWNED_OUT]]
-  // CHECK: assign [[OWNED_OUT_COPY]] to [[WRITE2]]
+  //
+  // DISCUSSION: We need a mark_dependence here to ensure that the destroy of
+  // the value in WRITE2 is not hoisted above the copy of OWNED_OUT. Otherwise,
+  // we may have a use-after-free if ref counts are low enough.
+  //
+  // CHECK: [[OWNED_OUT_COPY_DEP:%.*]] = mark_dependence [[OWNED_OUT_COPY]] : $C on [[WRITE2]]
+  // CHECK: assign [[OWNED_OUT_COPY_DEP]] to [[WRITE2]]
 
   var cq: C? = C()
   takesPlusZeroOptionalPointer(&cq)