Skip to content

Commit 7b791cf

Browse files
authored
Merge pull request #38515 from mikeash/fix-protocol-conformance-superclass-recursion
[Runtime] Fix infinite recursion in protocol conformance checking on class Sub: Super<Sub>.
2 parents 8180c26 + b37252d commit 7b791cf

File tree

1 file changed

+119
-47
lines changed

1 file changed

+119
-47
lines changed

stdlib/public/runtime/ProtocolConformance.cpp

Lines changed: 119 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -157,14 +157,20 @@ tryGetCompleteMetadataNonblocking(const Metadata *metadata) {
157157

158158
/// Get the superclass of metadata, which may be incomplete. When the metadata
159159
/// is not sufficiently complete, then we fall back to demangling the superclass
160-
/// in the nominal type descriptor, which is slow but works. Return NULL if the
161-
/// metadata is not a class.
160+
/// in the nominal type descriptor, which is slow but works. Return {NULL,
161+
/// MetadataState::Complete} if the metadata is not a class, or has no
162+
/// superclass.
162163
///
163164
/// If the metadata's current state is known, it may be passed in as
164165
/// knownMetadataState. This saves the cost of retrieving that info separately.
166+
///
167+
/// When instantiateSuperclassMetadata is true, this function will instantiate
168+
/// superclass metadata when necessary. When false, this will return {NULL,
169+
/// MetadataState::Abstract} to indicate that there's an uninstantiated
170+
/// superclass that was not returned.
165171
static MetadataResponse getSuperclassForMaybeIncompleteMetadata(
166-
const Metadata *metadata,
167-
llvm::Optional<MetadataState> knownMetadataState) {
172+
const Metadata *metadata, llvm::Optional<MetadataState> knownMetadataState,
173+
bool instantiateSuperclassMetadata) {
168174
const ClassMetadata *classMetadata = dyn_cast<ClassMetadata>(metadata);
169175
if (!classMetadata)
170176
return {_swift_class_getSuperclass(metadata), MetadataState::Complete};
@@ -192,36 +198,43 @@ static MetadataResponse getSuperclassForMaybeIncompleteMetadata(
192198
// The subclass metadata is complete. Fetch and return the superclass.
193199
auto *superMetadata = getMetadataForClass(classMetadata->Superclass);
194200
return {superMetadata, MetadataState::Complete};
195-
}
196-
if (metadataState == MetadataState::NonTransitiveComplete) {
201+
} else if (metadataState == MetadataState::NonTransitiveComplete) {
197202
// The subclass metadata is complete, but, unlike above, not transitively.
198203
// Its Superclass field is valid, so just read that field to get to the
199204
// superclass to proceed to the next step.
200205
auto *superMetadata = getMetadataForClass(classMetadata->Superclass);
201206
auto superState = tryGetCompleteMetadataNonblocking(superMetadata);
202207
return {superMetadata, superState};
203-
} else {
208+
} else if (instantiateSuperclassMetadata) {
204209
// The subclass metadata is either LayoutComplete or Abstract, so the
205210
// Superclass field is not valid. To get to the superclass, make the
206211
// expensive call to getSuperclassMetadata which demangles the superclass
207212
// name from the nominal type descriptor to get the metadata for the
208213
// superclass.
209-
MetadataRequest request(MetadataState::Complete,
214+
MetadataRequest request(MetadataState::Abstract,
210215
/*non-blocking*/ true);
211216
return getSuperclassMetadata(request, classMetadata);
217+
} else {
218+
// The Superclass field is not valid and the caller did not request
219+
// instantiation. Return a NULL superclass and Abstract to indicate that a
220+
// superclass exists but is not yet instantiated.
221+
return {nullptr, MetadataState::Abstract};
212222
}
213223
}
214224

215-
class MaybeIncompleteSuperclassIterator {
225+
struct MaybeIncompleteSuperclassIterator {
216226
const Metadata *metadata;
217227
llvm::Optional<MetadataState> state;
228+
bool instantiateSuperclassMetadata;
218229

219-
public:
220-
MaybeIncompleteSuperclassIterator(const Metadata *metadata)
221-
: metadata(metadata), state(llvm::None) {}
230+
MaybeIncompleteSuperclassIterator(const Metadata *metadata,
231+
bool instantiateSuperclassMetadata)
232+
: metadata(metadata), state(llvm::None),
233+
instantiateSuperclassMetadata(instantiateSuperclassMetadata) {}
222234

223235
MaybeIncompleteSuperclassIterator &operator++() {
224-
auto response = getSuperclassForMaybeIncompleteMetadata(metadata, state);
236+
auto response = getSuperclassForMaybeIncompleteMetadata(
237+
metadata, state, instantiateSuperclassMetadata);
225238
metadata = response.Value;
226239
state = response.State;
227240
return *this;
@@ -238,10 +251,12 @@ class MaybeIncompleteSuperclassIterator {
238251
/// superclasses in order. If the metadata is not a class, iteration will
239252
/// provide that metadata and then stop.
240253
iterator_range<MaybeIncompleteSuperclassIterator>
241-
iterateMaybeIncompleteSuperclasses(const Metadata *metadata) {
254+
iterateMaybeIncompleteSuperclasses(const Metadata *metadata,
255+
bool instantiateSuperclassMetadata) {
242256
return iterator_range<MaybeIncompleteSuperclassIterator>(
243-
MaybeIncompleteSuperclassIterator(metadata),
244-
MaybeIncompleteSuperclassIterator(nullptr));
257+
MaybeIncompleteSuperclassIterator(metadata,
258+
instantiateSuperclassMetadata),
259+
MaybeIncompleteSuperclassIterator(nullptr, false));
245260
}
246261

247262
/// Take the type reference inside a protocol conformance record and fetch the
@@ -562,12 +577,14 @@ swift::swift_registerProtocolConformances(const ProtocolConformanceRecord *begin
562577
/// A return value of `{ false, nullptr }` indicates nothing was cached.
563578
static std::pair<bool, const WitnessTable *>
564579
searchInConformanceCache(const Metadata *type,
565-
const ProtocolDescriptor *protocol) {
580+
const ProtocolDescriptor *protocol,
581+
bool instantiateSuperclassMetadata) {
566582
auto &C = Conformances.get();
567583
auto origType = type;
568584
auto snapshot = C.Cache.snapshot();
569585

570-
for (auto type : iterateMaybeIncompleteSuperclasses(type)) {
586+
for (auto type : iterateMaybeIncompleteSuperclasses(
587+
type, instantiateSuperclassMetadata)) {
571588
if (auto *Value = snapshot.find(ConformanceCacheKey(type, protocol))) {
572589
return {type == origType, Value->getWitnessTable()};
573590
}
@@ -655,9 +672,10 @@ namespace {
655672
/// Retrieve the type that matches the conformance candidate, which may
656673
/// be a superclass of the given type. Returns null if this type does not
657674
/// match this conformance.
658-
const Metadata *getMatchingType(const Metadata *conformingType) const {
659-
for (auto conformingType :
660-
iterateMaybeIncompleteSuperclasses(conformingType)) {
675+
const Metadata *getMatchingType(const Metadata *conformingType,
676+
bool instantiateSuperclassMetadata) const {
677+
for (auto conformingType : iterateMaybeIncompleteSuperclasses(
678+
conformingType, instantiateSuperclassMetadata)) {
661679
if (matches(conformingType))
662680
return conformingType;
663681
}
@@ -671,7 +689,8 @@ static void validateSharedCacheResults(
671689
ConformanceState &C, const Metadata *type,
672690
const ProtocolDescriptor *protocol,
673691
const WitnessTable *dyldCachedWitnessTable,
674-
const ProtocolConformanceDescriptor *dyldCachedConformanceDescriptor) {
692+
const ProtocolConformanceDescriptor *dyldCachedConformanceDescriptor,
693+
bool instantiateSuperclassMetadata) {
675694
#if USE_DYLD_SHARED_CACHE_CONFORMANCE_TABLES
676695
if (!C.sharedCacheOptimizationsActive() || !C.validateSharedCacheResults)
677696
return;
@@ -684,7 +703,7 @@ static void validateSharedCacheResults(
684703
continue;
685704

686705
ConformanceCandidate candidate(descriptor);
687-
if (candidate.getMatchingType(type))
706+
if (candidate.getMatchingType(type, instantiateSuperclassMetadata))
688707
conformances.push_back(&descriptor);
689708
}
690709
}
@@ -733,7 +752,8 @@ static void validateSharedCacheResults(
733752
static std::tuple<const WitnessTable *, const ProtocolConformanceDescriptor *,
734753
bool>
735754
findSharedCacheConformance(ConformanceState &C, const Metadata *type,
736-
const ProtocolDescriptor *protocol) {
755+
const ProtocolDescriptor *protocol,
756+
bool instantiateSuperclassMetadata) {
737757
#if USE_DYLD_SHARED_CACHE_CONFORMANCE_TABLES
738758
const ContextDescriptor *description;
739759
llvm::StringRef foreignTypeIdentity;
@@ -767,7 +787,8 @@ findSharedCacheConformance(ConformanceState &C, const Metadata *type,
767787
dyldResult.value);
768788

769789
assert(conformanceDescriptor->getProtocol() == protocol);
770-
assert(ConformanceCandidate{*conformanceDescriptor}.getMatchingType(type));
790+
assert(ConformanceCandidate{*conformanceDescriptor}.getMatchingType(
791+
type, instantiateSuperclassMetadata));
771792

772793
if (conformanceDescriptor->getGenericWitnessTable()) {
773794
SHARED_CACHE_LOG(
@@ -818,9 +839,15 @@ findSharedCacheConformance(ConformanceState &C, const Metadata *type,
818839
#endif
819840
}
820841

821-
static const WitnessTable *
822-
swift_conformsToProtocolImpl(const Metadata *const type,
823-
const ProtocolDescriptor *protocol) {
842+
/// Check if a type conforms to a protocol, possibly instantiating superclasses
843+
/// that have not yet been instantiated. The return value is a pair consisting
844+
/// of the witness table for the conformance (or NULL if no conformance was
845+
/// found), and a boolean indicating whether there are uninstantiated
846+
/// superclasses that were not searched.
847+
static std::pair<const WitnessTable *, bool>
848+
swift_conformsToProtocolMaybeInstantiateSuperclasses(
849+
const Metadata *const type, const ProtocolDescriptor *protocol,
850+
bool instantiateSuperclassMetadata) {
824851
auto &C = Conformances.get();
825852

826853
const WitnessTable *dyldCachedWitnessTable = nullptr;
@@ -830,51 +857,56 @@ swift_conformsToProtocolImpl(const Metadata *const type,
830857
// Search the shared cache tables for a conformance for this type, and for
831858
// superclasses (if it's a class).
832859
if (C.sharedCacheOptimizationsActive()) {
833-
for (auto dyldSearchType : iterateMaybeIncompleteSuperclasses(type)) {
860+
for (auto dyldSearchType : iterateMaybeIncompleteSuperclasses(
861+
type, instantiateSuperclassMetadata)) {
834862
bool definitiveFailure;
835863
std::tie(dyldCachedWitnessTable, dyldCachedConformanceDescriptor,
836864
definitiveFailure) =
837-
findSharedCacheConformance(C, dyldSearchType, protocol);
865+
findSharedCacheConformance(C, dyldSearchType, protocol,
866+
instantiateSuperclassMetadata);
838867

839868
if (definitiveFailure)
840-
return nullptr;
869+
return {nullptr, false};
841870

842871
if (dyldCachedWitnessTable || dyldCachedConformanceDescriptor)
843872
break;
844873
}
845874

846875
validateSharedCacheResults(C, type, protocol, dyldCachedWitnessTable,
847-
dyldCachedConformanceDescriptor);
876+
dyldCachedConformanceDescriptor,
877+
instantiateSuperclassMetadata);
848878
// Return a cached result if we got a witness table. We can't do this if
849879
// scanSectionsBackwards is set, since a scanned conformance can override a
850880
// cached result in that case.
851881
if (!C.scanSectionsBackwards)
852882
if (dyldCachedWitnessTable)
853-
return dyldCachedWitnessTable;
883+
return {dyldCachedWitnessTable, false};
854884
}
855885

856886
// See if we have an authoritative cached conformance. The
857887
// ConcurrentReadableHashMap data structure allows us to search the map
858888
// concurrently without locking.
859-
auto found = searchInConformanceCache(type, protocol);
889+
auto found =
890+
searchInConformanceCache(type, protocol, instantiateSuperclassMetadata);
860891
if (found.first) {
861892
// An authoritative negative result can be overridden by a result from dyld.
862893
if (!found.second) {
863894
if (dyldCachedWitnessTable)
864-
return dyldCachedWitnessTable;
895+
return {dyldCachedWitnessTable, false};
865896
}
866-
return found.second;
897+
return {found.second, false};
867898
}
868899

869900
if (dyldCachedConformanceDescriptor) {
870901
ConformanceCandidate candidate(*dyldCachedConformanceDescriptor);
871-
auto *matchingType = candidate.getMatchingType(type);
902+
auto *matchingType =
903+
candidate.getMatchingType(type, instantiateSuperclassMetadata);
872904
assert(matchingType);
873905
auto witness = dyldCachedConformanceDescriptor->getWitnessTable(matchingType);
874906
C.cacheResult(type, protocol, witness, /*always cache*/ 0);
875907
SHARED_CACHE_LOG("Caching generic conformance to %s found in shared cache",
876908
protocol->Name.get());
877-
return witness;
909+
return {witness, false};
878910
}
879911

880912
// Scan conformance records.
@@ -890,7 +922,8 @@ swift_conformsToProtocolImpl(const Metadata *const type,
890922
// The matching type is exact, so they can't go stale, and we should
891923
// always cache them.
892924
ConformanceCandidate candidate(descriptor);
893-
if (auto *matchingType = candidate.getMatchingType(type)) {
925+
if (auto *matchingType =
926+
candidate.getMatchingType(type, instantiateSuperclassMetadata)) {
894927
auto witness = descriptor.getWitnessTable(matchingType);
895928
C.cacheResult(matchingType, protocol, witness, /*always cache*/ 0);
896929
foundWitnesses.insert({matchingType, witness});
@@ -918,7 +951,13 @@ swift_conformsToProtocolImpl(const Metadata *const type,
918951
// Find the most specific conformance that was scanned.
919952
const WitnessTable *foundWitness = nullptr;
920953
const Metadata *foundType = nullptr;
921-
for (auto searchType : iterateMaybeIncompleteSuperclasses(type)) {
954+
955+
// Use MaybeIncompleteSuperclassIterator directly so we can examine its final
956+
// state. Complete indicates that we finished normally, Abstract indicates
957+
// that there's an uninstantiated superclass we didn't iterate over.
958+
MaybeIncompleteSuperclassIterator superclassIterator{
959+
type, instantiateSuperclassMetadata};
960+
for (; auto searchType = superclassIterator.metadata; ++superclassIterator) {
922961
const WitnessTable *witness = foundWitnesses.lookup(searchType);
923962
if (witness) {
924963
if (!foundType) {
@@ -936,17 +975,49 @@ swift_conformsToProtocolImpl(const Metadata *const type,
936975
}
937976
}
938977

978+
// Do not cache negative results if there were uninstantiated superclasses we
979+
// didn't search. They might have a conformance that will be found later.
980+
bool hasUninstantiatedSuperclass =
981+
superclassIterator.state == MetadataState::Abstract;
982+
939983
// If it's for a superclass or if we didn't find anything, then add an
940984
// authoritative entry for this type.
941985
if (foundType != type)
942-
C.cacheResult(type, protocol, foundWitness, snapshot.count());
986+
if (foundWitness || !hasUninstantiatedSuperclass)
987+
C.cacheResult(type, protocol, foundWitness, snapshot.count());
943988

944989
// A negative result can be overridden by a result from dyld.
945-
if (foundWitness) {
990+
if (!foundWitness) {
946991
if (dyldCachedWitnessTable)
947-
return dyldCachedWitnessTable;
992+
return {dyldCachedWitnessTable, false};
948993
}
949-
return foundWitness;
994+
return {foundWitness, hasUninstantiatedSuperclass};
995+
}
996+
997+
static const WitnessTable *
998+
swift_conformsToProtocolImpl(const Metadata *const type,
999+
const ProtocolDescriptor *protocol) {
1000+
const WitnessTable *table;
1001+
bool hasUninstantiatedSuperclass;
1002+
1003+
// First, try without instantiating any new superclasses. This avoids
1004+
// an infinite loop for cases like `class Sub: Super<Sub>`. In cases like
1005+
// that, the conformance must exist on the subclass (or at least somewhere
1006+
// in the chain before we get to an uninstantiated superclass) so this search
1007+
// will succeed without trying to instantiate Super while it's already being
1008+
// instantiated.=
1009+
std::tie(table, hasUninstantiatedSuperclass) =
1010+
swift_conformsToProtocolMaybeInstantiateSuperclasses(
1011+
type, protocol, false /*instantiateSuperclassMetadata*/);
1012+
1013+
// If no conformance was found, and there is an uninstantiated superclass that
1014+
// was not searched, then try the search again and instantiate all
1015+
// superclasses.
1016+
if (!table && hasUninstantiatedSuperclass)
1017+
std::tie(table, hasUninstantiatedSuperclass) =
1018+
swift_conformsToProtocolMaybeInstantiateSuperclasses(
1019+
type, protocol, true /*instantiateSuperclassMetadata*/);
1020+
return table;
9501021
}
9511022

9521023
const ContextDescriptor *
@@ -973,8 +1044,8 @@ bool isSwiftClassMetadataSubclass(const ClassMetadata *subclass,
9731044

9741045
llvm::Optional<MetadataState> subclassState = llvm::None;
9751046
while (true) {
976-
auto response =
977-
getSuperclassForMaybeIncompleteMetadata(subclass, subclassState);
1047+
auto response = getSuperclassForMaybeIncompleteMetadata(
1048+
subclass, subclassState, true /*instantiateSuperclassMetadata*/);
9781049
if (response.Value == superclass)
9791050
return true;
9801051
if (!response.Value)
@@ -1177,7 +1248,8 @@ const Metadata *swift::findConformingSuperclass(
11771248
// Figure out which type we're looking for.
11781249
ConformanceCandidate candidate(*conformance);
11791250

1180-
const Metadata *conformingType = candidate.getMatchingType(type);
1251+
const Metadata *conformingType =
1252+
candidate.getMatchingType(type, true /*instantiateSuperclassMetadata*/);
11811253
assert(conformingType);
11821254
return conformingType;
11831255
}

0 commit comments

Comments
 (0)