From 68b9ccc4ad0b9d7f104c5033caa3c9127e9eba41 Mon Sep 17 00:00:00 2001
From: Arnold Schwaighofer <aschwaighofer@apple.com>
Date: Tue, 19 Oct 2021 16:48:38 -0700
Subject: [PATCH 1/2] Make sure that the future fragment's storage pointer is
 properly aligned

---
 include/swift/ABI/Task.h           | 20 +++++++++-----------
 stdlib/public/Concurrency/Task.cpp |  2 +-
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/include/swift/ABI/Task.h b/include/swift/ABI/Task.h
index cbbb88b714a7b..9f959a82108b0 100644
--- a/include/swift/ABI/Task.h
+++ b/include/swift/ABI/Task.h
@@ -482,25 +482,23 @@ class AsyncTask : public Job {
 
     /// Retrieve a pointer to the storage of result.
     OpaqueValue *getStoragePtr() {
+      auto *startAddr = reinterpret_cast<char *>(this) + sizeof(FutureFragment);
+      uintptr_t startAddrVal = (uintptr_t)startAddr;
+      uintptr_t alignment = resultType->vw_alignment();
+      startAddrVal = (startAddrVal + alignment -1) & ~(alignment -1);
       return reinterpret_cast<OpaqueValue *>(
-          reinterpret_cast<char *>(this) + storageOffset(resultType));
+          reinterpret_cast<char *>(startAddrVal));
     }
 
     /// 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);
-      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();
+    static size_t fragmentSize(size_t initialOffset, const Metadata *resultType) {
+      size_t alignment = resultType->vw_alignment();
+      size_t padding = alignment - ((sizeof(FutureFragment) + initialOffset) % alignment);
+      return sizeof(FutureFragment) + padding + resultType->vw_size();
     }
   };
 
diff --git a/stdlib/public/Concurrency/Task.cpp b/stdlib/public/Concurrency/Task.cpp
index b7861059c0f4e..8c8d0d3a7eadc 100644
--- a/stdlib/public/Concurrency/Task.cpp
+++ b/stdlib/public/Concurrency/Task.cpp
@@ -513,7 +513,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 {

From d7cae261717642037d1ac54459a846e05b45578c Mon Sep 17 00:00:00 2001
From: John McCall <rjmccall@apple.com>
Date: Wed, 20 Oct 2021 02:07:48 -0400
Subject: [PATCH 2/2] Better comments, fix a calculation.

---
 include/swift/ABI/Task.h | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/include/swift/ABI/Task.h b/include/swift/ABI/Task.h
index 9f959a82108b0..451b9eaf5c8e7 100644
--- a/include/swift/ABI/Task.h
+++ b/include/swift/ABI/Task.h
@@ -480,25 +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() {
-      auto *startAddr = reinterpret_cast<char *>(this) + sizeof(FutureFragment);
-      uintptr_t startAddrVal = (uintptr_t)startAddr;
+      // 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();
-      startAddrVal = (startAddrVal + alignment -1) & ~(alignment -1);
-      return reinterpret_cast<OpaqueValue *>(
-          reinterpret_cast<char *>(startAddrVal));
+      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; }
 
-    /// Determine the size of the future fragment given a particular future
-    /// result type.
-    static size_t fragmentSize(size_t initialOffset, const Metadata *resultType) {
+    /// 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();
-      size_t padding = alignment - ((sizeof(FutureFragment) + initialOffset) % alignment);
-      return sizeof(FutureFragment) + padding + 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);
     }
   };