Skip to content

Commit f17baef

Browse files
committed
IRGen: Fix miscompile when a generic parameter is fixed to a tuple containing a pack
If you had something like: struct G<T> { func f<each U>(_: repeat each U) where T == (repeat each U) {} } We would fulfill 'each U' from the metadata for 'G<(repeat each U)>', by taking apart the tuple metadata for `(repeat each U)` and forming a pack. However this code path was only intended to kick in for a tuple conformance witness thunk. In the general case, this optimization is not correct, because if 'each U' is substituted with a one-element pack, the generic argument of `G<(repeat each U)>` is just that one element's metadata, and not a tuple. In fact, we cannot distinguish the one-element tuple case, because the wrapped element may itself be a tuple. The fix is to just split off FulfillmentMap::searchTupleTypeMetadata() from searchTypeMetadata(), and only call the former when we're in the specific situation that requires it. - Fixes #78191. - Fixes rdar://problem/135325886.
1 parent be860a8 commit f17baef

File tree

6 files changed

+103
-35
lines changed

6 files changed

+103
-35
lines changed

lib/IRGen/Fulfillment.cpp

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -178,35 +178,44 @@ bool FulfillmentMap::searchTypeMetadata(IRGenModule &IGM, CanType type,
178178
source, std::move(path), keys);
179179
}
180180

181-
if (auto tupleType = dyn_cast<TupleType>(type)) {
182-
if (tupleType->getNumElements() == 1 &&
183-
isa<PackExpansionType>(tupleType.getElementType(0))) {
184-
185-
bool hadFulfillment = false;
186-
auto packType = tupleType.getInducedPackType();
187-
188-
{
189-
auto argPath = path;
190-
argPath.addTuplePackComponent();
191-
hadFulfillment |= searchTypeMetadataPack(IGM, packType,
192-
isExact, metadataState, source,
193-
std::move(argPath), keys);
194-
}
181+
// TODO: functions
182+
// TODO: metatypes
195183

196-
{
197-
auto argPath = path;
198-
argPath.addTupleShapeComponent();
199-
hadFulfillment |= searchShapeRequirement(IGM, packType, source,
200-
std::move(argPath));
184+
return false;
185+
}
201186

202-
}
187+
/// Metadata fulfillment in a tuple conformance witness thunks.
188+
///
189+
/// \return true if any fulfillments were added by this search.
190+
bool FulfillmentMap::searchTupleTypeMetadata(IRGenModule &IGM, CanTupleType tupleType,
191+
IsExact_t isExact,
192+
MetadataState metadataState,
193+
unsigned source, MetadataPath &&path,
194+
const InterestingKeysCallback &keys) {
195+
if (tupleType->getNumElements() == 1 &&
196+
isa<PackExpansionType>(tupleType.getElementType(0))) {
203197

204-
return hadFulfillment;
198+
bool hadFulfillment = false;
199+
auto packType = tupleType.getInducedPackType();
200+
201+
{
202+
auto argPath = path;
203+
argPath.addTuplePackComponent();
204+
hadFulfillment |= searchTypeMetadataPack(IGM, packType,
205+
isExact, metadataState, source,
206+
std::move(argPath), keys);
205207
}
206-
}
207208

208-
// TODO: functions
209-
// TODO: metatypes
209+
{
210+
auto argPath = path;
211+
argPath.addTupleShapeComponent();
212+
hadFulfillment |= searchShapeRequirement(IGM, packType, source,
213+
std::move(argPath));
214+
215+
}
216+
217+
return hadFulfillment;
218+
}
210219

211220
return false;
212221
}

lib/IRGen/Fulfillment.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,14 @@ class FulfillmentMap {
102102
unsigned sourceIndex, MetadataPath &&path,
103103
const InterestingKeysCallback &interestingKeys);
104104

105+
/// Metadata fulfillment in tuple conformance witness thunks.
106+
///
107+
/// \return true if any fulfillments were added by this search.
108+
bool searchTupleTypeMetadata(IRGenModule &IGM, CanTupleType type, IsExact_t isExact,
109+
MetadataState metadataState,
110+
unsigned sourceIndex, MetadataPath &&path,
111+
const InterestingKeysCallback &interestingKeys);
112+
105113
/// Search the given type metadata pack for useful fulfillments.
106114
///
107115
/// \return true if any fulfillments were added by this search.

lib/IRGen/GenProto.cpp

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@ class PolymorphicConvention {
163163
CanType type, Args... args);
164164
bool considerType(CanType type, IsExact_t isExact,
165165
unsigned sourceIndex, MetadataPath &&path);
166+
bool considerTupleType(CanTupleType type, IsExact_t isExact,
167+
unsigned sourceIndex, MetadataPath &&path);
166168

167169
/// Testify to generic parameters in the Self type of a protocol
168170
/// witness method.
@@ -354,6 +356,15 @@ bool PolymorphicConvention::considerType(CanType type, IsExact_t isExact,
354356
std::move(path), callbacks);
355357
}
356358

359+
bool PolymorphicConvention::considerTupleType(CanTupleType type, IsExact_t isExact,
360+
unsigned sourceIndex,
361+
MetadataPath &&path) {
362+
FulfillmentMapCallback callbacks(*this);
363+
return Fulfillments.searchTupleTypeMetadata(IGM, type, isExact,
364+
MetadataState::Complete, sourceIndex,
365+
std::move(path), callbacks);
366+
}
367+
357368
void PolymorphicConvention::considerWitnessSelf(CanSILFunctionType fnType) {
358369
CanType selfTy = fnType->getSelfInstanceType(
359370
IGM.getSILModule(), IGM.getMaximalTypeExpansionContext());
@@ -362,13 +373,17 @@ void PolymorphicConvention::considerWitnessSelf(CanSILFunctionType fnType) {
362373
// First, bind type metadata for Self.
363374
Sources.emplace_back(MetadataSource::Kind::SelfMetadata, selfTy);
364375

365-
if (selfTy->is<GenericTypeParamType>()) {
366-
// The Self type is abstract, so we can fulfill its metadata from
367-
// the Self metadata parameter.
368-
addSelfMetadataFulfillment(selfTy);
369-
}
376+
if (auto tupleTy = dyn_cast<TupleType>(selfTy)) {
377+
considerTupleType(tupleTy, IsInexact, Sources.size() - 1, MetadataPath());
378+
} else {
379+
if (isa<GenericTypeParamType>(selfTy)) {
380+
// The Self type is abstract, so we can fulfill its metadata from
381+
// the Self metadata parameter.
382+
addSelfMetadataFulfillment(selfTy);
383+
}
370384

371-
considerType(selfTy, IsInexact, Sources.size() - 1, MetadataPath());
385+
considerType(selfTy, IsInexact, Sources.size() - 1, MetadataPath());
386+
}
372387

373388
// The witness table for the Self : P conformance can be
374389
// fulfilled from the Self witness table parameter.

lib/IRGen/LocalTypeData.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -735,11 +735,20 @@ void LocalTypeDataCache::addAbstractForTypeMetadata(IRGenFunction &IGF,
735735
// Look for anything at all that's fulfilled by this. If we don't find
736736
// anything, stop.
737737
FulfillmentMap fulfillments;
738-
if (!fulfillments.searchTypeMetadata(IGF.IGM, type, isExact,
739-
metadata.getStaticLowerBoundOnState(),
740-
/*source*/ 0, MetadataPath(),
741-
callbacks)) {
742-
return;
738+
if (auto tupleType = dyn_cast<TupleType>(type)) {
739+
if (!fulfillments.searchTupleTypeMetadata(IGF.IGM, tupleType, isExact,
740+
metadata.getStaticLowerBoundOnState(),
741+
/*source*/ 0, MetadataPath(),
742+
callbacks)) {
743+
return;
744+
}
745+
} else {
746+
if (!fulfillments.searchTypeMetadata(IGF.IGM, type, isExact,
747+
metadata.getStaticLowerBoundOnState(),
748+
/*source*/ 0, MetadataPath(),
749+
callbacks)) {
750+
return;
751+
}
743752
}
744753

745754
addAbstractForFulfillments(IGF, std::move(fulfillments),
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// RUN: %target-swift-frontend -emit-ir %s -target %target-swift-5.9-abi-triple | %FileCheck %s
2+
3+
struct G<T> {
4+
var t: T
5+
6+
// Make sure we *don't* try to fulfill the pack from the tuple, because we cannot
7+
// distinguish the one-element case that way.
8+
9+
// CHECK-LABEL: define {{.*}} void @"$s34variadic_generic_fulfillment_tuple1GV20fixOuterGenericParamyACyqd__qd__Qp_tGqd__qd__Qp_t_tRvd__qd__qd__Qp_tRszlF"(ptr noalias sret(%swift.opaque) %0, ptr noalias %1, {{i32|i64}} %2, ptr %"each U", ptr noalias swiftself %3)
10+
func fixOuterGenericParam<each U>(_ t: T) -> G<T> where T == (repeat each U) {
11+
return G<T>(t: t)
12+
}
13+
}

test/Interpreter/variadic_generic_tuples.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,5 +85,19 @@ tuples.test("labels") {
8585
expectEqual("(label: Swift.Int, Swift.String)", _typeName(oneElementLabeledTuple(t: String.self)))
8686
}
8787

88+
// https://github.com/swiftlang/swift/issues/78191
89+
tuples.test("fulfillment") {
90+
struct S<T> {
91+
let t: T
92+
93+
func f<each A, each B>(_ b: S<(repeat each B)>) -> S<(repeat each A, repeat each B)>
94+
where T == (repeat each A) {
95+
return S<(repeat each A, repeat each B)>(t: (repeat each t, repeat each b.t))
96+
}
97+
}
98+
99+
let s = S(t: "hello")
100+
expectEqual("S<(String, Int, Int)>(t: (\"hello\", 1, 2))", String(describing: s.f(S(t: (1, 2)))))
101+
}
88102

89103
runAllTests()

0 commit comments

Comments
 (0)