[SILGen] avoid hop before autoreleased foreign error is retained

kavon · kavon · commit 40a184e55d8a · 2023-09-11T18:04:54.000-07:00
For an isolated ObjC function that is not async, we emit a hops around the call. But if that function returns an autoreleased pointer, we need to ensure we're retaining that pointer before hopping back after the call. We weren't doing that in the case of an autoreleased NSError: ``` %10 = alloc_stack $@sil_unmanaged Optional<NSError> %19 = ... a bunch of steps to wrap up %10 ... %20 = enum $Optional<AutoreleasingUnsafeMutablePointer<Optional<NSError>>>, #Optional.some!enumelt, %19 : $AutoreleasingUnsafeMutablePointer<Optional<NSError>> hop_to_executor $MainActor %26 = apply X(Y, %20) : $@convention(objc_method) (NSObject, Optional<AutoreleasingUnsafeMutablePointer<Optional<NSError>>>) -> @autoreleased Optional<NSString> hop_to_executor $Optional<Builtin.Executor> // retain the autoreleased pointer written-out. %28 = load [trivial] %10 : $*@sil_unmanaged Optional<NSError> %29 = unmanaged_to_ref %28 : $@sil_unmanaged Optional<NSError> to $Optional<NSError> %30 = copy_value %29 : $Optional<NSError> assign %31 to %7 : $*Optional<NSError> ``` This patch sinks the hop emission after the call so it happens after doing that copy. rdar://114049646
diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp
@@ -5553,10 +5553,6 @@ RValue SILGenFunction::emitApply(
     // plan's finish method is called, because it must happen in the
     // successors of the `await_async_continuation` terminator.
     resultPlan->deferExecutorBreadcrumb(std::move(breadcrumb));
-
-  } else {
-    // In the ordinary case, we hop back to the current executor
-    breadcrumb.emit(*this, loc);
   }
 
   // Pop the argument scope.
@@ -5629,6 +5625,12 @@ RValue SILGenFunction::emitApply(
         });
   }
 
+  if (!calleeTypeInfo.foreign.async) {
+    // For a non-foreign-async callee, we hop back to the current executor
+    // _after_ popping the argument scope and collecting the results.
+    breadcrumb.emit(*this, loc);
+  }
+
   SILValue bridgedForeignError;
   // If there was a foreign error convention, consider it.
   // TODO: maybe this should happen after managing the result if it's
diff --git a/test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h b/test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h
@@ -327,4 +327,11 @@ UI_ACTOR
 - (void)icedMochaService:(NSObject *)mochaService generateMochaWithCompletion:(void (^)(NSObject *_Nullable ingredient1, NSObject *ingredient2, NSObject *ingredient3))completionHandler;
 @end
 
+// rdar://114049646
+MAIN_ACTOR
+@protocol HotdogCompetitor
+- (nullable NSString *)pileOfHotdogsToEatWithLimit:(NSObject *)limit
+                                                   error:(NSError * __autoreleasing *)error;
+@end
+
 #pragma clang assume_nonnull end
diff --git a/test/SILGen/async_initializer.swift b/test/SILGen/async_initializer.swift
@@ -131,7 +131,7 @@ enum Birb {
 // CHECK-NEXT:     hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
 // CHECK:          hop_to_executor {{%[0-9]+}} : $MainActor
 // CHECK-NEXT:     {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(method) (@owned String, @thick Cat.Type) -> @owned Cat
-// CHECK-NEXT:     hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
+// CHECK:          hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
 // CHECK:        } // end sil function '$s12initializers7makeCatyyYaF'
 func makeCat() async {
   _ = await Cat(name: "Socks")
@@ -142,7 +142,7 @@ func makeCat() async {
 // CHECK-NEXT:     hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
 // CHECK:          hop_to_executor {{%[0-9]+}} : $MainActor
 // CHECK-NEXT:     {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(method) (@owned String, @thin Dog.Type) -> Dog
-// CHECK-NEXT:     hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
+// CHECK:          hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
 // CHECK:        } // end sil function '$s12initializers7makeDogyyYaF'
 func makeDog() async {
   _ = await Dog(name: "Lassie")
@@ -153,7 +153,7 @@ func makeDog() async {
 // CHECK-NEXT:     hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
 // CHECK:          hop_to_executor {{%[0-9]+}} : $MainActor
 // CHECK-NEXT:     {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(method) (@owned String, @thin Birb.Type) -> Birb
-// CHECK-NEXT:     hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
+// CHECK:          hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
 // CHECK:        } // end sil function '$s12initializers8makeBirbyyYaF'
 func makeBirb() async {
   _ = await Birb(name: "Chirpy")
diff --git a/test/SILGen/hop_to_executor.swift b/test/SILGen/hop_to_executor.swift
@@ -47,7 +47,7 @@ actor MyActor {
   // CHECK:        [[BORROWED_SELF:%[0-9]+]] = begin_borrow %0 : $MyActor
   // CHECK:        hop_to_executor [[BORROWED_SELF]] : $MyActor 
   // CHECK:        = apply {{.*}} : $@convention(method) @async (Int, @guaranteed MyActor) -> ()
-  // CHECK-NEXT:   hop_to_executor [[BORROWED_SELF]] : $MyActor 
+  // CHECK:        hop_to_executor [[BORROWED_SELF]] : $MyActor
   // CHECK:      } // end sil function '$s4test7MyActorC0A22ConsumingAsyncFunctionyyYaF'
   __consuming func testConsumingAsyncFunction() async {
     await callee(p)
@@ -271,9 +271,9 @@ struct BlueActor {
 // CHECK:       hop_to_executor [[REDEXE]] : $RedActorImpl
       // ---- now invoke redFn, hop back to Blue, and clean-up ----
 // CHECK-NEXT:  {{%[0-9]+}} = apply [[CALLEE]]([[ARG]]) : $@convention(thin) (Int) -> ()
+// CHECK-NEXT:  end_borrow [[REDEXE]] : $RedActorImpl
+// CHECK-NEXT:  destroy_value [[R]] : $RedActorImpl
 // CHECK-NEXT:  hop_to_executor [[BLUEEXE]] : $BlueActorImpl
-// CHECK:       end_borrow [[REDEXE]] : $RedActorImpl
-// CHECK:       destroy_value [[R]] : $RedActorImpl
 // CHECK:       end_borrow [[BLUEEXE]] : $BlueActorImpl
 // CHECK:       destroy_value [[B]] : $BlueActorImpl
 // CHECK: } // end sil function '$s4test6blueFnyyYaF'
@@ -287,8 +287,9 @@ struct BlueActor {
 // CHECK:         [[BORROW:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $RedActorImpl
 // CHECK-NEXT:    hop_to_executor [[BORROW]] : $RedActorImpl
 // CHECK-NEXT:    {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}) : $@convention(thin) (Int) -> ()
-// CHECK-NEXT:    hop_to_executor [[GENERIC_EXEC]]
 // CHECK-NEXT:    end_borrow [[BORROW]] : $RedActorImpl
+// CHECK-NEXT:    destroy_value
+// CHECK-NEXT:    hop_to_executor [[GENERIC_EXEC]]
 // CHECK: } // end sil function '$s4test20unspecifiedAsyncFuncyyYaF'
 func unspecifiedAsyncFunc() async {
   await redFn(200)
@@ -315,7 +316,7 @@ func anotherUnspecifiedAsyncFunc(_ red : RedActorImpl) async {
 // CHECK: hop_to_executor [[RED:%[0-9]+]] : $RedActorImpl
 // CHECK-NEXT: begin_borrow
 // CHECK-NEXT: apply
-// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC:%[0-9]+]] : $Optional<Builtin.Executor>
+// CHECK:      hop_to_executor [[GENERIC_EXEC:%[0-9]+]] : $Optional<Builtin.Executor>
 func testGlobalActorFuncValue(_ fn: @RedActor () -> Void) async {
   await fn()
 }
diff --git a/test/SILGen/hop_to_executor_async_prop.swift b/test/SILGen/hop_to_executor_async_prop.swift
@@ -123,9 +123,9 @@ func accessSweaterOfSweater(cat : Cat) async -> Sweater {
 
 // CHECK:    hop_to_executor [[GLOBAL_CAT]] : $Cat
 // CHECK:    [[THE_STRING:%[0-9]+]] = apply [[GETTER]]([[CAT]]) : $@convention(method) (@guaranteed Cat) -> @owned String
-// CHECK:    hop_to_executor [[GENERIC_EXEC]]
 // CHECK:    end_borrow [[GLOBAL_CAT]] : $Cat
 // CHECK:    destroy_value [[GLOBAL_CAT_REF]] : $Cat
+// CHECK:    hop_to_executor [[GENERIC_EXEC]]
 // CHECK:    return [[THE_STRING]] : $String
 // CHECK: } // end sil function '$s4test26accessGlobalIsolatedMember3catSSAA3CatC_tYaF'
 func accessGlobalIsolatedMember(cat : Cat) async -> String {
diff --git a/test/SILGen/objc_async.swift b/test/SILGen/objc_async.swift
@@ -276,3 +276,21 @@ func testThrowingMethodFromMain(slowServer: SlowServer) async -> String {
 func checkCostcoMembership() async -> Bool {
   return await CostcoManager.shared().isCustomerEnrolled(inExecutiveProgram: Person.asCustomer())
 }
+
+
+// CHECK-LABEL: sil {{.*}} @$s10objc_async12checkHotdogsySSSgx_So8NSObjectCtYaKSo16HotdogCompetitorRzlF
+// CHECK: hop_to_executor {{.*}} : $MainActor
+// CHECK: [[AUTO_REL_STR:%.*]] = apply {{.*}}<some HotdogCompetitor>({{.*}}) : $@convention(objc_method)
+// CHECK: [[UNMANAGED_OPTIONAL:%.*]] = load [trivial] {{.*}} : $*@sil_unmanaged Optional<NSError>
+// CHECK: [[MANAGED_OPTIONAL:%.*]] = unmanaged_to_ref [[UNMANAGED_OPTIONAL]] : $@sil_unmanaged Optional<NSError> to $Optional<NSError>
+// CHECK: [[RETAINED_OPTIONAL:%.*]] = copy_value [[MANAGED_OPTIONAL]] : $Optional<NSError>
+// CHECK: [[MARKED:%.*]] = mark_dependence [[RETAINED_OPTIONAL]] : $Optional<NSError> on {{.*}} : $*Optional<NSError> // user: %32
+// CHECK: assign [[MARKED]] to {{.*}} : $*Optional<NSError>
+// CHECK: destroy_value {{.*}} : $MainActor
+// CHECK: dealloc_stack {{.*}} : $*AutoreleasingUnsafeMutablePointer<Optional<NSError>>
+// CHECK: dealloc_stack {{.*}} : $*@sil_unmanaged Optional<NSError>
+// CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
+// CHECK: switch_enum
+func checkHotdogs(_ v: some HotdogCompetitor, _ timeLimit: NSObject) async throws -> String? {
+    return try await v.pileOfHotdogsToEat(withLimit: timeLimit)
+}
diff --git a/test/SILGen/toplevel_globalactorvars.swift b/test/SILGen/toplevel_globalactorvars.swift
@@ -68,8 +68,8 @@ await printFromMyActor(value: a)
 // CHECK: [[ACTORREF:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $MyActorImpl
 // CHECK: hop_to_executor [[ACTORREF]] : $MyActorImpl
 // CHECK: {{%[0-9]+}} = apply [[PRINTFROMMYACTOR_FUNC]]([[AGLOBAL]])
-// CHECK: hop_to_executor [[MAIN_OPTIONAL]]
 // CHECK: end_borrow [[ACTORREF]]
+// CHECK: hop_to_executor [[MAIN_OPTIONAL]]
 
 if a < 10 {
 // CHECK: [[AACCESS:%[0-9]+]] = begin_access [read] [dynamic] [[AREF]] : $*Int
@@ -122,6 +122,6 @@ if a < 10 {
     // CHECK: [[ACTORREF:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $MyActorImpl
     // CHECK: hop_to_executor [[ACTORREF]] : $MyActorImpl
     // CHECK: {{%[0-9]+}} = apply [[PRINTFROMMYACTOR_FUNC]]([[AGLOBAL]])
-    // CHECK: hop_to_executor [[MAIN_OPTIONAL]]
     // CHECK: end_borrow [[ACTORREF]]
+    // CHECK: hop_to_executor [[MAIN_OPTIONAL]]
 }
