Skip to content

Commit 06ac850

Browse files
kavonkavon
committed
[SILGen] avoid hop before autoreleased foreign error is retained
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
1 parent 6cc5d67 commit 06ac850

File tree

8 files changed

+48
-16
lines changed

8 files changed

+48
-16
lines changed

‎lib/SILGen/SILGenApply.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5608,10 +5608,6 @@ RValue SILGenFunction::emitApply(
56085608
// plan's finish method is called, because it must happen in the
56095609
// successors of the `await_async_continuation` terminator.
56105610
resultPlan->deferExecutorBreadcrumb(std::move(breadcrumb));
5611-
5612-
} else {
5613-
// In the ordinary case, we hop back to the current executor
5614-
breadcrumb.emit(*this, loc);
56155611
}
56165612

56175613
// Pop the argument scope.
@@ -5684,6 +5680,14 @@ RValue SILGenFunction::emitApply(
56845680
});
56855681
}
56865682

5683+
if (!calleeTypeInfo.foreign.async) {
5684+
// For a non-foreign-async callee, we hop back to the current executor
5685+
// _after_ popping the argument scope and collecting the results. This is
5686+
// important because we may need to, for example, retain one of the results
5687+
// prior to changing actors in the case of an autorelease'd return value.
5688+
breadcrumb.emit(*this, loc);
5689+
}
5690+
56875691
SILValue bridgedForeignError;
56885692
// If there was a foreign error convention, consider it.
56895693
// TODO: maybe this should happen after managing the result if it's

‎test/Concurrency/isolated_default_argument_eval.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ func nonisolatedAsyncCaller() async {
2020
// CHECK: [[GETARG:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval19mainActorDefaultArg5valueySi_tFfA_
2121
// CHECK: hop_to_executor {{.*}} : $MainActor
2222
// CHECK-NEXT: apply [[GETARG]]()
23+
// CHECK-NEXT: end_borrow {{.*}} : $MainActor
24+
// CHECK-NEXT: destroy_value {{.*}} : $MainActor
2325
// CHECK-NEXT: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
2426
await mainActorDefaultArg()
2527
}

‎test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,4 +336,11 @@ MAIN_ACTOR
336336
@property(readonly) BOOL isVisible;
337337
@end
338338

339+
// rdar://114049646
340+
MAIN_ACTOR
341+
@protocol HotdogCompetitor
342+
- (nullable NSString *)pileOfHotdogsToEatWithLimit:(NSObject *)limit
343+
error:(NSError * __autoreleasing *)error;
344+
@end
345+
339346
#pragma clang assume_nonnull end

‎test/SILGen/async_initializer.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ enum Birb {
131131
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
132132
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
133133
// CHECK-NEXT: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(method) (@owned String, @thick Cat.Type) -> @owned Cat
134-
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
134+
// CHECK: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
135135
// CHECK: } // end sil function '$s12initializers7makeCatyyYaF'
136136
func makeCat() async {
137137
_ = await Cat(name: "Socks")
@@ -142,7 +142,7 @@ func makeCat() async {
142142
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
143143
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
144144
// CHECK-NEXT: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(method) (@owned String, @thin Dog.Type) -> Dog
145-
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
145+
// CHECK: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
146146
// CHECK: } // end sil function '$s12initializers7makeDogyyYaF'
147147
func makeDog() async {
148148
_ = await Dog(name: "Lassie")
@@ -153,7 +153,7 @@ func makeDog() async {
153153
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
154154
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
155155
// CHECK-NEXT: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(method) (@owned String, @thin Birb.Type) -> Birb
156-
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
156+
// CHECK: hop_to_executor [[GENERIC_EXEC]] : $Optional<Builtin.Executor>
157157
// CHECK: } // end sil function '$s12initializers8makeBirbyyYaF'
158158
func makeBirb() async {
159159
_ = await Birb(name: "Chirpy")

‎test/SILGen/hop_to_executor.swift

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ actor MyActor {
4747
// CHECK: [[BORROWED_SELF:%[0-9]+]] = begin_borrow %0 : $MyActor
4848
// CHECK: hop_to_executor [[BORROWED_SELF]] : $MyActor
4949
// CHECK: = apply {{.*}} : $@convention(method) @async (Int, @guaranteed MyActor) -> ()
50-
// CHECK-NEXT: hop_to_executor [[BORROWED_SELF]] : $MyActor
50+
// CHECK: hop_to_executor [[BORROWED_SELF]] : $MyActor
5151
// CHECK: } // end sil function '$s4test7MyActorC0A22ConsumingAsyncFunctionyyYaF'
5252
__consuming func testConsumingAsyncFunction() async {
5353
await callee(p)
@@ -225,8 +225,8 @@ actor BlueActorImpl {
225225
// CHECK: [[METH:%[0-9]+]] = class_method [[REDBORROW]] : $RedActorImpl, #RedActorImpl.hello : (isolated RedActorImpl) -> (Int) -> (), $@convention(method) (Int, @guaranteed RedActorImpl) -> ()
226226
// CHECK: hop_to_executor [[REDBORROW]] : $RedActorImpl
227227
// CHECK-NEXT: = apply [[METH]]([[INTARG]], [[REDBORROW]]) : $@convention(method) (Int, @guaranteed RedActorImpl) -> ()
228+
// CHECK-NEXT: end_borrow [[REDBORROW]] : $RedActorImpl
228229
// CHECK-NEXT: hop_to_executor [[BLUE]] : $BlueActorImpl
229-
// CHECK: end_borrow [[REDBORROW]] : $RedActorImpl
230230
// CHECK: destroy_value [[REDMOVE]] : $RedActorImpl
231231
// CHECK: } // end sil function '$s4test13BlueActorImplC14createAndGreetyyYaF'
232232
func createAndGreet() async {
@@ -272,9 +272,9 @@ struct BlueActor {
272272
// CHECK: hop_to_executor [[REDEXE]] : $RedActorImpl
273273
// ---- now invoke redFn, hop back to Blue, and clean-up ----
274274
// CHECK-NEXT: {{%[0-9]+}} = apply [[CALLEE]]([[ARG]]) : $@convention(thin) (Int) -> ()
275+
// CHECK-NEXT: end_borrow [[REDEXE]] : $RedActorImpl
276+
// CHECK-NEXT: destroy_value [[R]] : $RedActorImpl
275277
// CHECK-NEXT: hop_to_executor [[BLUEEXE]] : $BlueActorImpl
276-
// CHECK: end_borrow [[REDEXE]] : $RedActorImpl
277-
// CHECK: destroy_value [[R]] : $RedActorImpl
278278
// CHECK: end_borrow [[BLUEEXE]] : $BlueActorImpl
279279
// CHECK: destroy_value [[B]] : $BlueActorImpl
280280
// CHECK: } // end sil function '$s4test6blueFnyyYaF'
@@ -288,8 +288,9 @@ struct BlueActor {
288288
// CHECK: [[BORROW:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $RedActorImpl
289289
// CHECK-NEXT: hop_to_executor [[BORROW]] : $RedActorImpl
290290
// CHECK-NEXT: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}) : $@convention(thin) (Int) -> ()
291-
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]]
292291
// CHECK-NEXT: end_borrow [[BORROW]] : $RedActorImpl
292+
// CHECK-NEXT: destroy_value
293+
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC]]
293294
// CHECK: } // end sil function '$s4test20unspecifiedAsyncFuncyyYaF'
294295
func unspecifiedAsyncFunc() async {
295296
await redFn(200)
@@ -316,7 +317,7 @@ func anotherUnspecifiedAsyncFunc(_ red : RedActorImpl) async {
316317
// CHECK: hop_to_executor [[RED:%[0-9]+]] : $RedActorImpl
317318
// CHECK-NEXT: begin_borrow
318319
// CHECK-NEXT: apply
319-
// CHECK-NEXT: hop_to_executor [[GENERIC_EXEC:%[0-9]+]] : $Optional<Builtin.Executor>
320+
// CHECK: hop_to_executor [[GENERIC_EXEC:%[0-9]+]] : $Optional<Builtin.Executor>
320321
func testGlobalActorFuncValue(_ fn: @RedActor () -> Void) async {
321322
await fn()
322323
}

‎test/SILGen/hop_to_executor_async_prop.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,9 @@ func accessSweaterOfSweater(cat : Cat) async -> Sweater {
123123

124124
// CHECK: hop_to_executor [[GLOBAL_CAT]] : $Cat
125125
// CHECK: [[THE_STRING:%[0-9]+]] = apply [[GETTER]]([[CAT]]) : $@convention(method) (@guaranteed Cat) -> @owned String
126-
// CHECK: hop_to_executor [[GENERIC_EXEC]]
127126
// CHECK: end_borrow [[GLOBAL_CAT]] : $Cat
128127
// CHECK: destroy_value [[GLOBAL_CAT_REF]] : $Cat
128+
// CHECK: hop_to_executor [[GENERIC_EXEC]]
129129
// CHECK: return [[THE_STRING]] : $String
130130
// CHECK: } // end sil function '$s4test26accessGlobalIsolatedMember3catSSAA3CatC_tYaF'
131131
func accessGlobalIsolatedMember(cat : Cat) async -> String {

‎test/SILGen/objc_async.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,3 +301,21 @@ extension OptionalMemberLookups {
301301
await self.generateMaybe!()
302302
}
303303
}
304+
305+
306+
// CHECK-LABEL: sil {{.*}} @$s10objc_async12checkHotdogsySSSgx_So8NSObjectCtYaKSo16HotdogCompetitorRzlF
307+
// CHECK: hop_to_executor {{.*}} : $MainActor
308+
// CHECK: [[AUTO_REL_STR:%.*]] = apply {{.*}}<some HotdogCompetitor>({{.*}}) : $@convention(objc_method)
309+
// CHECK: [[UNMANAGED_OPTIONAL:%.*]] = load [trivial] {{.*}} : $*@sil_unmanaged Optional<NSError>
310+
// CHECK: [[MANAGED_OPTIONAL:%.*]] = unmanaged_to_ref [[UNMANAGED_OPTIONAL]] : $@sil_unmanaged Optional<NSError> to $Optional<NSError>
311+
// CHECK: [[RETAINED_OPTIONAL:%.*]] = copy_value [[MANAGED_OPTIONAL]] : $Optional<NSError>
312+
// CHECK: [[MARKED:%.*]] = mark_dependence [[RETAINED_OPTIONAL]] : $Optional<NSError> on {{.*}} : $*Optional<NSError> // user: %32
313+
// CHECK: assign [[MARKED]] to {{.*}} : $*Optional<NSError>
314+
// CHECK: destroy_value {{.*}} : $MainActor
315+
// CHECK: dealloc_stack {{.*}} : $*AutoreleasingUnsafeMutablePointer<Optional<NSError>>
316+
// CHECK: dealloc_stack {{.*}} : $*@sil_unmanaged Optional<NSError>
317+
// CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
318+
// CHECK: switch_enum
319+
func checkHotdogs(_ v: some HotdogCompetitor, _ timeLimit: NSObject) async throws -> String? {
320+
return try await v.pileOfHotdogsToEat(withLimit: timeLimit)
321+
}

‎test/SILGen/toplevel_globalactorvars.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ await printFromMyActor(value: a)
6666
// CHECK: [[ACTORREF:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $MyActorImpl
6767
// CHECK: hop_to_executor [[ACTORREF]] : $MyActorImpl
6868
// CHECK: {{%[0-9]+}} = apply [[PRINTFROMMYACTOR_FUNC]]([[AGLOBAL]])
69-
// CHECK: hop_to_executor [[MAIN_OPTIONAL]]
7069
// CHECK: end_borrow [[ACTORREF]]
70+
// CHECK: hop_to_executor [[MAIN_OPTIONAL]]
7171

7272
if a < 10 {
7373
// CHECK: [[AACCESS:%[0-9]+]] = begin_access [read] [dynamic] [[AREF]] : $*Int
@@ -120,6 +120,6 @@ if a < 10 {
120120
// CHECK: [[ACTORREF:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $MyActorImpl
121121
// CHECK: hop_to_executor [[ACTORREF]] : $MyActorImpl
122122
// CHECK: {{%[0-9]+}} = apply [[PRINTFROMMYACTOR_FUNC]]([[AGLOBAL]])
123-
// CHECK: hop_to_executor [[MAIN_OPTIONAL]]
124123
// CHECK: end_borrow [[ACTORREF]]
124+
// CHECK: hop_to_executor [[MAIN_OPTIONAL]]
125125
}

0 commit comments

Comments
 (0)