[silgen] When initializing tuples in SILGen, first evaluate all tuple elts and then perform sub-initialization.

Previsouly we were evaluating a tuple elt and then performing the relevant
sub-initialization. The problem is that a sub-initialization can invoke code
that could perform an early exit cleanup. So any later tuple-elements that may
need to be cleaned up along such path will not have had their cleanups
initialized, resulting in a leak along such paths.

With this commit, we instead evaluate all of the tuple elements and only them
perform the sub-initialization ensuring that any early exits clean up all of the
tuple elements.

rdar://83770295

Author: gottesmm

lib/SILGen/SILGenBuilder.cpp

@@ -869,6 +869,16 @@ void SILGenBuilder::emitDestructureValueOperation(
   }
 }
 
+void SILGenBuilder::emitDestructureValueOperation(
+    SILLocation loc, ManagedValue value,
+    SmallVectorImpl<ManagedValue> &destructuredValues) {
+  CleanupCloner cloner(*this, value);
+  emitDestructureValueOperation(
+      loc, value.forward(SGF), [&](unsigned index, SILValue subValue) {
+        destructuredValues.push_back(cloner.clone(subValue));
+      });
+}
+
 ManagedValue SILGenBuilder::createProjectBox(SILLocation loc, ManagedValue mv,
                                              unsigned index) {
   auto *pbi = createProjectBox(loc, mv.getValue(), index);

lib/SILGen/SILGenBuilder.h

@@ -389,6 +389,9 @@ class SILGenBuilder : public SILBuilder {
   void emitDestructureValueOperation(
       SILLocation loc, ManagedValue value,
       function_ref<void(unsigned, ManagedValue)> func);
+  void emitDestructureValueOperation(
+      SILLocation loc, ManagedValue value,
+      SmallVectorImpl<ManagedValue> &destructuredValues);
 
   using SILBuilder::createProjectBox;
   ManagedValue createProjectBox(SILLocation loc, ManagedValue mv,

lib/SILGen/SILGenDecl.cpp

@@ -39,50 +39,42 @@ using namespace Lowering;
 void Initialization::_anchor() {}
 void SILDebuggerClient::anchor() {}
 
-static void copyOrInitValueIntoHelper(
-    SILGenFunction &SGF, SILLocation loc, ManagedValue value, bool isInit,
-    ArrayRef<InitializationPtr> subInitializations,
-    llvm::function_ref<ManagedValue(ManagedValue, unsigned, SILType)> func) {
-  auto sourceType = value.getType().castTo<TupleType>();
-  auto sourceSILType = value.getType();
-  for (unsigned i = 0, e = sourceType->getNumElements(); i != e; ++i) {
-    SILType fieldTy = sourceSILType.getTupleElementType(i);
-    ManagedValue elt = func(value, i, fieldTy);
-    subInitializations[i]->copyOrInitValueInto(SGF, loc, elt, isInit);
-    subInitializations[i]->finishInitialization(SGF);
-  }
-}
-
 void TupleInitialization::copyOrInitValueInto(SILGenFunction &SGF,
                                               SILLocation loc,
                                               ManagedValue value, bool isInit) {
+  // Process all values before initialization all at once to ensure all cleanups
+  // are setup on all tuple elements before a potential early exit.
+  SmallVector<ManagedValue, 8> destructuredValues;
+
   // In the object case, emit a destructure operation and return.
   if (value.getType().isObject()) {
-    return SGF.B.emitDestructureValueOperation(
-        loc, value, [&](unsigned i, ManagedValue subValue) {
-          auto &subInit = SubInitializations[i];
-          subInit->copyOrInitValueInto(SGF, loc, subValue, isInit);
-          subInit->finishInitialization(SGF);
-        });
-  }
-
-  // In the address case, we forward the underlying value and store it
-  // into memory and then create a +1 cleanup. since we assume here
-  // that we have a +1 value since we are forwarding into memory.
-  assert(value.isPlusOne(SGF) && "Can not store a +0 value into memory?!");
-  value = ManagedValue::forUnmanaged(value.forward(SGF));
-  return copyOrInitValueIntoHelper(
-      SGF, loc, value, isInit, SubInitializations,
-      [&](ManagedValue aggregate, unsigned i,
-          SILType fieldType) -> ManagedValue {
-        ManagedValue elt =
-            SGF.B.createTupleElementAddr(loc, value, i, fieldType);
-        if (!fieldType.isAddressOnly(SGF.F)) {
-          return SGF.B.createLoadTake(loc, elt);
-        }
+    SGF.B.emitDestructureValueOperation(loc, value, destructuredValues);
+  } else {
+    // In the address case, we forward the underlying value and store it
+    // into memory and then create a +1 cleanup. since we assume here
+    // that we have a +1 value since we are forwarding into memory.
+    assert(value.isPlusOne(SGF) && "Can not store a +0 value into memory?!");
+    CleanupCloner cloner(SGF, value);
+    SILValue v = value.forward(SGF);
+
+    auto sourceType = value.getType().castTo<TupleType>();
+    auto sourceSILType = value.getType();
+    for (unsigned i : range(sourceType->getNumElements())) {
+      SILType fieldTy = sourceSILType.getTupleElementType(i);
+      SILValue elt = SGF.B.createTupleElementAddr(loc, v, i, fieldTy);
+      if (!fieldTy.isAddressOnly(SGF.F)) {
+        elt = SGF.B.emitLoadValueOperation(loc, elt,
+                                           LoadOwnershipQualifier::Take);
+      }
+      destructuredValues.push_back(cloner.clone(elt));
+    }
+  }
 
-        return SGF.emitManagedRValueWithCleanup(elt.getValue());
-      });
+  for (unsigned i : indices(destructuredValues)) {
+    SubInitializations[i]->copyOrInitValueInto(SGF, loc, destructuredValues[i],
+                                               isInit);
+    SubInitializations[i]->finishInitialization(SGF);
+  }
 }
 
 void TupleInitialization::finishUninitialized(SILGenFunction &SGF) {

test/SILGen/enum.swift

@@ -219,3 +219,23 @@ func sr7799_1(bar: SR7799??) {
   default: print("default")
   }
 }
+
+// Make sure that we handle enum, tuple initialization composed
+// correctly. Previously, we leaked down a failure path due to us misusing
+// scopes.
+enum rdar81817725 {
+    case localAddress
+    case setOption(Int, Any)
+
+    static func takeAny(_:Any) -> Bool { return true }
+
+    static func testSwitchCleanup(syscall: rdar81817725, expectedLevel: Int,
+                                  valueMatcher: (Any) -> Bool)
+      throws -> Bool {
+        if case .setOption(expectedLevel, let value) = syscall {
+            return rdar81817725.takeAny(value)
+        } else {
+            return false
+        }
+    }
+}

test/SILGen/indirect_enum.swift

@@ -448,8 +448,8 @@ func guardTreeB<T>(_ tree: TreeB<T>) {
     // CHECK:   [[TUPLE_ADDR:%.*]] = project_box [[BOX]]
     // CHECK:   copy_addr [[TUPLE_ADDR]] to [initialization] [[TUPLE_COPY:%.*]] :
     // CHECK:   [[L_COPY:%.*]] = tuple_element_addr [[TUPLE_COPY]]
-    // CHECK:   copy_addr [take] [[L_COPY]] to [initialization] [[L]]
     // CHECK:   [[R_COPY:%.*]] = tuple_element_addr [[TUPLE_COPY]]
+    // CHECK:   copy_addr [take] [[L_COPY]] to [initialization] [[L]]
     // CHECK:   copy_addr [take] [[R_COPY]] to [initialization] [[R]]
     // CHECK:   destroy_value [[BOX]]
     guard case .Branch(left: let l, right: let r) = tree else { return }
@@ -492,8 +492,8 @@ func guardTreeB<T>(_ tree: TreeB<T>) {
     // CHECK:   [[TUPLE_ADDR:%.*]] = project_box [[BOX]]
     // CHECK:   copy_addr [[TUPLE_ADDR]] to [initialization] [[TUPLE_COPY:%.*]] :
     // CHECK:   [[L_COPY:%.*]] = tuple_element_addr [[TUPLE_COPY]]
-    // CHECK:   copy_addr [take] [[L_COPY]] to [initialization] [[L]]
     // CHECK:   [[R_COPY:%.*]] = tuple_element_addr [[TUPLE_COPY]]
+    // CHECK:   copy_addr [take] [[L_COPY]] to [initialization] [[L]]
     // CHECK:   copy_addr [take] [[R_COPY]] to [initialization] [[R]]
     // CHECK:   destroy_value [[BOX]]
     // CHECK:   destroy_addr [[R]]