Skip to content

Commit 05214f0

Browse files
committed
Revert "[SR-15703] Fix missing hop in error path of foreign async throws call"
This reverts commit afd26d3 to solve a critical miscompile caused by a hop_to_executor appearing between a get_continuation and await_continuation instruction in SIL. A reimplementation of SR-15703 will be forthcoming. Fixes rdar://91502776
1 parent 2d2586e commit 05214f0

File tree

4 files changed

+9
-82
lines changed

4 files changed

+9
-82
lines changed

lib/SILGen/ExecutorBreadcrumb.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,6 @@ 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
4540
};
4641

4742
} // namespace Lowering

lib/SILGen/SILGenApply.cpp

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4430,27 +4430,6 @@ class FixLifetimeDestroyCleanup : public Cleanup {
44304430
#endif
44314431
}
44324432
};
4433-
4434-
class EmitBreadcrumbCleanup : public Cleanup {
4435-
ExecutorBreadcrumb breadcrumb;
4436-
4437-
public:
4438-
EmitBreadcrumbCleanup(ExecutorBreadcrumb &&breadcrumb)
4439-
: breadcrumb(std::move(breadcrumb)) {}
4440-
4441-
void emit(SILGenFunction &SGF, CleanupLocation l,
4442-
ForUnwind_t forUnwind) override {
4443-
breadcrumb.emit(SGF, l);
4444-
}
4445-
4446-
void dump(SILGenFunction &SGF) const override {
4447-
#ifndef NDEBUG
4448-
llvm::errs() << "EmitBreadcrumbCleanup "
4449-
<< "State:" << getState()
4450-
<< "NeedsEmit:" << breadcrumb.needsEmit();
4451-
#endif
4452-
}
4453-
};
44544433
} // end anonymous namespace
44554434

44564435
//===----------------------------------------------------------------------===//
@@ -4697,14 +4676,12 @@ RValue SILGenFunction::emitApply(
46974676
*foreignError, calleeTypeInfo.foreign.async);
46984677
}
46994678

4700-
// For objc async calls, push cleanups to be used on
4679+
// For objc async calls, push cleanup to be used on
47014680
// both result and throw paths prior to finishing the result plan.
47024681
if (calleeTypeInfo.foreign.async) {
47034682
for (auto unmanagedCopy : unmanagedCopies) {
47044683
Cleanups.pushCleanup<FixLifetimeDestroyCleanup>(unmanagedCopy);
47054684
}
4706-
// save breadcrumb as a clean-up so it is emitted in result / throw cases.
4707-
Cleanups.pushCleanup<EmitBreadcrumbCleanup>(std::move(breadcrumb));
47084685
} else {
47094686
assert(unmanagedCopies.empty());
47104687
}
@@ -4714,6 +4691,10 @@ RValue SILGenFunction::emitApply(
47144691
directResultsArray, bridgedForeignError);
47154692
assert(directResultsArray.empty() && "didn't claim all direct results");
47164693

4694+
if (calleeTypeInfo.foreign.async) {
4695+
breadcrumb.emit(*this, loc);
4696+
}
4697+
47174698
return result;
47184699
}
47194700

test/SILGen/objc_async.swift

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -201,43 +201,3 @@ 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: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,28 +25,19 @@ 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+
2832
// CHECK: objc_method {{.*}} $@convention(objc_method) <τ_0_0 where τ_0_0 : SlowServing> (@convention(block) (Int, NSString) -> (), τ_0_0) -> ()
2933
// CHECK: hop_to_executor [[GENERIC_EXECUTOR]] :
3034
let _: (Int, String) = await p.requestIntAndString()
3135

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

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-
5041
class SlowSwiftServer: NSObject, SlowServing {
5142
// CHECK-LABEL: sil {{.*}} @$s21objc_async_from_swift15SlowSwiftServerC10requestIntSiyYaF
5243
// CHECK: [[GENERIC_EXECUTOR:%.*]] = enum $Optional<Builtin.Executor>, #Optional.none

0 commit comments

Comments
 (0)