Skip to content

Commit 872afda

Browse files
jckarterjckarter
authored
Merge pull request swiftlang#36298 from jckarter/created-task-closure-context-leak
SIL: Clean up ownership handling in `createAsyncTask` builtins.
2 parents beadd4a + d9798c0 commit 872afda

File tree

17 files changed

+262
-272
lines changed

17 files changed

+262
-272
lines changed

‎include/swift/ABI/MetadataValues.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,6 +1183,8 @@ namespace SpecialPointerAuthDiscriminators {
11831183
const uint16_t AsyncContextYield = 0xe207; // = 57863
11841184
const uint16_t CancellationNotificationFunction = 0x1933; // = 6451
11851185
const uint16_t EscalationNotificationFunction = 0x5be4; // = 23524
1186+
const uint16_t AsyncThinNullaryFunction = 0x0f08; // = 3848
1187+
const uint16_t AsyncFutureFunction = 0x720f; // = 29199
11861188

11871189
/// Swift async context parameter stored in the extended frame info.
11881190
const uint16_t SwiftAsyncContextExtendedFrameEntry = 0xc31a; // = 49946

‎include/swift/ABI/Task.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,15 @@ class FutureAsyncContext : public AsyncContext {
553553
using AsyncContext::AsyncContext;
554554
};
555555

556+
/// An asynchronous context within a task that describes a general "Future"
557+
/// task that was started with a closure context.
558+
class FutureClosureAsyncContext : public FutureAsyncContext {
559+
public:
560+
HeapObject *closureContext;
561+
562+
using FutureAsyncContext::FutureAsyncContext;
563+
};
564+
556565
} // end namespace swift
557566

558567
#endif

‎include/swift/AST/Builtins.def‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -747,14 +747,6 @@ BUILTIN_MISC_OPERATION_WITH_SILGEN(GetCurrentAsyncTask, "getCurrentAsyncTask", "
747747
/// Cancel the given asynchronous task.
748748
BUILTIN_MISC_OPERATION_WITH_SILGEN(CancelAsyncTask, "cancelAsyncTask", "", Special)
749749

750-
/// createAsyncTask(): (
751-
/// Int, Builtin.NativeObject?, @escaping () async throws -> Void
752-
/// ) -> Builtin.NativeObject
753-
///
754-
/// Create a new asynchronous task, given flags, an (optional) parent task, and
755-
/// a function to execute.
756-
BUILTIN_MISC_OPERATION_WITH_SILGEN(CreateAsyncTask, "createAsyncTask", "", Special)
757-
758750
/// createAsyncTaskFuture(): (
759751
/// Int, Builtin.NativeObject?, @escaping () async throws -> T
760752
/// ) -> Builtin.NativeObject

‎include/swift/AST/Builtins.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ llvm::AtomicOrdering decodeLLVMAtomicOrdering(StringRef O);
137137
bool canBuiltinBeOverloadedForType(BuiltinValueKind ID, Type Ty);
138138

139139
/// Retrieve the AST-level AsyncTaskAndContext type, used for the
140-
/// createAsyncTask builtin.
140+
/// createAsyncTask* builtins.
141141
Type getAsyncTaskAndContextType(ASTContext &ctx);
142142

143143
}

‎include/swift/Runtime/Concurrency.h‎

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -30,21 +30,6 @@ struct AsyncTaskAndContext {
3030
AsyncContext *InitialContext;
3131
};
3232

33-
/// Create a task object with no future which will run the given
34-
/// function.
35-
///
36-
/// The task is not yet scheduled.
37-
///
38-
/// If a parent task is provided, flags.task_hasChildFragment() must
39-
/// be true, and this must be called synchronously with the parent.
40-
/// The parent is responsible for creating a ChildTaskStatusRecord.
41-
/// TODO: should we have a single runtime function for creating a task
42-
/// and doing this child task status record management?
43-
SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
44-
AsyncTaskAndContext swift_task_create(JobFlags flags,
45-
AsyncTask *parent,
46-
const ThinNullaryAsyncSignature::FunctionPointer *function);
47-
4833
/// Create a task object with no future which will run the given
4934
/// function.
5035
SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
@@ -59,22 +44,12 @@ using FutureAsyncSignature =
5944
AsyncSignature<void(void*), /*throws*/ true>;
6045

6146
/// Create a task object with a future which will run the given
62-
/// function.
63-
///
64-
/// The task is not yet scheduled.
65-
///
66-
/// If a parent task is provided, flags.task_hasChildFragment() must
67-
/// be true, and this must be called synchronously with the parent.
68-
/// The parent is responsible for creating a ChildTaskStatusRecord.
69-
/// TODO: should we have a single runtime function for creating a task
70-
/// and doing this child task status record management?
71-
///
72-
/// flags.task_isFuture must be set. \c futureResultType is the type
73-
///
47+
/// closure.
7448
SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
7549
AsyncTaskAndContext swift_task_create_future(
7650
JobFlags flags, AsyncTask *parent, const Metadata *futureResultType,
77-
const FutureAsyncSignature::FunctionPointer *function);
51+
void *closureEntryPoint,
52+
HeapObject * /* +1 */ closureContext);
7853

7954
/// Create a task object with a future which will run the given
8055
/// function.
@@ -84,6 +59,16 @@ AsyncTaskAndContext swift_task_create_future_f(
8459
FutureAsyncSignature::FunctionType *function,
8560
size_t initialContextSize);
8661

62+
/// Create a task object with a future which will run the given
63+
/// closure, and offer its result to the task group
64+
SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
65+
AsyncTaskAndContext swift_task_create_group_future(
66+
JobFlags flags,
67+
AsyncTask *parent, TaskGroup *group,
68+
const Metadata *futureResultType,
69+
void *closureEntryPoint,
70+
HeapObject * /* +1 */ closureContext);
71+
8772
/// Create a task object with a future which will run the given
8873
/// function, and offer its result to the task group
8974
SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)

‎include/swift/Runtime/RuntimeFunctions.def‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1502,7 +1502,7 @@ FUNCTION(TaskCancel,
15021502
ARGS(SwiftTaskPtrTy),
15031503
ATTRS(NoUnwind, ArgMemOnly))
15041504

1505-
// AsyncTaskAndContext swift_task_create(
1505+
// AsyncTaskAndContext swift_task_create_f(
15061506
// size_t flags, AsyncTask *task,
15071507
// TaskContinuationFunction* function,
15081508
// size_t contextSize);
@@ -1513,6 +1513,18 @@ FUNCTION(TaskCreateFunc,
15131513
ARGS(SizeTy, SwiftTaskPtrTy, TaskContinuationFunctionPtrTy, SizeTy),
15141514
ATTRS(NoUnwind, ArgMemOnly))
15151515

1516+
// AsyncTaskAndContext swift_task_create_future(
1517+
// size_t flags, AsyncTask *task,
1518+
// const Metadata *futureResultType,
1519+
// void *closureEntry, HeapObject *closureContext);
1520+
FUNCTION(TaskCreateFuture,
1521+
swift_task_create_future, SwiftCC,
1522+
ConcurrencyAvailability,
1523+
RETURNS(AsyncTaskAndContextTy),
1524+
ARGS(SizeTy, SwiftTaskPtrTy, TypeMetadataPtrTy,
1525+
Int8PtrTy, RefCountedPtrTy),
1526+
ATTRS(NoUnwind, ArgMemOnly))
1527+
15161528
// AsyncTaskAndContext swift_task_create_future_f(
15171529
// size_t flags, AsyncTask *task,
15181530
// const Metadata *futureResultType,
@@ -1525,6 +1537,19 @@ FUNCTION(TaskCreateFutureFunc,
15251537
TaskContinuationFunctionPtrTy, SizeTy),
15261538
ATTRS(NoUnwind, ArgMemOnly))
15271539

1540+
// AsyncTaskAndContext swift_task_create_group_future(
1541+
// size_t flags, AsyncTask *task, TaskGroup *group,
1542+
// const Metadata *futureResultType,
1543+
// void *closureEntry, HeapObject *closureContext);
1544+
FUNCTION(TaskCreateGroupFuture,
1545+
swift_task_create_group_future, SwiftCC,
1546+
ConcurrencyAvailability,
1547+
RETURNS(AsyncTaskAndContextTy),
1548+
ARGS(SizeTy, SwiftTaskPtrTy, SwiftTaskGroupPtrTy,
1549+
TypeMetadataPtrTy,
1550+
Int8PtrTy, RefCountedPtrTy),
1551+
ATTRS(NoUnwind, ArgMemOnly))
1552+
15281553
// AsyncTaskAndContext swift_task_create_group_future_f(
15291554
// size_t flags, AsyncTask *task, TaskGroup *group,
15301555
// const Metadata *futureResultType,

‎lib/AST/Builtins.cpp‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1356,16 +1356,6 @@ Type swift::getAsyncTaskAndContextType(ASTContext &ctx) {
13561356
return TupleType::get(resultTupleElements, ctx);
13571357
}
13581358

1359-
static ValueDecl *getCreateAsyncTask(ASTContext &ctx, Identifier id) {
1360-
auto extInfo = ASTExtInfoBuilder().withAsync().withThrows().build();
1361-
return getBuiltinFunction(
1362-
id,
1363-
{ ctx.getIntDecl()->getDeclaredInterfaceType(),
1364-
OptionalType::get(ctx.TheNativeObjectType),
1365-
FunctionType::get({ }, ctx.TheEmptyTupleType, extInfo) },
1366-
getAsyncTaskAndContextType(ctx));
1367-
}
1368-
13691359
static ValueDecl *getCreateAsyncTaskFuture(ASTContext &ctx, Identifier id) {
13701360
BuiltinFunctionBuilder builder(ctx);
13711361
auto genericParam = makeGenericParam().build(builder);
@@ -2575,9 +2565,6 @@ ValueDecl *swift::getBuiltinValueDecl(ASTContext &Context, Identifier Id) {
25752565
case BuiltinValueKind::CancelAsyncTask:
25762566
return getCancelAsyncTask(Context, Id);
25772567

2578-
case BuiltinValueKind::CreateAsyncTask:
2579-
return getCreateAsyncTask(Context, Id);
2580-
25812568
case BuiltinValueKind::CreateAsyncTaskFuture:
25822569
return getCreateAsyncTaskFuture(Context, Id);
25832570

‎lib/IRGen/GenBuiltin.cpp‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,7 @@ void irgen::emitBuiltinCall(IRGenFunction &IGF, const BuiltinInfo &Builtin,
226226
return;
227227
}
228228

229-
if (Builtin.ID == BuiltinValueKind::CreateAsyncTask ||
230-
Builtin.ID == BuiltinValueKind::CreateAsyncTaskFuture ||
229+
if (Builtin.ID == BuiltinValueKind::CreateAsyncTaskFuture ||
231230
Builtin.ID == BuiltinValueKind::CreateAsyncTaskGroupFuture) {
232231

233232
auto flags = args.claimNext();
@@ -244,11 +243,6 @@ void irgen::emitBuiltinCall(IRGenFunction &IGF, const BuiltinInfo &Builtin,
244243
auto taskFunction = args.claimNext();
245244
auto taskContext = args.claimNext();
246245

247-
// FIXME: SIL treats the function/context parameter as "guaranteed", but
248-
// the runtime entry point assumes it is owned. Introduce an extra retain
249-
// of the context to balance things out.
250-
IGF.emitNativeStrongRetain(taskContext, IGF.getDefaultAtomicity());
251-
252246
auto newTaskAndContext = emitTaskCreate(
253247
IGF, flags, parentTask, taskGroup, futureResultType, taskFunction, taskContext,
254248
substitutions);

‎lib/IRGen/GenCall.cpp‎

Lines changed: 6 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -3767,86 +3767,24 @@ llvm::Value *irgen::emitTaskCreate(
37673767
parentTask = IGF.Builder.CreateBitOrPointerCast(
37683768
parentTask, IGF.IGM.SwiftTaskPtrTy);
37693769

3770-
// Determine the size of the async context for the closure.
3771-
ASTContext &ctx = IGF.IGM.IRGen.SIL.getASTContext();
3772-
auto extInfo = ASTExtInfoBuilder().withAsync().withThrows().build();
3773-
CanSILFunctionType taskFunctionType;
3774-
CanSILFunctionType substTaskFunctionType;
3775-
if (futureResultType) {
3776-
auto genericParam = GenericTypeParamType::get(0, 0, ctx);
3777-
auto genericSig = GenericSignature::get({genericParam}, {});
3778-
auto *ty = GenericFunctionType::get(genericSig, { }, genericParam, extInfo);
3779-
3780-
taskFunctionType = IGF.IGM.getLoweredType(ty).castTo<SILFunctionType>();
3781-
substTaskFunctionType = taskFunctionType->withInvocationSubstitutions(subs);
3782-
} else {
3783-
auto *ty = FunctionType::get({ }, ctx.TheEmptyTupleType, extInfo);
3784-
taskFunctionType = IGF.IGM.getLoweredType(ty).castTo<SILFunctionType>();
3785-
substTaskFunctionType = taskFunctionType;
3786-
}
3787-
auto layout = getAsyncContextLayout(
3788-
IGF.IGM, taskFunctionType, substTaskFunctionType, subs,
3789-
/*suppress generics*/ false);
3790-
3791-
CanSILFunctionType taskContinuationFunctionTy = [&]() {
3792-
ASTContext &ctx = IGF.IGM.IRGen.SIL.getASTContext();
3793-
auto extInfo =
3794-
ASTExtInfoBuilder()
3795-
.withRepresentation(FunctionTypeRepresentation::CFunctionPointer)
3796-
.build();
3797-
// FIXME: Use the appropriate signature for TaskContinuationFunction:
3798-
//
3799-
// using TaskContinuationFunction =
3800-
// SWIFT_CC(swift)
3801-
// void (AsyncTask *, ExecutorRef, AsyncContext *);
3802-
auto ty = FunctionType::get({}, ctx.TheEmptyTupleType, extInfo);
3803-
return IGF.IGM.getLoweredType(ty).castTo<SILFunctionType>();
3804-
}();
3805-
3806-
// Call the function.
38073770
llvm::CallInst *result;
3808-
llvm::Value *theSize, *theFunction;
3809-
auto taskFunctionPointer = FunctionPointer::forExplosionValue(
3810-
IGF, taskFunction, substTaskFunctionType);
3811-
std::tie(theFunction, theSize) =
3812-
getAsyncFunctionAndSize(IGF, SILFunctionTypeRepresentation::Thick,
3813-
taskFunctionPointer, localContextInfo);
3814-
if (auto authInfo = PointerAuthInfo::forFunctionPointer(
3815-
IGF.IGM, taskContinuationFunctionTy)) {
3816-
theFunction = emitPointerAuthResign(
3817-
IGF, theFunction, taskFunctionPointer.getAuthInfo(), authInfo);
3818-
}
3819-
theFunction = IGF.Builder.CreateBitOrPointerCast(
3820-
theFunction, IGF.IGM.TaskContinuationFunctionPtrTy);
3821-
theSize = IGF.Builder.CreateZExtOrBitCast(theSize, IGF.IGM.SizeTy);
38223771
if (taskGroup && futureResultType) {
38233772
taskGroup = IGF.Builder.CreateBitOrPointerCast(
38243773
taskGroup, IGF.IGM.SwiftTaskGroupPtrTy);
38253774
result = IGF.Builder.CreateCall(
3826-
IGF.IGM.getTaskCreateGroupFutureFuncFn(),
3827-
{flags, parentTask, taskGroup, futureResultType, theFunction, theSize});
3775+
IGF.IGM.getTaskCreateGroupFutureFn(),
3776+
{flags, parentTask, taskGroup, futureResultType,
3777+
taskFunction, localContextInfo});
38283778
} else if (futureResultType) {
38293779
result = IGF.Builder.CreateCall(
3830-
IGF.IGM.getTaskCreateFutureFuncFn(),
3831-
{ flags, parentTask, futureResultType, theFunction, theSize });
3780+
IGF.IGM.getTaskCreateFutureFn(),
3781+
{flags, parentTask, futureResultType, taskFunction, localContextInfo});
38323782
} else {
3833-
result = IGF.Builder.CreateCall(IGF.IGM.getTaskCreateFuncFn(),
3834-
{flags, parentTask, theFunction, theSize});
3783+
llvm_unreachable("no future?!");
38353784
}
38363785
result->setDoesNotThrow();
38373786
result->setCallingConv(IGF.IGM.SwiftCC);
38383787

3839-
// Write the local context information into the initial context for the task.
3840-
assert(layout.hasLocalContext());
3841-
// Dig out the initial context returned from task creation.
3842-
auto initialContext = IGF.Builder.CreateExtractValue(result, {1});
3843-
Address initialContextAddr = layout.emitCastTo(IGF, initialContext);
3844-
3845-
auto localContextLayout = layout.getLocalContextLayout();
3846-
auto localContextAddr =
3847-
localContextLayout.project(IGF, initialContextAddr, llvm::None);
3848-
IGF.Builder.CreateStore(localContextInfo, localContextAddr);
3849-
38503788
return result;
38513789
}
38523790

‎lib/SIL/IR/OperandOwnership.cpp‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,9 @@ namespace {
574574
struct OperandOwnershipBuiltinClassifier
575575
: SILBuiltinVisitor<OperandOwnershipBuiltinClassifier, OperandOwnership> {
576576
using Map = OperandOwnership;
577+
578+
const Operand &op;
579+
OperandOwnershipBuiltinClassifier(const Operand &op) : op(op) {}
577580

578581
OperandOwnership visitLLVMIntrinsic(BuiltinInst *bi, llvm::Intrinsic::ID id) {
579582
// LLVM intrinsics do not traffic in ownership, so if we have a result, it
@@ -744,14 +747,35 @@ BUILTIN_OPERAND_OWNERSHIP(InstantaneousUse, IntInstrprofIncrement)
744747
BUILTIN_OPERAND_OWNERSHIP(ForwardingConsume, COWBufferForReading)
745748
BUILTIN_OPERAND_OWNERSHIP(ForwardingConsume, UnsafeGuaranteed)
746749

747-
// FIXME: These are considered InteriorPointer because they may propagate a
748-
// pointer into a borrowed values. If they do not propagate an interior pointer,
749-
// then they should be InstantaneousUse instead and should not require a
750-
// guaranteed value.
750+
OperandOwnership
751+
OperandOwnershipBuiltinClassifier::visitCreateAsyncTaskFuture(BuiltinInst *bi,
752+
StringRef attr) {
753+
// The function operand is consumed by the new task.
754+
if (&op == &bi->getOperandRef(3))
755+
return OperandOwnership::DestroyingConsume;
756+
757+
// FIXME: These are considered InteriorPointer because they may propagate a
758+
// pointer into a borrowed values. If they do not propagate an interior pointer,
759+
// then they should be InstantaneousUse instead and should not require a
760+
// guaranteed value.
761+
return OperandOwnership::InteriorPointer;
762+
}
763+
764+
OperandOwnership
765+
OperandOwnershipBuiltinClassifier::visitCreateAsyncTaskGroupFuture(BuiltinInst *bi,
766+
StringRef attr) {
767+
// The function operand is consumed by the new task.
768+
if (&op == &bi->getOperandRef(4))
769+
return OperandOwnership::DestroyingConsume;
770+
771+
// FIXME: These are considered InteriorPointer because they may propagate a
772+
// pointer into a borrowed values. If they do not propagate an interior pointer,
773+
// then they should be InstantaneousUse instead and should not require a
774+
// guaranteed value.
775+
return OperandOwnership::InteriorPointer;
776+
}
777+
751778
BUILTIN_OPERAND_OWNERSHIP(InteriorPointer, CancelAsyncTask)
752-
BUILTIN_OPERAND_OWNERSHIP(InteriorPointer, CreateAsyncTask)
753-
BUILTIN_OPERAND_OWNERSHIP(InteriorPointer, CreateAsyncTaskFuture)
754-
BUILTIN_OPERAND_OWNERSHIP(InteriorPointer, CreateAsyncTaskGroupFuture)
755779
BUILTIN_OPERAND_OWNERSHIP(InteriorPointer, InitializeDefaultActor)
756780
BUILTIN_OPERAND_OWNERSHIP(InteriorPointer, DestroyDefaultActor)
757781

@@ -787,7 +811,7 @@ SHOULD_NEVER_VISIT_BUILTIN(GetCurrentAsyncTask)
787811
#include "swift/AST/Builtins.def"
788812

789813
OperandOwnership OperandOwnershipClassifier::visitBuiltinInst(BuiltinInst *bi) {
790-
return OperandOwnershipBuiltinClassifier().check(bi);
814+
return OperandOwnershipBuiltinClassifier(op).check(bi);
791815
}
792816

793817
//===----------------------------------------------------------------------===//

0 commit comments

Comments
 (0)