From 63a1bb070512ecdc002d5985e7d7122dad11924c Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Thu, 3 Apr 2025 19:45:46 -0700 Subject: [PATCH] [CoroutineAccessors] Use async bit in descriptors. To facilitate back deployment, make use of the fact that the async bit has up to now never been set for read and modify accessors and claim that set bit to indicate that it is a callee-allocated coroutine. This has the virtue of being completely back deployable because like async function pointers coro function pointers must be auth'd and signed as data. --- include/swift/ABI/MetadataValues.h | 34 +++++++++++++++++------------- lib/IRGen/GenMeta.cpp | 33 ++++++++++++++++++++--------- stdlib/public/runtime/Metadata.cpp | 4 ---- 3 files changed, 42 insertions(+), 29 deletions(-) diff --git a/include/swift/ABI/MetadataValues.h b/include/swift/ABI/MetadataValues.h index 8007e3ab75e82..f239ae336c9cb 100644 --- a/include/swift/ABI/MetadataValues.h +++ b/include/swift/ABI/MetadataValues.h @@ -374,8 +374,6 @@ class MethodDescriptorFlags { Setter, ModifyCoroutine, ReadCoroutine, - Read2Coroutine, - Modify2Coroutine, }; private: @@ -438,23 +436,27 @@ class MethodDescriptorFlags { /// Note that 'init' is not considered an instance member. bool isInstance() const { return Value & IsInstanceMask; } - bool isAsync() const { return Value & IsAsyncMask; } + bool _hasAsyncBitSet() const { return Value & IsAsyncMask; } - bool isCalleeAllocatedCoroutine() const { + bool isAsync() const { return !isCoroutine() && _hasAsyncBitSet(); } + + bool isCoroutine() const { switch (getKind()) { case Kind::Method: case Kind::Init: case Kind::Getter: case Kind::Setter: + return false; case Kind::ModifyCoroutine: case Kind::ReadCoroutine: - return false; - case Kind::Read2Coroutine: - case Kind::Modify2Coroutine: return true; } } + bool isCalleeAllocatedCoroutine() const { + return isCoroutine() && _hasAsyncBitSet(); + } + bool isData() const { return isAsync() || isCalleeAllocatedCoroutine(); } uint16_t getExtraDiscriminator() const { @@ -615,8 +617,6 @@ class ProtocolRequirementFlags { ModifyCoroutine, AssociatedTypeAccessFunction, AssociatedConformanceAccessFunction, - Read2Coroutine, - Modify2Coroutine, }; private: @@ -666,26 +666,30 @@ class ProtocolRequirementFlags { /// Note that 'init' is not considered an instance member. bool isInstance() const { return Value & IsInstanceMask; } - bool isAsync() const { return Value & IsAsyncMask; } + bool _hasAsyncBitSet() const { return Value & IsAsyncMask; } - bool isCalleeAllocatedCoroutine() const { + bool isAsync() const { return !isCoroutine() && _hasAsyncBitSet(); } + + bool isCoroutine() const { switch (getKind()) { case Kind::BaseProtocol: case Kind::Method: case Kind::Init: case Kind::Getter: case Kind::Setter: - case Kind::ReadCoroutine: - case Kind::ModifyCoroutine: case Kind::AssociatedTypeAccessFunction: case Kind::AssociatedConformanceAccessFunction: return false; - case Kind::Read2Coroutine: - case Kind::Modify2Coroutine: + case Kind::ReadCoroutine: + case Kind::ModifyCoroutine: return true; } } + bool isCalleeAllocatedCoroutine() const { + return isCoroutine() && _hasAsyncBitSet(); + } + bool isData() const { return isAsync() || isCalleeAllocatedCoroutine(); } bool isSignedWithAddress() const { diff --git a/lib/IRGen/GenMeta.cpp b/lib/IRGen/GenMeta.cpp index 26dbc957695d3..f4bc54fae4dab 100644 --- a/lib/IRGen/GenMeta.cpp +++ b/lib/IRGen/GenMeta.cpp @@ -255,22 +255,24 @@ static Flags getMethodDescriptorFlags(ValueDecl *fn) { return flags; } - auto kind = [&] { + auto kindAndIsCalleeAllocatedCoroutine = + [&]() -> std::pair { auto accessor = dyn_cast(fn); - if (!accessor) return Flags::Kind::Method; + if (!accessor) + return {Flags::Kind::Method, false}; switch (accessor->getAccessorKind()) { case AccessorKind::Get: - return Flags::Kind::Getter; + return {Flags::Kind::Getter, false}; case AccessorKind::Set: - return Flags::Kind::Setter; + return {Flags::Kind::Setter, false}; case AccessorKind::Read: - return Flags::Kind::ReadCoroutine; + return {Flags::Kind::ReadCoroutine, false}; case AccessorKind::Read2: - return Flags::Kind::Read2Coroutine; + return {Flags::Kind::ReadCoroutine, true}; case AccessorKind::Modify: - return Flags::Kind::ModifyCoroutine; + return {Flags::Kind::ModifyCoroutine, false}; case AccessorKind::Modify2: - return Flags::Kind::Modify2Coroutine; + return {Flags::Kind::ModifyCoroutine, true}; #define OPAQUE_ACCESSOR(ID, KEYWORD) #define ACCESSOR(ID) \ case AccessorKind::ID: @@ -280,9 +282,20 @@ static Flags getMethodDescriptorFlags(ValueDecl *fn) { } llvm_unreachable("bad kind"); }(); - bool hasAsync = false; + auto kind = kindAndIsCalleeAllocatedCoroutine.first; + + // Because no async old-ABI accessor coroutines exist or presumably ever will + // (if async coroutines accessors are added, they will presumably be new-ABI), + // the pairs {Flags::Kind::ReadCoroutine, isAsync} and + // {Flags::Kind::ModifyCoroutine, isAsync} can't mean "async old-ABI + // accessor coroutine". As such, we repurpose that pair to mean "new-ABI + // accessor coroutine". This has the important virtue of resulting in ptrauth + // authing/signing coro function pointers as data on old OSes where the bit + // means "async" and where adding new accessor kinds requires a back + // deployment library. + bool hasAsync = kindAndIsCalleeAllocatedCoroutine.second; if (auto *afd = dyn_cast(fn)) - hasAsync = afd->hasAsync(); + hasAsync |= afd->hasAsync(); return Flags(kind).withIsInstance(!fn->isStatic()).withIsAsync(hasAsync); } diff --git a/stdlib/public/runtime/Metadata.cpp b/stdlib/public/runtime/Metadata.cpp index ae8859203ec5d..71a3961b805a9 100644 --- a/stdlib/public/runtime/Metadata.cpp +++ b/stdlib/public/runtime/Metadata.cpp @@ -6162,9 +6162,7 @@ static void initProtocolWitness(void **slot, void *witness, case ProtocolRequirementFlags::Kind::Getter: case ProtocolRequirementFlags::Kind::Setter: case ProtocolRequirementFlags::Kind::ReadCoroutine: - case ProtocolRequirementFlags::Kind::Read2Coroutine: case ProtocolRequirementFlags::Kind::ModifyCoroutine: - case ProtocolRequirementFlags::Kind::Modify2Coroutine: swift_ptrauth_init_code_or_data(slot, witness, reqt.Flags.getExtraDiscriminator(), !reqt.Flags.isData()); @@ -6205,9 +6203,7 @@ static void copyProtocolWitness(void **dest, void * const *src, case ProtocolRequirementFlags::Kind::Getter: case ProtocolRequirementFlags::Kind::Setter: case ProtocolRequirementFlags::Kind::ReadCoroutine: - case ProtocolRequirementFlags::Kind::Read2Coroutine: case ProtocolRequirementFlags::Kind::ModifyCoroutine: - case ProtocolRequirementFlags::Kind::Modify2Coroutine: swift_ptrauth_copy_code_or_data( dest, src, reqt.Flags.getExtraDiscriminator(), !reqt.Flags.isData(), /*allowNull*/ true); // NULL allowed for VFE (methods in the vtable