Skip to content

Commit a64564f

Browse files
authored
Merge pull request swiftlang#39829 from rjmccall/fix_alignment_future_fragment
Fix the alignment of future fragments for highly-aligned result types
2 parents bab9189 + d7cae26 commit a64564f

File tree

2 files changed

+30
-15
lines changed

2 files changed

+30
-15
lines changed

include/swift/ABI/Task.h

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -480,27 +480,42 @@ class AsyncTask : public Job {
480480
return resultType;
481481
}
482482

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

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

492-
/// Compute the offset of the storage from the base of the future
493-
/// fragment.
494-
static size_t storageOffset(const Metadata *resultType) {
495-
size_t offset = sizeof(FutureFragment);
509+
/// Determine the size of the future fragment given the result type
510+
/// of the future.
511+
static size_t fragmentSize(size_t fragmentOffset,
512+
const Metadata *resultType) {
513+
assert((fragmentOffset & (alignof(FutureFragment) - 1)) == 0);
496514
size_t alignment = resultType->vw_alignment();
497-
return (offset + alignment - 1) & ~(alignment - 1);
498-
}
499-
500-
/// Determine the size of the future fragment given a particular future
501-
/// result type.
502-
static size_t fragmentSize(const Metadata *resultType) {
503-
return storageOffset(resultType) + resultType->vw_size();
515+
size_t resultOffset = fragmentOffset + sizeof(FutureFragment);
516+
resultOffset = (resultOffset + alignment - 1) & ~(alignment - 1);
517+
size_t endOffset = resultOffset + resultType->vw_size();
518+
return (endOffset - fragmentOffset);
504519
}
505520
};
506521

stdlib/public/Concurrency/Task.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ static AsyncTaskAndContext swift_task_create_commonImpl(
513513
headerSize += sizeof(AsyncTask::GroupChildFragment);
514514
}
515515
if (futureResultType) {
516-
headerSize += FutureFragment::fragmentSize(futureResultType);
516+
headerSize += FutureFragment::fragmentSize(headerSize, futureResultType);
517517
// Add the future async context prefix.
518518
headerSize += sizeof(FutureAsyncContextPrefix);
519519
} else {

0 commit comments

Comments
 (0)