Skip to content

Commit 3372ab7

Browse files
committed
Fix arg cleanup for objc continuations
Current SILGen for objc continuations, can end the lifetime of args before await_async_continuation. This PR fixes it by extending the lifetime of the args until await_async_continuation by creating copies. And then inserts manual cleanup with fix_lifetime + destroy Fixes rdar://78982371
1 parent befeab2 commit 3372ab7

File tree

4 files changed

+84
-2
lines changed

4 files changed

+84
-2
lines changed

lib/SILGen/ManagedValue.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,24 @@ ManagedValue ManagedValue::copy(SILGenFunction &SGF, SILLocation loc) const {
3737
return SGF.emitManagedRValueWithCleanup(buf, lowering);
3838
}
3939

40+
// Emit an unmanaged copy of this value
41+
// WARNING: Callers of this API should manage the cleanup of this value!
42+
ManagedValue ManagedValue::unmanagedCopy(SILGenFunction &SGF,
43+
SILLocation loc) const {
44+
auto &lowering = SGF.getTypeLowering(getType());
45+
if (lowering.isTrivial())
46+
return *this;
47+
48+
if (getType().isObject()) {
49+
auto copy = SGF.B.emitCopyValueOperation(loc, getValue());
50+
return ManagedValue::forUnmanaged(copy);
51+
}
52+
53+
SILValue buf = SGF.emitTemporaryAllocation(loc, getType());
54+
SGF.B.createCopyAddr(loc, getValue(), buf, IsNotTake, IsInitialization);
55+
return ManagedValue::forUnmanaged(buf);
56+
}
57+
4058
/// Emit a copy of this value with independent ownership.
4159
ManagedValue ManagedValue::formalAccessCopy(SILGenFunction &SGF,
4260
SILLocation loc) {

lib/SILGen/ManagedValue.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,10 @@ class ManagedValue {
270270
/// Emit a copy of this value with independent ownership.
271271
ManagedValue copy(SILGenFunction &SGF, SILLocation loc) const;
272272

273+
/// Returns an unmanaged copy of this value.
274+
/// WARNING: Callers of this API should manage the cleanup of this value!
275+
ManagedValue unmanagedCopy(SILGenFunction &SGF, SILLocation loc) const;
276+
273277
/// Emit a copy of this value with independent ownership into the current
274278
/// formal evaluation scope.
275279
ManagedValue formalAccessCopy(SILGenFunction &SGF, SILLocation loc);

lib/SILGen/SILGenApply.cpp

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4325,6 +4325,29 @@ bool SILGenModule::isNonMutatingSelfIndirect(SILDeclRef methodRef) {
43254325
return self.isFormalIndirect();
43264326
}
43274327

4328+
namespace {
4329+
/// Cleanup to insert fix_lifetime and destroy
4330+
class FixLifetimeDestroyCleanup : public Cleanup {
4331+
SILValue val;
4332+
4333+
public:
4334+
FixLifetimeDestroyCleanup(SILValue val) : val(val) {}
4335+
4336+
void emit(SILGenFunction &SGF, CleanupLocation l,
4337+
ForUnwind_t forUnwind) override {
4338+
SGF.B.emitFixLifetime(l, val);
4339+
SGF.B.emitDestroyOperation(l, val);
4340+
}
4341+
4342+
void dump(SILGenFunction &SGF) const override {
4343+
#ifndef NDEBUG
4344+
llvm::errs() << "FixLifetimeDestroyCleanup "
4345+
<< "State:" << getState() << " "
4346+
<< "Value: " << val << "\n";
4347+
#endif
4348+
}
4349+
};
4350+
} // end anonymous namespace
43284351

43294352
//===----------------------------------------------------------------------===//
43304353
// Top Level Entrypoints
@@ -4474,6 +4497,19 @@ RValue SILGenFunction::emitApply(ResultPlanPtr &&resultPlan,
44744497
// hop back to the current executor
44754498
breadcrumb.emit(*this, loc);
44764499

4500+
// For objc async calls, lifetime extend the args until the result plan which
4501+
// generates `await_async_continuation`.
4502+
// Lifetime is extended by creating unmanaged copies here and by pushing the
4503+
// cleanups required just before the result plan is generated.
4504+
SmallVector<ManagedValue, 8> unmanagedCopies;
4505+
if (calleeTypeInfo.foreign.async) {
4506+
for (auto arg : args) {
4507+
if (arg.hasCleanup()) {
4508+
unmanagedCopies.push_back(arg.unmanagedCopy(*this, loc));
4509+
}
4510+
}
4511+
}
4512+
44774513
// Pop the argument scope.
44784514
argScope.pop();
44794515

@@ -4549,12 +4585,30 @@ RValue SILGenFunction::emitApply(ResultPlanPtr &&resultPlan,
45494585
emitForeignErrorCheck(loc, directResults, errorTemp, doesNotThrow,
45504586
*foreignError);
45514587
}
4552-
4588+
4589+
// For objc async calls, push cleanup to be used on throw paths in the result
4590+
// planner.
4591+
for (unsigned i : indices(unmanagedCopies)) {
4592+
SILValue value = unmanagedCopies[i].getValue();
4593+
Cleanups.pushCleanup<FixLifetimeDestroyCleanup>(value);
4594+
unmanagedCopies[i] = ManagedValue(value, Cleanups.getTopCleanup());
4595+
}
4596+
45534597
auto directResultsArray = makeArrayRef(directResults);
45544598
RValue result =
45554599
resultPlan->finish(*this, loc, substResultType, directResultsArray);
45564600
assert(directResultsArray.empty() && "didn't claim all direct results");
45574601

4602+
// For objc async calls, generate cleanup on the resume path here and forward
4603+
// the previously pushed cleanups.
4604+
if (calleeTypeInfo.foreign.async) {
4605+
for (auto unmanagedCopy : unmanagedCopies) {
4606+
auto value = unmanagedCopy.forward(*this);
4607+
B.emitFixLifetime(loc, value);
4608+
B.emitDestroyOperation(loc, value);
4609+
}
4610+
}
4611+
45584612
return result;
45594613
}
45604614

test/SILGen/objc_async.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import ObjCConcurrency
88
// CHECK-LABEL: sil {{.*}}@${{.*}}14testSlowServer
99
func testSlowServer(slowServer: SlowServer) async throws {
1010
// CHECK: [[RESUME_BUF:%.*]] = alloc_stack $Int
11+
// CHECK: [[STRINGINIT:%.*]] = function_ref @$sSS10FoundationE19_bridgeToObjectiveCSo8NSStringCyF :
12+
// CHECK: [[ARG:%.*]] = apply [[STRINGINIT]]
1113
// CHECK: [[METHOD:%.*]] = objc_method {{.*}} $@convention(objc_method) (NSString, @convention(block) (Int) -> (), SlowServer) -> ()
1214
// CHECK: [[CONT:%.*]] = get_async_continuation_addr Int, [[RESUME_BUF]]
1315
// CHECK: [[WRAPPED:%.*]] = struct $UnsafeContinuation<Int, Never> ([[CONT]] : $Builtin.RawUnsafeContinuation)
@@ -16,10 +18,14 @@ func testSlowServer(slowServer: SlowServer) async throws {
1618
// CHECK: store [[WRAPPED]] to [trivial] [[CONT_SLOT]]
1719
// CHECK: [[BLOCK_IMPL:%.*]] = function_ref @[[INT_COMPLETION_BLOCK:.*]] : $@convention(c) (@inout_aliasable @block_storage UnsafeContinuation<Int, Never>, Int) -> ()
1820
// CHECK: [[BLOCK:%.*]] = init_block_storage_header [[BLOCK_STORAGE]] {{.*}}, invoke [[BLOCK_IMPL]]
19-
// CHECK: apply [[METHOD]]({{.*}}, [[BLOCK]], %0)
21+
// CHECK: apply [[METHOD]]([[ARG]], [[BLOCK]], %0)
22+
// CHECK: [[COPY:%.*]] = copy_value [[ARG]]
23+
// CHECK: destroy_value [[ARG]]
2024
// CHECK: await_async_continuation [[CONT]] {{.*}}, resume [[RESUME:bb[0-9]+]]
2125
// CHECK: [[RESUME]]:
2226
// CHECK: [[RESULT:%.*]] = load [trivial] [[RESUME_BUF]]
27+
// CHECK: fix_lifetime [[COPY]]
28+
// CHECK: destroy_value [[COPY]]
2329
// CHECK: dealloc_stack [[RESUME_BUF]]
2430
let _: Int = await slowServer.doSomethingSlow("mail")
2531

0 commit comments

Comments
 (0)