Merge pull request #80463 from eeckstein/fix-sil-combine

SILCombine: fix an ownership violation when replacing open existential apply arguments

Author: eeckstein

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

@@ -765,6 +765,14 @@ void SILCombiner::replaceWitnessMethodInst(
     eraseInstFromFunction(*WMI);
 }
 
+static bool hasUse(SILValue v, Operand *op) {
+  for (Operand *use : v->getUses()) {
+    if (use == op)
+      return true;
+  }
+  return false;
+}
+
 // This function determines concrete type of an opened existential argument
 // using ProtocolConformanceAnalysis. The concrete type of the argument can be a
 // class, struct, or an enum.
@@ -839,6 +847,13 @@ SILCombiner::buildConcreteOpenedExistentialInfoFromSoleConformingType(
   // Prepare the code by adding UncheckedCast instructions that cast opened
   // existentials to concrete types. Set the ConcreteValue of CEI.
   if (auto *OER = dyn_cast<OpenExistentialRefInst>(OAI.OpenedArchetypeValue)) {
+    // Make sure that the existential value outlives the apply. This is the case
+    // if the apply uses it as argument operand.
+    if (OER->getOwnershipKind() == OwnershipKind::Owned &&
+        !hasUse(OER, &ArgOperand)) {
+      return std::nullopt;
+    }
+
     SILBuilderWithScope b(std::next(OER->getIterator()), Builder);
     auto loc = RegularLocation::getAutoGeneratedLocation();
     SoleCEI.ConcreteValue = b.createUncheckedRefCast(loc, OER, concreteSILType);
@@ -880,6 +895,11 @@ SILCombiner::buildConcreteOpenedExistentialInfo(Operand &ArgOperand) {
     auto ConcreteValue = COEI.CEI->ConcreteValue;
     if (ConcreteValue->getFunction()->hasOwnership() &&
         ConcreteValue->getOwnershipKind() == OwnershipKind::Owned) {
+      // Make sure that the existential value outlives the apply. This is the case
+      // if the apply uses it as argument operand.
+      if (!hasUse(COEI.OAI.OpenedArchetypeValue, &ArgOperand))
+        return std::nullopt;
+
       SILBuilderWithScope b(COEI.OAI.OpenedArchetypeValue->getNextInstruction(),
                             Builder);
       auto loc = RegularLocation::getAutoGeneratedLocation();

lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp

@@ -145,7 +145,7 @@ class SingleBlockOwnedForwardingInstFolder {
     if (!hasOneNonDebugUse(next))
       return false;
 
-    assert(rest.empty() || getSingleNonDebugUser(rest.back()) == next);
+    assert(rest.empty() || getSingleNonDebugUser(next) == rest.back());
     rest.push_back(next);
     return true;
   }

test/SILOptimizer/sil_combine_ossa.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt -sil-print-types -enable-copy-propagation=requested-passes-only -enable-objc-interop -enforce-exclusivity=none -enable-sil-verify-all %s -update-borrowed-from -sil-combine | %FileCheck %s
+// RUN: %target-sil-opt -sil-print-types -enable-copy-propagation=requested-passes-only -enable-objc-interop -enforce-exclusivity=none -enable-sil-verify-all %s -update-borrowed-from -sil-combine -wmo | %FileCheck %s
 // RUN: %target-sil-opt -sil-print-types -enable-copy-propagation=requested-passes-only -enable-objc-interop -enforce-exclusivity=none -enable-sil-verify-all %s -update-borrowed-from -sil-combine -generic-specializer | %FileCheck %s  --check-prefix=CHECK_FORWARDING_OWNERSHIP_KIND
 //
 // FIXME: check copy-propagation output instead once it is the default mode
@@ -5447,3 +5447,54 @@ bb0(%0 : @owned $C):
   return %6 : $()
 }
 
+private protocol Pr: AnyObject {
+}
+
+private class Cr: Pr {
+}
+
+sil @use_pr : $@convention(thin) <τ_0_0 where τ_0_0 : Pr> (@in τ_0_0) -> ()
+
+// CHECK-LABEL: sil [ossa] @existential_consumed_too_early1 :
+// CHECK:         apply {{.*}}<@opened
+// CHECK:       } // end sil function 'existential_consumed_too_early1'
+sil [ossa] @existential_consumed_too_early1 : $@convention(thin) (@owned Pr) -> () {
+bb0(%0 : @owned $Pr):
+  %2 = open_existential_ref %0 to $@opened("5554ACAA-0F2B-11F0-86AA-0EA13E3AABB1", any Pr) Self
+  %3 = alloc_stack $@opened("5554ACAA-0F2B-11F0-86AA-0EA13E3AABB1", any Pr) Self
+  store %2 to [init] %3
+  %5 = function_ref @use_pr : $@convention(thin) <τ_0_0 where τ_0_0 : Pr> (@in τ_0_0) -> ()
+  %6 = apply %5<@opened("5554ACAA-0F2B-11F0-86AA-0EA13E3AABB1", any Pr) Self>(%3) : $@convention(thin) <τ_0_0 where τ_0_0 : Pr> (@in τ_0_0) -> ()
+  dealloc_stack %3
+  %8 = tuple ()
+  return %8
+}
+
+// CHECK-LABEL: sil [ossa] @existential_consumed_too_early2 :
+// CHECK:         apply {{.*}}<@opened
+// CHECK:       } // end sil function 'existential_consumed_too_early2'
+sil [ossa] @existential_consumed_too_early2 : $@convention(thin) (@owned Cr) -> () {
+bb0(%0 : @owned $Cr):
+  %1 = init_existential_ref %0 : $Cr : $Cr, $any Pr
+  %2 = open_existential_ref %1 to $@opened("5554ACAA-0F2B-11F0-86AA-0EA13E3AABB1", any Pr) Self
+  %3 = alloc_stack $@opened("5554ACAA-0F2B-11F0-86AA-0EA13E3AABB1", any Pr) Self
+  store %2 to [init] %3
+  %5 = function_ref @use_pr : $@convention(thin) <τ_0_0 where τ_0_0 : Pr> (@in τ_0_0) -> ()
+  %6 = apply %5<@opened("5554ACAA-0F2B-11F0-86AA-0EA13E3AABB1", any Pr) Self>(%3) : $@convention(thin) <τ_0_0 where τ_0_0 : Pr> (@in τ_0_0) -> ()
+  dealloc_stack %3
+  %8 = tuple ()
+  return %8
+}
+
+// CHECK-LABEL: sil [ossa] @init_existential_with_debug_value :
+// CHECK:         return %0
+// CHECK:       } // end sil function 'init_existential_with_debug_value'
+sil [ossa] @init_existential_with_debug_value : $@convention(thin) (@owned Cr) -> @owned Cr {
+bb0(%1 : @owned $Cr):
+  %5 = init_existential_ref %1 : $Cr : $Cr, $any Pr
+  debug_value %5, let, name "interactor", argno 2
+  %48 = open_existential_ref %5 to $@opened("F9D78C78-0FCD-11F0-9F91-0EA13E3AABB1", any Pr) Self
+  %49 = unchecked_ref_cast %48 to $Cr
+  return %49
+}
+