Skip to content

[5.5] Fix the alignment of future fragments for highly-aligned result types #39830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 29 additions & 14 deletions include/swift/ABI/Task.h
Original file line number Diff line number Diff line change
Expand Up @@ -480,27 +480,42 @@ class AsyncTask : public Job {
return resultType;
}

/// Retrieve a pointer to the storage of result.
/// Retrieve a pointer to the storage of the result.
OpaqueValue *getStoragePtr() {
return reinterpret_cast<OpaqueValue *>(
reinterpret_cast<char *>(this) + storageOffset(resultType));
// The result storage starts at the first aligned offset following
// the fragment header. This offset will agree with the abstract
// calculation for `resultOffset` in the fragmentSize function below
// because the entire task is aligned to at least the target
// alignment (because it's aligned to MaxAlignment), which means
// `this` must have the same value modulo that alignment as
// `fragmentOffset` has in that function.
char *fragmentAddr = reinterpret_cast<char *>(this);
uintptr_t alignment = resultType->vw_alignment();
char *resultAddr = fragmentAddr + sizeof(FutureFragment);
uintptr_t unalignedResultAddrInt =
reinterpret_cast<uintptr_t>(resultAddr);
uintptr_t alignedResultAddrInt =
(unalignedResultAddrInt + alignment - 1) & ~(alignment - 1);
// We could just cast alignedResultAddrInt back to a pointer, but
// doing pointer arithmetic is more strictly conformant and less
// likely to annoy the optimizer.
resultAddr += (alignedResultAddrInt - unalignedResultAddrInt);
return reinterpret_cast<OpaqueValue *>(resultAddr);
}

/// Retrieve the error.
SwiftError *&getError() { return error; }

/// Compute the offset of the storage from the base of the future
/// fragment.
static size_t storageOffset(const Metadata *resultType) {
size_t offset = sizeof(FutureFragment);
/// Determine the size of the future fragment given the result type
/// of the future.
static size_t fragmentSize(size_t fragmentOffset,
const Metadata *resultType) {
assert((fragmentOffset & (alignof(FutureFragment) - 1)) == 0);
size_t alignment = resultType->vw_alignment();
return (offset + alignment - 1) & ~(alignment - 1);
}

/// Determine the size of the future fragment given a particular future
/// result type.
static size_t fragmentSize(const Metadata *resultType) {
return storageOffset(resultType) + resultType->vw_size();
size_t resultOffset = fragmentOffset + sizeof(FutureFragment);
resultOffset = (resultOffset + alignment - 1) & ~(alignment - 1);
size_t endOffset = resultOffset + resultType->vw_size();
return (endOffset - fragmentOffset);
}
};

Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/Concurrency/Task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ static AsyncTaskAndContext swift_task_create_commonImpl(
headerSize += sizeof(AsyncTask::GroupChildFragment);
}
if (futureResultType) {
headerSize += FutureFragment::fragmentSize(futureResultType);
headerSize += FutureFragment::fragmentSize(headerSize, futureResultType);
// Add the future async context prefix.
headerSize += sizeof(FutureAsyncContextPrefix);
} else {
Expand Down