Skip to content

Commit 3a2ea08

Browse files
committed
[Runtime] Fix race condition in protocol conformance lookups that caused false negatives.
In the uncached case, we'd scan conformances, cache them, then re-query the cache. This worked fine when the cache always grew, but now we clear the cache when loading new Swift images into the process. If that happens between the scan and the re-query, we lose the entry and return a false negative. Instead, track what we've found in the scan in a separate local table, then query that after completing the scan. While we're in there, fix a bug in TypeLookupError where operator= accidentally copied this->Context instead of other.Context. This caused the runtime to crash when trying to print error messages due to the false negative. Add a no-parameter constructor to TypeLookupErrorOr<> to distinguish the case where it's being initialized with nothing from the case where it's being initialized with nullptr.
1 parent e2563e6 commit 3a2ea08

File tree

3 files changed

+22
-8
lines changed

3 files changed

+22
-8
lines changed

include/swift/Demangling/TypeLookupError.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ class TypeLookupError {
110110

111111
Fn(Context, Command::DestroyContext, nullptr);
112112
Fn = other.Fn;
113-
Context = Fn(Context, Command::CopyContext, nullptr);
113+
Context = Fn(other.Context, Command::CopyContext, nullptr);
114114

115115
return *this;
116116
}
@@ -169,6 +169,8 @@ template <typename T> class TypeLookupErrorOr {
169169
TaggedUnion<T, TypeLookupError> Value;
170170

171171
public:
172+
TypeLookupErrorOr() : Value(TypeLookupError("freshly constructed error")) {}
173+
172174
TypeLookupErrorOr(const T &t) : Value(t) {
173175
if (!t)
174176
Value = TypeLookupError("unknown error");

stdlib/public/runtime/Metadata.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5006,7 +5006,7 @@ swift_getAssociatedTypeWitnessSlowImpl(
50065006
Demangle::makeSymbolicMangledNameStringRef(mangledNameBase);
50075007

50085008
// Demangle the associated type.
5009-
TypeLookupErrorOr<TypeInfo> result = TypeInfo();
5009+
TypeLookupErrorOr<TypeInfo> result;
50105010
if (inProtocolContext) {
50115011
// The protocol's Self is the only generic parameter that can occur in the
50125012
// type.

stdlib/public/runtime/ProtocolConformance.cpp

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "swift/Runtime/HeapObject.h"
2323
#include "swift/Runtime/Metadata.h"
2424
#include "swift/Basic/Unreachable.h"
25+
#include "llvm/ADT/DenseMap.h"
2526
#include "CompatibilityOverride.h"
2627
#include "ImageInspection.h"
2728
#include "Private.h"
@@ -470,6 +471,7 @@ swift_conformsToProtocolImpl(const Metadata *const type,
470471
return found.second;
471472

472473
// Scan conformance records.
474+
llvm::SmallDenseMap<const Metadata *, const WitnessTable *> foundWitnesses;
473475
auto processSection = [&](const ConformanceSection &section) {
474476
// Eagerly pull records for nondependent witnesses into our cache.
475477
auto processDescriptor = [&](const ProtocolConformanceDescriptor &descriptor) {
@@ -484,6 +486,7 @@ swift_conformsToProtocolImpl(const Metadata *const type,
484486
if (auto *matchingType = candidate.getMatchingType(type)) {
485487
auto witness = descriptor.getWitnessTable(matchingType);
486488
C.cacheResult(matchingType, protocol, witness, /*always cache*/ 0);
489+
foundWitnesses.insert({matchingType, witness});
487490
}
488491
};
489492

@@ -505,14 +508,23 @@ swift_conformsToProtocolImpl(const Metadata *const type,
505508
processSection(section);
506509
}
507510

508-
// Try the search again to look for the most specific cached conformance.
509-
found = searchInConformanceCache(type, protocol);
511+
// Find the most specific conformance that was scanned.
512+
const WitnessTable *foundWitness = nullptr;
513+
const Metadata *searchType = type;
514+
while (!foundWitness && searchType) {
515+
foundWitness = foundWitnesses.lookup(searchType);
510516

511-
// If it's not authoritative, then add an authoritative entry for this type.
512-
if (!found.first)
513-
C.cacheResult(type, protocol, found.second, snapshot.count());
517+
// If there's no entry here, move up to the superclass (if any).
518+
if (!foundWitness)
519+
searchType = _swift_class_getSuperclass(searchType);
520+
}
521+
522+
// If it's for a superclass or if we didn't find anything, then add an
523+
// authoritative entry for this type.
524+
if (searchType != type)
525+
C.cacheResult(type, protocol, foundWitness, snapshot.count());
514526

515-
return found.second;
527+
return foundWitness;
516528
}
517529

518530
const ContextDescriptor *

0 commit comments

Comments
 (0)