Skip to content

Commit afd26d3

Browse files
committed
[SR-15703] Fix missing hop in error path of foreign async throws call
resolves rdar://89479707
1 parent 01d470c commit afd26d3

File tree

4 files changed

+82
-9
lines changed

4 files changed

+82
-9
lines changed

lib/SILGen/ExecutorBreadcrumb.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ class ExecutorBreadcrumb {
3737
// Emits the hop back sequence, if any, necessary to get back to
3838
// the executor represented by this breadcrumb.
3939
void emit(SILGenFunction &SGF, SILLocation loc);
40+
41+
#ifndef NDEBUG
42+
// FOR ASSERTS ONLY: returns true if calling `emit` will emit a hop-back.
43+
bool needsEmit() const { return mustReturnToExecutor; }
44+
#endif
4045
};
4146

4247
} // namespace Lowering

lib/SILGen/SILGenApply.cpp

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4414,6 +4414,27 @@ class FixLifetimeDestroyCleanup : public Cleanup {
44144414
#endif
44154415
}
44164416
};
4417+
4418+
class EmitBreadcrumbCleanup : public Cleanup {
4419+
ExecutorBreadcrumb breadcrumb;
4420+
4421+
public:
4422+
EmitBreadcrumbCleanup(ExecutorBreadcrumb &&breadcrumb)
4423+
: breadcrumb(std::move(breadcrumb)) {}
4424+
4425+
void emit(SILGenFunction &SGF, CleanupLocation l,
4426+
ForUnwind_t forUnwind) override {
4427+
breadcrumb.emit(SGF, l);
4428+
}
4429+
4430+
void dump(SILGenFunction &SGF) const override {
4431+
#ifndef NDEBUG
4432+
llvm::errs() << "EmitBreadcrumbCleanup "
4433+
<< "State:" << getState()
4434+
<< "NeedsEmit:" << breadcrumb.needsEmit();
4435+
#endif
4436+
}
4437+
};
44174438
} // end anonymous namespace
44184439

44194440
//===----------------------------------------------------------------------===//
@@ -4660,12 +4681,14 @@ RValue SILGenFunction::emitApply(
46604681
*foreignError, calleeTypeInfo.foreign.async);
46614682
}
46624683

4663-
// For objc async calls, push cleanup to be used on
4684+
// For objc async calls, push cleanups to be used on
46644685
// both result and throw paths prior to finishing the result plan.
46654686
if (calleeTypeInfo.foreign.async) {
46664687
for (auto unmanagedCopy : unmanagedCopies) {
46674688
Cleanups.pushCleanup<FixLifetimeDestroyCleanup>(unmanagedCopy);
46684689
}
4690+
// save breadcrumb as a clean-up so it is emitted in result / throw cases.
4691+
Cleanups.pushCleanup<EmitBreadcrumbCleanup>(std::move(breadcrumb));
46694692
} else {
46704693
assert(unmanagedCopies.empty());
46714694
}
@@ -4675,10 +4698,6 @@ RValue SILGenFunction::emitApply(
46754698
directResultsArray, bridgedForeignError);
46764699
assert(directResultsArray.empty() && "didn't claim all direct results");
46774700

4678-
if (calleeTypeInfo.foreign.async) {
4679-
breadcrumb.emit(*this, loc);
4680-
}
4681-
46824701
return result;
46834702
}
46844703

test/SILGen/objc_async.swift

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,3 +201,43 @@ func testSlowServerFromMain(slowServer: SlowServer) async throws {
201201
// CHECK: dealloc_stack [[RESUME_BUF]]
202202
let _: Int = await slowServer.doSomethingSlow("mail")
203203
}
204+
205+
// CHECK-LABEL: sil {{.*}}@${{.*}}26testThrowingMethodFromMain
206+
@MainActor
207+
func testThrowingMethodFromMain(slowServer: SlowServer) async -> String {
208+
// CHECK: [[RESULT_BUF:%.*]] = alloc_stack $String
209+
// CHECK: [[STRING_ARG:%.*]] = apply {{%.*}}({{%.*}}) : $@convention(method) (@guaranteed String) -> @owned NSString
210+
// CHECK: [[METH:%.*]] = objc_method {{%.*}} : $SlowServer, #SlowServer.doSomethingDangerous!foreign
211+
// CHECK: [[RAW_CONT:%.*]] = get_async_continuation_addr [throws] String, [[RESULT_BUF]] : $*String
212+
// CHECK: [[CONT:%.*]] = struct $UnsafeContinuation<String, Error> ([[RAW_CONT]] : $Builtin.RawUnsafeContinuation)
213+
// CHECK: [[STORE_ALLOC:%.*]] = alloc_stack $@block_storage UnsafeContinuation<String, Error>
214+
// CHECK: [[PROJECTED:%.*]] = project_block_storage [[STORE_ALLOC]] : $*@block_storage
215+
// CHECK: store [[CONT]] to [trivial] [[PROJECTED]] : $*UnsafeContinuation<String, Error>
216+
// CHECK: [[INVOKER:%.*]] = function_ref @$sSo8NSStringCSgSo7NSErrorCSgIeyByy_SSTz_
217+
// CHECK: [[BLOCK:%.*]] = init_block_storage_header [[STORE_ALLOC]] {{.*}}, invoke [[INVOKER]]
218+
// CHECK: [[OPTIONAL_BLK:%.*]] = enum {{.*}}, #Optional.some!enumelt, [[BLOCK]]
219+
// CHECK: %28 = apply [[METH]]([[STRING_ARG]], [[OPTIONAL_BLK]], {{%.*}}) : $@convention(objc_method) (NSString, Optional<@convention(block) (Optional<NSString>, Optional<NSError>) -> ()>, SlowServer) -> ()
220+
// CHECK: [[STRING_ARG_COPY:%.*]] = copy_value [[STRING_ARG]] : $NSString
221+
// CHECK: dealloc_stack [[STORE_ALLOC]] : $*@block_storage UnsafeContinuation<String, Error>
222+
// CHECK: destroy_value [[STRING_ARG]] : $NSString
223+
// CHECK: await_async_continuation [[RAW_CONT]] : $Builtin.RawUnsafeContinuation, resume [[RESUME:bb[0-9]+]], error [[ERROR:bb[0-9]+]]
224+
225+
// CHECK: [[RESUME]]
226+
// CHECK: {{.*}} = load [take] [[RESULT_BUF]] : $*String
227+
// CHECK: hop_to_executor {{%.*}} : $MainActor
228+
// CHECK: fix_lifetime [[STRING_ARG_COPY]] : $NSString
229+
// CHECK: destroy_value [[STRING_ARG_COPY]] : $NSString
230+
// CHECK: dealloc_stack [[RESULT_BUF]] : $*String
231+
232+
// CHECK: [[ERROR]]
233+
// CHECK: hop_to_executor {{%.*}} : $MainActor
234+
// CHECK: fix_lifetime [[STRING_ARG_COPY]] : $NSString
235+
// CHECK: destroy_value [[STRING_ARG_COPY]] : $NSString
236+
// CHECK: dealloc_stack [[RESULT_BUF]] : $*String
237+
238+
do {
239+
return try await slowServer.doSomethingDangerous("run-with-scissors")
240+
} catch {
241+
return "none"
242+
}
243+
}

test/SILGen/objc_async_from_swift.swift

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,28 @@ func testSlowServing(p: SlowServing) async throws {
2525
// CHECK: hop_to_executor [[GENERIC_EXECUTOR]] :
2626
let _: String = await p.requestString()
2727

28-
// CHECK: objc_method {{.*}} $@convention(objc_method) <τ_0_0 where τ_0_0 : SlowServing> (@convention(block) (Optional<NSString>, Optional<NSError>) -> (), τ_0_0) -> ()
29-
// CHECK: hop_to_executor [[GENERIC_EXECUTOR]] :
30-
let _: String = try await p.tryRequestString()
31-
3228
// CHECK: objc_method {{.*}} $@convention(objc_method) <τ_0_0 where τ_0_0 : SlowServing> (@convention(block) (Int, NSString) -> (), τ_0_0) -> ()
3329
// CHECK: hop_to_executor [[GENERIC_EXECUTOR]] :
3430
let _: (Int, String) = await p.requestIntAndString()
3531

3632
// CHECK: objc_method {{.*}} $@convention(objc_method) <τ_0_0 where τ_0_0 : SlowServing> (@convention(block) (Int, Optional<NSString>, Optional<NSError>) -> (), τ_0_0) -> ()
3733
// CHECK: hop_to_executor [[GENERIC_EXECUTOR]] :
34+
// CHECK: builtin "willThrow"
35+
// CHECK-NEXT: hop_to_executor [[GENERIC_EXECUTOR]] :
3836
let _: (Int, String) = try await p.tryRequestIntAndString()
3937
}
4038

39+
// CHECK-LABEL: sil {{.*}}@{{.*}}20testSlowServingAgain
40+
func testSlowServingAgain(p: SlowServing) async throws {
41+
// CHECK: [[GENERIC_EXECUTOR:%.*]] = enum $Optional<Builtin.Executor>, #Optional.none
42+
// CHECK: hop_to_executor [[GENERIC_EXECUTOR]] :
43+
// CHECK: objc_method {{.*}} $@convention(objc_method) <τ_0_0 where τ_0_0 : SlowServing> (@convention(block) (Optional<NSString>, Optional<NSError>) -> (), τ_0_0) -> ()
44+
// CHECK: hop_to_executor [[GENERIC_EXECUTOR]] :
45+
// CHECK: builtin "willThrow"
46+
// CHECK-NEXT: hop_to_executor [[GENERIC_EXECUTOR]] :
47+
let _: String = try await p.tryRequestString()
48+
}
49+
4150
class SlowSwiftServer: NSObject, SlowServing {
4251
// CHECK-LABEL: sil {{.*}} @$s21objc_async_from_swift15SlowSwiftServerC10requestIntSiyYaF
4352
// CHECK: [[GENERIC_EXECUTOR:%.*]] = enum $Optional<Builtin.Executor>, #Optional.none

0 commit comments

Comments
 (0)