From dcf952f2b25c9c9615101d359e1243ea22abb03b Mon Sep 17 00:00:00 2001 From: Dario Rexin Date: Mon, 22 Apr 2024 13:54:39 -0700 Subject: [PATCH 1/2] [Runtime] Fix CVW for genreic single payload enums with no extra inhabitants rdar://126728925 When the payload of a generic SPE did not have any extra inhabitants, we erroneously always treated it as the no payload case. Additionally the offset and skip values were improperly computed. --- stdlib/public/runtime/BytecodeLayouts.cpp | 8 +++ stdlib/public/runtime/Enum.cpp | 4 +- .../layout_string_witnesses_types.swift | 5 ++ .../layout_string_witnesses_dynamic.swift | 67 ++++++++++++++++--- 4 files changed, 74 insertions(+), 10 deletions(-) diff --git a/stdlib/public/runtime/BytecodeLayouts.cpp b/stdlib/public/runtime/BytecodeLayouts.cpp index 1ce7761a72935..837751db575a4 100644 --- a/stdlib/public/runtime/BytecodeLayouts.cpp +++ b/stdlib/public/runtime/BytecodeLayouts.cpp @@ -413,6 +413,10 @@ static void singlePayloadEnumGeneric(const Metadata *metadata, if (tagBytes) { xiType = nullptr; + } else if (!xiType) { + // If there are no inhabitants and the extra tag bits are not set, + // we have a payload. + return; } } @@ -633,6 +637,10 @@ static void singlePayloadEnumGeneric(const Metadata *metadata, if (tagBytes) { xiType = nullptr; + } else if (!xiType) { + // If there are no inhabitants and the extra tag bits are not set, + // we have a payload. + return; } } diff --git a/stdlib/public/runtime/Enum.cpp b/stdlib/public/runtime/Enum.cpp index dd338d3fcda0c..2cb443dbd0744 100644 --- a/stdlib/public/runtime/Enum.cpp +++ b/stdlib/public/runtime/Enum.cpp @@ -305,10 +305,10 @@ void swift::swift_initEnumMetadataSinglePayloadWithLayoutString( _swift_addRefCountStringForMetatype(writer, flags, payloadType, fullOffset, previousFieldOffset); - writer.writeBytes((uint64_t)previousFieldOffset); + writer.writeBytes((uint64_t)previousFieldOffset + extraTagBytes); writer.offset = skipBytesOffset; - writer.writeBytes(size - previousFieldOffset); + writer.writeBytes(payloadSize - previousFieldOffset); // we mask out HasRelativePointers, because at this point they have all been // resolved to metadata pointers diff --git a/test/Interpreter/Inputs/layout_string_witnesses_types.swift b/test/Interpreter/Inputs/layout_string_witnesses_types.swift index cb357836f8965..2d1e6c41b33e0 100644 --- a/test/Interpreter/Inputs/layout_string_witnesses_types.swift +++ b/test/Interpreter/Inputs/layout_string_witnesses_types.swift @@ -347,6 +347,11 @@ public struct ContainsSinglePayloadSimpleClassEnum { } } +public enum TestOptional { + case empty + case nonEmpty(T) +} + public enum SinglePayloadEnum { case empty case nonEmpty(Int, T?) diff --git a/test/Interpreter/layout_string_witnesses_dynamic.swift b/test/Interpreter/layout_string_witnesses_dynamic.swift index b583f2ead377c..7ab7c38932d7e 100644 --- a/test/Interpreter/layout_string_witnesses_dynamic.swift +++ b/test/Interpreter/layout_string_witnesses_dynamic.swift @@ -1181,27 +1181,48 @@ func testTupleAlignment() { testTupleAlignment() -#if os(macOS) +func testWeakRefOptionalNative() { + let ptr = allocateInternalGenericPtr(of: TestOptional.self) + let ptr2 = allocateInternalGenericPtr(of: TestOptional.self) -import Foundation + do { + let classInstance = SimpleClass(x: 23) -@objc -final class ObjcClass: NSObject { - deinit { - print("ObjcClass deinitialized!") + do { + let x = TestOptional.nonEmpty(WeakNativeWrapper(x: classInstance)) + let y = TestOptional.nonEmpty(WeakNativeWrapper(x: classInstance)) + testGenericInit(ptr, to: x) + testGenericInit(ptr2, to: y) + } + + testGenericDestroy(ptr, of: TestOptional.self) + testGenericDestroy(ptr2, of: TestOptional.self) + + // CHECK: Before deinit + print("Before deinit") + + // CHECK-NEXT: SimpleClass deinitialized! } + + ptr.deallocate() } +testWeakRefOptionalNative() + +#if os(macOS) + +import Foundation + func testGenericObjc() { let ptr = allocateInternalGenericPtr(of: ObjcClass.self) do { - let x = ObjcClass() + let x = ObjcClass(x: 23) testGenericInit(ptr, to: x) } do { - let y = ObjcClass() + let y = ObjcClass(x: 32) // CHECK-macosx: Before deinit print("Before deinit") @@ -1220,4 +1241,34 @@ func testGenericObjc() { testGenericObjc() +import Darwin + +func testWeakRefOptionalObjc() { + let ptr = allocateInternalGenericPtr(of: TestOptional.self) + let ptr2 = allocateInternalGenericPtr(of: TestOptional.self) + + do { + let classInstance = ObjcClass(x: 23) + + do { + let x = TestOptional.nonEmpty(WeakObjcWrapper(x: classInstance)) + let y = TestOptional.nonEmpty(WeakObjcWrapper(x: classInstance)) + testGenericInit(ptr, to: x) + testGenericInit(ptr2, to: y) + } + + testGenericDestroy(ptr, of: TestOptional.self) + testGenericDestroy(ptr2, of: TestOptional.self) + + // CHECK: Before deinit + print("Before deinit") + + // CHECK-NEXT: ObjcClass deinitialized! + } + + ptr.deallocate() +} + +testWeakRefOptionalObjc() + #endif From fe515dcd0d8953f996b25232e176fcd357854fb8 Mon Sep 17 00:00:00 2001 From: Dario Rexin Date: Mon, 22 Apr 2024 15:01:35 -0700 Subject: [PATCH 2/2] Fixed FileCheck string --- test/Interpreter/layout_string_witnesses_dynamic.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Interpreter/layout_string_witnesses_dynamic.swift b/test/Interpreter/layout_string_witnesses_dynamic.swift index 7ab7c38932d7e..c7e06c320f8f0 100644 --- a/test/Interpreter/layout_string_witnesses_dynamic.swift +++ b/test/Interpreter/layout_string_witnesses_dynamic.swift @@ -1260,10 +1260,10 @@ func testWeakRefOptionalObjc() { testGenericDestroy(ptr, of: TestOptional.self) testGenericDestroy(ptr2, of: TestOptional.self) - // CHECK: Before deinit + // CHECK-macosx: Before deinit print("Before deinit") - // CHECK-NEXT: ObjcClass deinitialized! + // CHECK-macosx-NEXT: ObjcClass deinitialized! } ptr.deallocate()