From f087b5e0fbd713ced8de3c4b0be9e12a2cfdd68d Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Thu, 15 Dec 2022 10:22:10 -0800 Subject: [PATCH 1/3] This PR aligns RemoteMirror more closely with the compiler's handling of enums that contain zero-sized payloads, which should resolve a number of issues with RemoteMirror incorrectly reflecting enum cases and/or measuring the layout of structures containing enums. Background: Enum cases that have zero-sized payloads are handled differently from other payload-bearing cases. 1. For layout purposes, they're treated as non-payload cases. This can cause an MPE to actually get represented in memory as a single-payload or non-payload enum. 2. However, zero-sized payloads are still considered for extra inhabitant calculations. Since they have no extra inhabitants, this tends to cause such enums to also not expose extra inhabitants to containing enums. This commit makes several change to how RemoteMirror determines enum layout: * The various "*EnumTypeInfo" classes now represent layout mechanisms -- as described in (1) above, this can differ from the source code concept. * An Enum "kind" is separately computed to reflect the source code concept; this ensures that the dumped type information reflects the source code. * For single-payload and no-payload _layouts_, the extra inhabitant calculation has been adjusted to ensure that zero-sized payloads are correctly considered. Resolves: rdar://92945673 --- include/swift/RemoteInspection/TypeLowering.h | 8 + .../public/RemoteInspection/TypeLowering.cpp | 99 ++++++--- test/Reflection/typeref_lowering.swift | 2 +- ...reflect_Enum_MultiPayload_degenerate.swift | 209 ++++++++++++++++++ .../reflect_Enum_SingleCaseNoPayload.swift | 9 +- 5 files changed, 290 insertions(+), 37 deletions(-) create mode 100644 validation-test/Reflection/reflect_Enum_MultiPayload_degenerate.swift diff --git a/include/swift/RemoteInspection/TypeLowering.h b/include/swift/RemoteInspection/TypeLowering.h index bdaf49bc452e4..1cdad3be5d93b 100644 --- a/include/swift/RemoteInspection/TypeLowering.h +++ b/include/swift/RemoteInspection/TypeLowering.h @@ -239,6 +239,14 @@ class EnumTypeInfo : public TypeInfo { return std::count_if(Cases.begin(), Cases.end(), [](const FieldInfo &Case){return Case.TR != 0;}); } + unsigned getNumNonEmptyPayloadCases() const { + auto Cases = getCases(); + return std::count_if(Cases.begin(), Cases.end(), + [](const FieldInfo &Case){ + return Case.TR != 0 + && Case.TI.getSize() != 0; + }); + } // Size of the payload area. unsigned getPayloadSize() const { return EnumTypeInfo::getPayloadSizeForCases(Cases); diff --git a/stdlib/public/RemoteInspection/TypeLowering.cpp b/stdlib/public/RemoteInspection/TypeLowering.cpp index 51546fc770ff8..204fe9b2a77cc 100644 --- a/stdlib/public/RemoteInspection/TypeLowering.cpp +++ b/stdlib/public/RemoteInspection/TypeLowering.cpp @@ -418,13 +418,13 @@ class EmptyEnumTypeInfo: public EnumTypeInfo { // Enum with a single non-payload case class TrivialEnumTypeInfo: public EnumTypeInfo { public: - TrivialEnumTypeInfo(const std::vector &Cases) + TrivialEnumTypeInfo(EnumKind Kind, const std::vector &Cases) : EnumTypeInfo(/*Size*/ 0, /* Alignment*/ 1, /*Stride*/ 1, /*NumExtraInhabitants*/ 0, /*BitwiseTakable*/ true, - EnumKind::NoPayloadEnum, Cases) {} + Kind, Cases) {} bool readExtraInhabitantIndex(remote::MemoryReader &reader, remote::RemoteAddress address, @@ -446,12 +446,13 @@ class NoPayloadEnumTypeInfo: public EnumTypeInfo { public: NoPayloadEnumTypeInfo(unsigned Size, unsigned Alignment, unsigned Stride, unsigned NumExtraInhabitants, + EnumKind Kind, const std::vector &Cases) : EnumTypeInfo(Size, Alignment, Stride, NumExtraInhabitants, /*BitwiseTakable*/ true, - EnumKind::NoPayloadEnum, Cases) { + Kind, Cases) { assert(Cases.size() >= 2); - assert(getNumPayloadCases() == 0); + assert(getNumNonEmptyPayloadCases() == 0); } bool readExtraInhabitantIndex(remote::MemoryReader &reader, @@ -491,11 +492,12 @@ class SinglePayloadEnumTypeInfo: public EnumTypeInfo { SinglePayloadEnumTypeInfo(unsigned Size, unsigned Alignment, unsigned Stride, unsigned NumExtraInhabitants, bool BitwiseTakable, + EnumKind Kind, const std::vector &Cases) : EnumTypeInfo(Size, Alignment, Stride, NumExtraInhabitants, - BitwiseTakable, EnumKind::SinglePayloadEnum, Cases) { + BitwiseTakable, Kind, Cases) { assert(Cases[0].TR != 0); - assert(getNumPayloadCases() == 1); + assert(getNumNonEmptyPayloadCases() == 1); } bool readExtraInhabitantIndex(remote::MemoryReader &reader, @@ -611,7 +613,7 @@ class SimpleMultiPayloadEnumTypeInfo: public EnumTypeInfo { BitwiseTakable, EnumKind::MultiPayloadEnum, Cases) { assert(Cases[0].TR != 0); assert(Cases[1].TR != 0); - assert(getNumPayloadCases() > 1); + assert(getNumNonEmptyPayloadCases() > 1); assert(getSize() > getPayloadSize()); assert(getCases().size() > 1); } @@ -986,7 +988,7 @@ class MultiPayloadEnumTypeInfo: public EnumTypeInfo { spareBitsMask(spareBitsMask) { assert(Cases[0].TR != 0); assert(Cases[1].TR != 0); - assert(getNumPayloadCases() > 1); + assert(getNumNonEmptyPayloadCases() > 1); } bool readExtraInhabitantIndex(remote::MemoryReader &reader, @@ -1043,7 +1045,7 @@ class MultiPayloadEnumTypeInfo: public EnumTypeInfo { // Check whether this tag is used for valid content auto payloadCases = getNumPayloadCases(); - auto nonPayloadCases = getNumCases() - getNumPayloadCases(); + auto nonPayloadCases = getNumCases() - payloadCases; uint32_t inhabitedTags; if (nonPayloadCases == 0) { inhabitedTags = payloadCases; @@ -2031,7 +2033,8 @@ class EnumTypeInfoBuilder { const TypeInfo *build(const TypeRef *TR, RemoteRef FD, remote::TypeInfoProvider *ExternalTypeInfo) { // Sort enum into payload and no-payload cases. - unsigned NoPayloadCases = 0; + unsigned TrueNoPayloadCases = 0; + unsigned EmptyPayloadCases = 0; std::vector PayloadCases; std::vector Fields; @@ -2042,41 +2045,71 @@ class EnumTypeInfoBuilder { for (auto Case : Fields) { if (Case.TR == nullptr) { - ++NoPayloadCases; + ++TrueNoPayloadCases; addCase(Case.Name); } else { - PayloadCases.push_back(Case); auto *CaseTR = getCaseTypeRef(Case); assert(CaseTR != nullptr); auto *CaseTI = TC.getTypeInfo(CaseTR, ExternalTypeInfo); + if (CaseTI->getSize() == 0) { + // Zero-sized payloads get special treatment + ++EmptyPayloadCases; + } else { + PayloadCases.push_back(Case); + } addCase(Case.Name, CaseTR, CaseTI); } } + // For layout purposes, cases w/ empty payload are + // treated the same as cases with no payload. + unsigned EffectiveNoPayloadCases = TrueNoPayloadCases + EmptyPayloadCases; if (Cases.empty()) { return TC.makeTypeInfo(Cases); } + // `Kind` is used when dumping data, so it reflects how the enum was + // declared in source; the various *TypeInfo classes mentioned below reflect + // the in-memory layout, which may be different because cases whose + // payload is zero-sized get treated (for layout purposes) as non-payload + // cases. + EnumKind Kind; + switch (PayloadCases.size() + EmptyPayloadCases) { + case 0: Kind = EnumKind::NoPayloadEnum; break; + case 1: Kind = EnumKind::SinglePayloadEnum; break; + default: Kind = EnumKind::MultiPayloadEnum; break; + } + if (PayloadCases.empty()) { // NoPayloadEnumImplStrategy - if (NoPayloadCases == 1) { - return TC.makeTypeInfo(Cases); - } else if (NoPayloadCases < 256) { - return TC.makeTypeInfo( - /* Size */ 1, /* Alignment */ 1, /* Stride */ 1, - /* NumExtraInhabitants */ 256 - NoPayloadCases, Cases); - } else if (NoPayloadCases < 65536) { - return TC.makeTypeInfo( - /* Size */ 2, /* Alignment */ 2, /* Stride */ 2, - /* NumExtraInhabitants */ 65536 - NoPayloadCases, Cases); + if (EffectiveNoPayloadCases == 1) { + return TC.makeTypeInfo(Kind, Cases); } else { - auto extraInhabitants = std::numeric_limits::max() - NoPayloadCases + 1; - if (extraInhabitants > ValueWitnessFlags::MaxNumExtraInhabitants) { - extraInhabitants = ValueWitnessFlags::MaxNumExtraInhabitants; + unsigned Size, NumExtraInhabitants; + if (EffectiveNoPayloadCases < 256) { + Size = 1; + NumExtraInhabitants = 256 - EffectiveNoPayloadCases; + } else if (EffectiveNoPayloadCases < 65536) { + Size = 2; + NumExtraInhabitants = 65536 - EffectiveNoPayloadCases; + } else { + Size = 4; + NumExtraInhabitants = std::numeric_limits::max() - EffectiveNoPayloadCases + 1; + } + if (EmptyPayloadCases > 0) { + // This enum uses no-payload layout, but the source actually does + // have payloads (they're just all zero-sized). + // If this is really a single-payload enum, we take extra inhabitants + // from the first payload, which is zero sized in this case. + // If this is really a multi-payload enum, ... + NumExtraInhabitants = 0; + } + if (NumExtraInhabitants > ValueWitnessFlags::MaxNumExtraInhabitants) { + NumExtraInhabitants = ValueWitnessFlags::MaxNumExtraInhabitants; } return TC.makeTypeInfo( - /* Size */ 4, /* Alignment */ 4, /* Stride */ 4, - /* NumExtraInhabitants */ extraInhabitants, Cases); + /* Size */ Size, /* Alignment */ Size, /* Stride */ Size, + NumExtraInhabitants, Kind, Cases); } } else if (PayloadCases.size() == 1) { // SinglePayloadEnumImplStrategy @@ -2087,26 +2120,26 @@ class EnumTypeInfoBuilder { } // An enum consisting of a single payload case and nothing else // is lowered as the payload type. - if (NoPayloadCases == 0) + if (EffectiveNoPayloadCases == 0) return CaseTI; // Below logic should match the runtime function // swift_initEnumMetadataSinglePayload(). auto PayloadExtraInhabitants = CaseTI->getNumExtraInhabitants(); - if (PayloadExtraInhabitants >= NoPayloadCases) { + if (PayloadExtraInhabitants >= EffectiveNoPayloadCases) { // Extra inhabitants can encode all no-payload cases. - NumExtraInhabitants = PayloadExtraInhabitants - NoPayloadCases; + NumExtraInhabitants = PayloadExtraInhabitants - EffectiveNoPayloadCases; } else { // Not enough extra inhabitants for all cases. We have to add an // extra tag field. NumExtraInhabitants = 0; - auto tagCounts = getEnumTagCounts(Size, NoPayloadCases, + auto tagCounts = getEnumTagCounts(Size, EffectiveNoPayloadCases, /*payloadCases=*/1); Size += tagCounts.numTagBytes; Alignment = std::max(Alignment, tagCounts.numTagBytes); } unsigned Stride = ((Size + Alignment - 1) & ~(Alignment - 1)); return TC.makeTypeInfo( - Size, Alignment, Stride, NumExtraInhabitants, BitwiseTakable, Cases); + Size, Alignment, Stride, NumExtraInhabitants, BitwiseTakable, Kind, Cases); } else { // MultiPayloadEnumImplStrategy @@ -2207,7 +2240,7 @@ class EnumTypeInfoBuilder { } else { // Dynamic multi-payload enums cannot use spare bits, so they // always use a separate tag value: - auto tagCounts = getEnumTagCounts(Size, NoPayloadCases, + auto tagCounts = getEnumTagCounts(Size, EffectiveNoPayloadCases, PayloadCases.size()); Size += tagCounts.numTagBytes; // Dynamic multi-payload enums use the tag representations not assigned diff --git a/test/Reflection/typeref_lowering.swift b/test/Reflection/typeref_lowering.swift index 8aa81b86af796..03df741b3d8e3 100644 --- a/test/Reflection/typeref_lowering.swift +++ b/test/Reflection/typeref_lowering.swift @@ -1086,7 +1086,7 @@ // CHECK-64-NEXT: (case name=C index=2) // CHECK-64-NEXT: (case name=D index=3))) // CHECK-64-NEXT: (field name=sillyNoPayload offset=1 -// CHECK-64-NEXT: (multi_payload_enum size=1 alignment=1 stride=1 num_extra_inhabitants=252 bitwise_takable=1 +// CHECK-64-NEXT: (multi_payload_enum size=1 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1 // CHECK-64-NEXT: (case name=A index=0 offset=0 // CHECK-64-NEXT: (no_payload_enum size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1)) // CHECK-64-NEXT: (case name=B index=1 offset=0 diff --git a/validation-test/Reflection/reflect_Enum_MultiPayload_degenerate.swift b/validation-test/Reflection/reflect_Enum_MultiPayload_degenerate.swift new file mode 100644 index 0000000000000..27835f6a92f26 --- /dev/null +++ b/validation-test/Reflection/reflect_Enum_MultiPayload_degenerate.swift @@ -0,0 +1,209 @@ +// RUN: %empty-directory(%t) +// RUN: %target-build-swift -g -lswiftSwiftReflectionTest %s -o %t/reflect_Enum_MultiPayload_degenerate +// RUN: %target-codesign %t/reflect_Enum_MultiPayload_degenerate + +// RUN: %target-run %target-swift-reflection-test %t/reflect_Enum_MultiPayload_degenerate | %FileCheck %s --check-prefix=CHECK-%target-ptrsize + +// REQUIRES: reflection_test_support +// REQUIRES: executable_test +// UNSUPPORTED: use_os_stdlib + +import SwiftReflectionTest + +// Only one case has a non-zero-sized payload, so this gets +// laid out the same as a single-payload enum +enum FooVoid { +case a([Int]) +case b(Void) +} + +reflect(enum: FooVoid.a([])) + +// CHECK-64: Reflecting an enum. +// CHECK-64: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}} +// CHECK-64: Type reference: +// CHECK-64: (enum reflect_Enum_MultiPayload_degenerate.FooVoid) + +// CHECK-64: Type info: +// CHECK-64: (multi_payload_enum size=8 alignment=8 stride=8 num_extra_inhabitants=2147483646 bitwise_takable=1 +// CHECK-64: (case name=a index=0 offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=_buffer offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=_storage offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=rawValue offset=0 +// CHECK-64: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1)))))))) +// CHECK-64: (case name=b index=1 offset=0 +// CHECK-64: (tuple size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))) +// CHECK-64: Mangled name: $s36reflect_Enum_MultiPayload_degenerate7FooVoidO +// CHECK-64: Demangled name: reflect_Enum_MultiPayload_degenerate.FooVoid + +// CHECK-64: Enum value: +// CHECK-64: (enum_value name=a index=0 +// CHECK-64: (bound_generic_struct Swift.Array +// CHECK-64: (struct Swift.Int)) +// CHECK-64: ) + +reflect(enum: FooVoid.b(())) + +// CHECK-64: Reflecting an enum. +// CHECK-64: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}} +// CHECK-64: Type reference: +// CHECK-64: (enum reflect_Enum_MultiPayload_degenerate.FooVoid) + +// CHECK-64: Type info: +// CHECK-64: (multi_payload_enum size=8 alignment=8 stride=8 num_extra_inhabitants=2147483646 bitwise_takable=1 +// CHECK-64: (case name=a index=0 offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=_buffer offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=_storage offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=rawValue offset=0 +// CHECK-64: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1)))))))) +// CHECK-64: (case name=b index=1 offset=0 +// CHECK-64: (tuple size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))) +// CHECK-64: Mangled name: $s36reflect_Enum_MultiPayload_degenerate7FooVoidO +// CHECK-64: Demangled name: reflect_Enum_MultiPayload_degenerate.FooVoid + +// CHECK-64: Enum value: +// CHECK-64: (enum_value name=b index=1 +// CHECK-64: (tuple) +// CHECK-64: ) + + +// Same as above, except the first payload has zero size +enum FooVoid2 { +case a(Void) +case b([Int]) +} + +reflect(enum: FooVoid2.a(())) + +// CHECK-64: Reflecting an enum. +// CHECK-64: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}} +// CHECK-64: Type reference: +// CHECK-64: (enum reflect_Enum_MultiPayload_degenerate.FooVoid2) + +// CHECK-64: Type info: +// CHECK-64: (multi_payload_enum size=8 alignment=8 stride=8 num_extra_inhabitants=2147483646 bitwise_takable=1 +// CHECK-64: (case name=b index=0 offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=_buffer offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=_storage offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=rawValue offset=0 +// CHECK-64: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1)))))))) +// CHECK-64: (case name=a index=1 offset=0 +// CHECK-64: (tuple size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))) +// CHECK-64: Mangled name: $s36reflect_Enum_MultiPayload_degenerate8FooVoid2O +// CHECK-64: Demangled name: reflect_Enum_MultiPayload_degenerate.FooVoid2 + +// CHECK-64: Enum value: +// CHECK-64: (enum_value name=a index=1 +// CHECK-64: (tuple) +// CHECK-64: ) + +reflect(enum: FooVoid2.b([])) + +// CHECK-64: Reflecting an enum. +// CHECK-64: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}} +// CHECK-64: Type reference: +// CHECK-64: (enum reflect_Enum_MultiPayload_degenerate.FooVoid2) + +// CHECK-64: Type info: +// CHECK-64: (multi_payload_enum size=8 alignment=8 stride=8 num_extra_inhabitants=2147483646 bitwise_takable=1 +// CHECK-64: (case name=b index=0 offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=_buffer offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=_storage offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=rawValue offset=0 +// CHECK-64: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1)))))))) +// CHECK-64: (case name=a index=1 offset=0 +// CHECK-64: (tuple size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))) +// CHECK-64: Mangled name: $s36reflect_Enum_MultiPayload_degenerate8FooVoid2O +// CHECK-64: Demangled name: reflect_Enum_MultiPayload_degenerate.FooVoid2 + +// CHECK-64: Enum value: +// CHECK-64: (enum_value name=b index=0 +// CHECK-64: (bound_generic_struct Swift.Array +// CHECK-64: (struct Swift.Int)) +// CHECK-64: ) + + +// As above, this is laid out as a single-payload enum +// because the `b` case payload has zero size +struct B {} +enum FooEmptyStruct { +case a([Int]) +case b(B) +} + +reflect(enum: FooEmptyStruct.a([])) + +// CHECK-64: Reflecting an enum. +// CHECK-64: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}} +// CHECK-64: Type reference: +// CHECK-64: (enum reflect_Enum_MultiPayload_degenerate.FooEmptyStruct) + +// CHECK-64: Type info: +// CHECK-64: (multi_payload_enum size=8 alignment=8 stride=8 num_extra_inhabitants=2147483646 bitwise_takable=1 +// CHECK-64: (case name=a index=0 offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=_buffer offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=_storage offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=rawValue offset=0 +// CHECK-64: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1)))))))) +// CHECK-64: (case name=b index=1 offset=0 +// CHECK-64: (struct size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))) +// CHECK-64: Mangled name: $s36reflect_Enum_MultiPayload_degenerate14FooEmptyStructO +// CHECK-64: Demangled name: reflect_Enum_MultiPayload_degenerate.FooEmptyStruct + +// CHECK-64: Enum value: +// CHECK-64: (enum_value name=a index=0 +// CHECK-64: (bound_generic_struct Swift.Array +// CHECK-64: (struct Swift.Int)) +// CHECK-64: ) + +reflect(enum: FooEmptyStruct.b(B())) + +// CHECK-64: Reflecting an enum. +// CHECK-64: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}} +// CHECK-64: Type reference: +// CHECK-64: (enum reflect_Enum_MultiPayload_degenerate.FooEmptyStruct) + +// CHECK-64: Type info: +// CHECK-64: (multi_payload_enum size=8 alignment=8 stride=8 num_extra_inhabitants=2147483646 bitwise_takable=1 +// CHECK-64: (case name=a index=0 offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=_buffer offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=_storage offset=0 +// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1 +// CHECK-64: (field name=rawValue offset=0 +// CHECK-64: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1)))))))) +// CHECK-64: (case name=b index=1 offset=0 +// CHECK-64: (struct size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))) +// CHECK-64: Mangled name: $s36reflect_Enum_MultiPayload_degenerate14FooEmptyStructO +// CHECK-64: Demangled name: reflect_Enum_MultiPayload_degenerate.FooEmptyStruct + +// CHECK-64: Enum value: +// CHECK-64: (enum_value name=b index=1 +// CHECK-64: (struct reflect_Enum_MultiPayload_degenerate.B) +// CHECK-64: ) + + + +// TODO: Variations of Foo where `b` payload is class, Bool + +doneReflecting() + +// CHECK-32: FAIL + +// CHECK: Done. diff --git a/validation-test/Reflection/reflect_Enum_SingleCaseNoPayload.swift b/validation-test/Reflection/reflect_Enum_SingleCaseNoPayload.swift index 49d49de257ae2..3111021a788e3 100644 --- a/validation-test/Reflection/reflect_Enum_SingleCaseNoPayload.swift +++ b/validation-test/Reflection/reflect_Enum_SingleCaseNoPayload.swift @@ -52,10 +52,13 @@ reflect(object: ClassWithSingleCaseNoPayloadEnum()) // CHECK-64: (case name=default index=0))) // CHECK-64: (case name=none index=1))) // CHECK-64: (field name=e4 offset=18 -// CHECK-64: (single_payload_enum size=1 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1 +// CHECK-64: (single_payload_enum size=2 alignment=1 stride=2 num_extra_inhabitants=0 bitwise_takable=1 // CHECK-64: (case name=some index=0 offset=0 -// CHECK-64: (no_payload_enum size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1 -// CHECK-64: (case name=default index=0))) +// CHECK-64: (single_payload_enum size=1 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1 +// CHECK-64: (case name=some index=0 offset=0 +// CHECK-64: (no_payload_enum size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1 +// CHECK-64: (case name=default index=0))) +// CHECK-64: (case name=none index=1))) // CHECK-64: (case name=none index=1))) // CHECK-64: (field name=marker offset=24 // CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=0 bitwise_takable=1 From 283c65669f71fba27b1570d0418ea14d94b5fecb Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Thu, 5 Jan 2023 18:28:06 -0800 Subject: [PATCH 2/3] Match the earlier behavior when we lack typeinfo for a particular payload. This should fix a test crash that occurred on x86_64 with CF payloads. --- stdlib/public/RemoteInspection/TypeLowering.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/stdlib/public/RemoteInspection/TypeLowering.cpp b/stdlib/public/RemoteInspection/TypeLowering.cpp index 204fe9b2a77cc..f8e7a705eb9ff 100644 --- a/stdlib/public/RemoteInspection/TypeLowering.cpp +++ b/stdlib/public/RemoteInspection/TypeLowering.cpp @@ -2051,7 +2051,13 @@ class EnumTypeInfoBuilder { auto *CaseTR = getCaseTypeRef(Case); assert(CaseTR != nullptr); auto *CaseTI = TC.getTypeInfo(CaseTR, ExternalTypeInfo); - if (CaseTI->getSize() == 0) { + if (CaseTI == nullptr) { + // We don't have typeinfo; assume it's not + // zero-sized to match earlier behavior. + // TODO: Maybe this should prompt us to fall + // back to UnsupportedEnumTypeInfo?? + PayloadCases.push_back(Case); + } else if (CaseTI->getSize() == 0) { // Zero-sized payloads get special treatment ++EmptyPayloadCases; } else { From 7c7ad18aa2e08b442cdce9ea7b774ccd38fc2cb2 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Fri, 6 Jan 2023 14:56:53 -0800 Subject: [PATCH 3/3] Consistently treat payloads with missing/invalid typeinfo as non-empty. --- include/swift/RemoteInspection/TypeLowering.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/swift/RemoteInspection/TypeLowering.h b/include/swift/RemoteInspection/TypeLowering.h index 1cdad3be5d93b..416356f512ff7 100644 --- a/include/swift/RemoteInspection/TypeLowering.h +++ b/include/swift/RemoteInspection/TypeLowering.h @@ -243,8 +243,11 @@ class EnumTypeInfo : public TypeInfo { auto Cases = getCases(); return std::count_if(Cases.begin(), Cases.end(), [](const FieldInfo &Case){ + // For our purposes here, assume any case + // with invalid (missing) typeinfo is non-empty return Case.TR != 0 - && Case.TI.getSize() != 0; + && (Case.TI.getKind() == TypeInfoKind::Invalid + || Case.TI.getSize() > 0); }); } // Size of the payload area.