Skip to content

Commit a105cf5

Browse files
committed
Fix multi-payload enums with Class Existential payloads on 32-bit targets
Class existentials expose spare bits from all of the pointers, not just the first one. Due to a bad bug here, we were properly exposing spare bits from the first pointer, but then claiming that all bits of subsequent pointers were spare. This accidentally resulted in the correct operation on 64-bit targets (it picked the highest-order spare bit, which happened to be spare in both the broken mask and the correct mask). But on 32-bit targets, this exposed the high-order bits of pointers, which is incorrect. Expand the test a bit while we're here as well. Resolves rdar://132715829
1 parent d1a26d0 commit a105cf5

File tree

2 files changed

+94
-10
lines changed

2 files changed

+94
-10
lines changed

stdlib/public/RemoteInspection/TypeLowering.cpp

+6-4
Original file line numberDiff line numberDiff line change
@@ -438,13 +438,15 @@ BitMask RecordTypeInfo::getSpareBits(TypeConverter &TC, bool &hasAddrOnly) const
438438
break;
439439
}
440440
case RecordKind::ClassExistential:
441-
// Class existential is a data pointer that does expose spare bits
441+
// Class existential is a bunch of pointers that expose spare bits
442442
// ... so we can fall through ...
443443
case RecordKind::ExistentialMetatype: {
444-
// Initial metadata pointer has spare bits
444+
// A bunch of pointers that expose spare bits
445445
auto mpePointerSpareBits = TC.getBuilder().getMultiPayloadEnumPointerMask();
446-
mask.andMask(mpePointerSpareBits, 0);
447-
mask.keepOnlyLeastSignificantBytes(TC.targetPointerSize());
446+
auto mpePointerSpareBitMask = BitMask(TC.targetPointerSize(), mpePointerSpareBits);
447+
for (int offset = 0; offset < (int)getSize(); offset += TC.targetPointerSize()) {
448+
mask.andMask(mpePointerSpareBitMask, offset);
449+
}
448450
return mask;
449451
}
450452
case RecordKind::ErrorExistential:

validation-test/Reflection/reflect_Enum_values10.swift

+88-6
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,6 @@
1010
// UNSUPPORTED: use_os_stdlib
1111
// UNSUPPORTED: asan
1212

13-
// This is broken on ARM64_32, disable it temporarily until we can fix it. rdar://137351613
14-
// UNSUPPORTED: CPU=arm64_32
15-
1613
import SwiftReflectionTest
1714

1815
protocol P : AnyObject {
@@ -24,7 +21,8 @@ class C : P {
2421
init() { a = 0; b = 0; }
2522
}
2623

27-
// MemoryLayout<B>.size == 8
24+
// On 64-bit: MemoryLayout<B>.size == 8
25+
// On 32-bit: MemoryLayout<B>.size == 4
2826
enum B {
2927
case a(C)
3028
case b(C)
@@ -44,8 +42,8 @@ reflect(enumValue: B.b(C()))
4442
// CHECKALL-NEXT: (enum reflect_Enum_values10.B)
4543
// CHECKALL-NEXT: Value: .b(_)
4644

47-
// MemoryLayout<Q>.size == 16
48-
// MemoryLayout<P>.size == 16
45+
// On 64-bit: MemoryLayout<Q>.size == MemoryLayout<P>.size == 16
46+
// On 32-bit: MemoryLayout<Q>.size == MemoryLayout<P>.size == 8
4947
enum Q {
5048
case a(P)
5149
case b(P)
@@ -65,6 +63,90 @@ reflect(enumValue: Q.b(C()))
6563
// CHECKALL-NEXT: (enum reflect_Enum_values10.Q)
6664
// CHECKALL-NEXT: Value: .b(_)
6765

66+
enum B1 {
67+
case a(C)
68+
case b
69+
}
70+
71+
reflect(enumValue: B1.a(C()))
72+
73+
// CHECKALL: Reflecting an enum value.
74+
// CHECKALL-NEXT: Type reference:
75+
// CHECKALL-NEXT: (enum reflect_Enum_values10.B1)
76+
// CHECKALL-NEXT: Value: .a(_)
77+
78+
reflect(enumValue: B1.b)
79+
80+
// CHECKALL: Reflecting an enum value.
81+
// CHECKALL-NEXT: Type reference:
82+
// CHECKALL-NEXT: (enum reflect_Enum_values10.B1)
83+
// CHECKALL-NEXT: Value: .b
84+
85+
enum Q1 {
86+
case a(P)
87+
case b
88+
}
89+
90+
reflect(enumValue: Q1.a(C()))
91+
92+
// CHECKALL: Reflecting an enum value.
93+
// CHECKALL-NEXT: Type reference:
94+
// CHECKALL-NEXT: (enum reflect_Enum_values10.Q1)
95+
// CHECKALL-NEXT: Value: .a(_)
96+
97+
reflect(enumValue: Q1.b)
98+
99+
// CHECKALL: Reflecting an enum value.
100+
// CHECKALL-NEXT: Type reference:
101+
// CHECKALL-NEXT: (enum reflect_Enum_values10.Q1)
102+
// CHECKALL-NEXT: Value: .b
103+
104+
enum B2 {
105+
case a(C)
106+
case b(C)
107+
case c(C)
108+
case d(C)
109+
case e(C)
110+
}
111+
112+
reflect(enumValue: B2.a(C()))
113+
114+
// CHECKALL: Reflecting an enum value.
115+
// CHECKALL-NEXT: Type reference:
116+
// CHECKALL-NEXT: (enum reflect_Enum_values10.B2)
117+
// CHECKALL-NEXT: Value: .a(_)
118+
119+
reflect(enumValue: B2.e(C()))
120+
121+
// CHECKALL: Reflecting an enum value.
122+
// CHECKALL-NEXT: Type reference:
123+
// CHECKALL-NEXT: (enum reflect_Enum_values10.B2)
124+
// CHECKALL-NEXT: Value: .e(_)
125+
126+
// On 64-bit: MemoryLayout<Q>.size == MemoryLayout<P>.size == 16
127+
// On 32-bit: MemoryLayout<Q>.size == MemoryLayout<P>.size == 8
128+
enum Q2 {
129+
case a(P)
130+
case b(P)
131+
case c(P)
132+
case d(P)
133+
case e(P)
134+
}
135+
136+
reflect(enumValue: Q2.a(C()))
137+
138+
// CHECKALL: Reflecting an enum value.
139+
// CHECKALL-NEXT: Type reference:
140+
// CHECKALL-NEXT: (enum reflect_Enum_values10.Q2)
141+
// CHECKALL-NEXT: Value: .a(_)
142+
143+
reflect(enumValue: Q2.e(C()))
144+
145+
// CHECKALL: Reflecting an enum value.
146+
// CHECKALL-NEXT: Type reference:
147+
// CHECKALL-NEXT: (enum reflect_Enum_values10.Q2)
148+
// CHECKALL-NEXT: Value: .e(_)
149+
68150
doneReflecting()
69151

70152
// CHECKALL: Done.

0 commit comments

Comments
 (0)