From ceb32c014a619d90cba5fff4e58b5176b92499af Mon Sep 17 00:00:00 2001
From: Erik Eckstein <eeckstein@apple.com>
Date: Tue, 20 Jun 2023 20:47:19 +0200
Subject: [PATCH] SIL: hop_to_executor can release

The `hop_to_executor` instruction is a synchronization point and any kind of other code might run at this point,
which potentially can release objects.

Fixes a miscompile

rdar://110924258
---
 include/swift/SIL/SILNodes.def                |  2 +-
 lib/SIL/IR/SILInstruction.cpp                 |  1 +
 .../retain_release_code_motion.sil            | 28 +++++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/swift/SIL/SILNodes.def b/include/swift/SIL/SILNodes.def
index 6b7a76ad47dd0..112f319f93e0c 100644
--- a/include/swift/SIL/SILNodes.def
+++ b/include/swift/SIL/SILNodes.def
@@ -796,7 +796,7 @@ NON_VALUE_INST(FixLifetimeInst, fix_lifetime,
                SILInstruction, MayHaveSideEffects, DoesNotRelease)
 
 NON_VALUE_INST(HopToExecutorInst, hop_to_executor,
-               SILInstruction, MayHaveSideEffects, DoesNotRelease)
+               SILInstruction, MayHaveSideEffects, MayRelease)
 
 NON_VALUE_INST(DestroyValueInst, destroy_value,
                SILInstruction, MayHaveSideEffects, MayRelease)
diff --git a/lib/SIL/IR/SILInstruction.cpp b/lib/SIL/IR/SILInstruction.cpp
index 2c21e4876497a..68eb933ca0210 100644
--- a/lib/SIL/IR/SILInstruction.cpp
+++ b/lib/SIL/IR/SILInstruction.cpp
@@ -1119,6 +1119,7 @@ bool SILInstruction::mayRelease() const {
     return true;
 
   case SILInstructionKind::DestroyValueInst:
+  case SILInstructionKind::HopToExecutorInst:
     return true;
 
   case SILInstructionKind::UnconditionalCheckedCastAddrInst:
diff --git a/test/SILOptimizer/retain_release_code_motion.sil b/test/SILOptimizer/retain_release_code_motion.sil
index 916f271dfafcf..8404d75d01777 100644
--- a/test/SILOptimizer/retain_release_code_motion.sil
+++ b/test/SILOptimizer/retain_release_code_motion.sil
@@ -50,6 +50,13 @@ class C2  {
  init()
 }
 
+class Y {
+  @_hasStorage public var c: C2 { get }
+}
+
+actor Act {
+}
+
 struct S {
   var ptr : Builtin.NativeObject
 }
@@ -1078,3 +1085,24 @@ bb0:
   %12 = tuple ()
   return %12 : $()
 }
+
+// CHECK-LABEL: sil @test_hop_to_executor :
+// CHECK:         strong_retain
+// CHECK:         hop_to_executor
+// CHECK:         strong_release
+// CHECK:       } // end sil function 'test_hop_to_executor'
+sil @test_hop_to_executor : $@convention(thin) @async (@guaranteed Y, @guaranteed Act) -> A {
+bb0(%0 : $Y, %1 : $Act):
+  %2 = ref_element_addr %0 : $Y, #Y.c
+  %3 = load %2 : $*C2
+  strong_retain %3 : $C2
+
+  // This is a synchronization point and any kind of other code might run here,
+  // which potentially can release C2.
+  hop_to_executor %1 : $Act
+
+  %6 = ref_element_addr %3 : $C2, #C2.current
+  %7 = load %6 : $*A
+  strong_release %3 : $C2
+  return %7 : $A
+}