Skip to content

Commit 6200df1

Browse files
committed
Make the lazy assignment of an assoociated type/wtable atomic.
Should fix SR-12760.
1 parent 9a141a8 commit 6200df1

File tree

2 files changed

+80
-13
lines changed

2 files changed

+80
-13
lines changed

stdlib/public/runtime/Metadata.cpp

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4645,6 +4645,8 @@ static StringRef findAssociatedTypeName(const ProtocolDescriptor *protocol,
46454645
return StringRef();
46464646
}
46474647

4648+
using AssociatedTypeWitness = std::atomic<const Metadata *>;
4649+
46484650
SWIFT_CC(swift)
46494651
static MetadataResponse
46504652
swift_getAssociatedTypeWitnessSlowImpl(
@@ -4668,8 +4670,8 @@ swift_getAssociatedTypeWitnessSlowImpl(
46684670

46694671
// Retrieve the witness.
46704672
unsigned witnessIndex = assocType - reqBase;
4671-
auto *witnessAddr = &((const Metadata **)wtable)[witnessIndex];
4672-
auto witness = *witnessAddr;
4673+
auto *witnessAddr = &((AssociatedTypeWitness*)wtable)[witnessIndex];
4674+
auto witness = witnessAddr->load(std::memory_order_acquire);
46734675

46744676
#if SWIFT_PTRAUTH
46754677
uint16_t extraDiscriminator = assocType->Flags.getExtraDiscriminator();
@@ -4773,8 +4775,14 @@ swift_getAssociatedTypeWitnessSlowImpl(
47734775
if (response.State == MetadataState::Complete) {
47744776
// We pass type metadata around as unsigned pointers, but we sign them
47754777
// in witness tables, which doesn't provide all that much extra security.
4776-
initAssociatedTypeProtocolWitness(witnessAddr, assocTypeMetadata,
4777-
*assocType);
4778+
auto valueToStore = assocTypeMetadata;
4779+
#if SWIFT_PTRAUTH
4780+
valueToStore = ptrauth_sign_unauthenticated(valueToStore,
4781+
swift_ptrauth_key_associated_type,
4782+
ptrauth_blend_discriminator(witnessAddr,
4783+
extraDiscriminator));
4784+
#endif
4785+
witnessAddr->store(valueToStore, std::memory_order_release);
47784786
}
47794787

47804788
return response;
@@ -4791,8 +4799,8 @@ swift::swift_getAssociatedTypeWitness(MetadataRequest request,
47914799

47924800
// If the low bit of the witness is clear, it's already a metadata pointer.
47934801
unsigned witnessIndex = assocType - reqBase;
4794-
auto *witnessAddr = &((const void* *)wtable)[witnessIndex];
4795-
auto witness = *witnessAddr;
4802+
auto *witnessAddr = &((const AssociatedTypeWitness *)wtable)[witnessIndex];
4803+
auto witness = witnessAddr->load(std::memory_order_acquire);
47964804

47974805
#if SWIFT_PTRAUTH
47984806
uint16_t extraDiscriminator = assocType->Flags.getExtraDiscriminator();
@@ -4812,6 +4820,8 @@ swift::swift_getAssociatedTypeWitness(MetadataRequest request,
48124820
reqBase, assocType);
48134821
}
48144822

4823+
using AssociatedConformanceWitness = std::atomic<void *>;
4824+
48154825
SWIFT_CC(swift)
48164826
static const WitnessTable *swift_getAssociatedConformanceWitnessSlowImpl(
48174827
WitnessTable *wtable,
@@ -4837,8 +4847,8 @@ static const WitnessTable *swift_getAssociatedConformanceWitnessSlowImpl(
48374847

48384848
// Retrieve the witness.
48394849
unsigned witnessIndex = assocConformance - reqBase;
4840-
auto *witnessAddr = &((void**)wtable)[witnessIndex];
4841-
auto witness = *witnessAddr;
4850+
auto *witnessAddr = &((AssociatedConformanceWitness*)wtable)[witnessIndex];
4851+
auto witness = witnessAddr->load(std::memory_order_acquire);
48424852

48434853
#if SWIFT_PTRAUTH
48444854
// For associated protocols, the witness is signed with address
@@ -4897,9 +4907,18 @@ static const WitnessTable *swift_getAssociatedConformanceWitnessSlowImpl(
48974907

48984908
// The access function returns an unsigned pointer for now.
48994909

4900-
// We can't just use initAssociatedConformanceProtocolWitness because we
4901-
// also use this function for base protocols.
4902-
initProtocolWitness(witnessAddr, assocWitnessTable, *assocConformance);
4910+
auto valueToStore = assocWitnessTable;
4911+
#if SWIFT_PTRAUTH
4912+
if (assocConformance->Flags.isSignedWithAddress()) {
4913+
uint16_t extraDiscriminator =
4914+
assocConformance->Flags.getExtraDiscriminator();
4915+
valueToStore = ptrauth_sign_unauthenticated(valueToStore,
4916+
swift_ptrauth_key_associated_conformance,
4917+
ptrauth_blend_discriminator(witnessAddr,
4918+
extraDiscriminator));
4919+
}
4920+
#endif
4921+
witnessAddr->store(valueToStore, std::memory_order_release);
49034922

49044923
return assocWitnessTable;
49054924
}
@@ -4920,8 +4939,8 @@ const WitnessTable *swift::swift_getAssociatedConformanceWitness(
49204939

49214940
// Retrieve the witness.
49224941
unsigned witnessIndex = assocConformance - reqBase;
4923-
auto *witnessAddr = &((const void* *)wtable)[witnessIndex];
4924-
auto witness = *witnessAddr;
4942+
auto *witnessAddr = &((AssociatedConformanceWitness*)wtable)[witnessIndex];
4943+
auto witness = witnessAddr->load(std::memory_order_acquire);
49254944

49264945
#if SWIFT_PTRAUTH
49274946
uint16_t extraDiscriminator = assocConformance->Flags.getExtraDiscriminator();
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// RUN: %target-swiftc_driver %s -g %import-libdispatch -o %t
2+
// RUN: %target-codesign %t
3+
// RUN: %target-run %t
4+
// REQUIRES: libdispatch
5+
// REQUIRES: executable_test
6+
7+
import Dispatch
8+
9+
protocol P {
10+
associatedtype Unused
11+
}
12+
struct A<T> : P {
13+
// We never actually read this associated type, but its presence in the
14+
// wtable should force it to be instantiated rather than shared for
15+
// all specializations of A.
16+
typealias Unused = A<A<T>>
17+
}
18+
19+
protocol Q {
20+
associatedtype Assoc: P
21+
22+
func foo()
23+
}
24+
struct B<T: P> : Q {
25+
// Both the metadata and the wtable for this associated type require
26+
// runtime instantiation and must be fetched lazily.
27+
typealias Assoc = A<T>
28+
29+
func foo() {}
30+
}
31+
32+
// It's possible that the optimizer might someday be able to recognize
33+
// that this is a no-op.
34+
func rundown<T: Q>(value: T, count: Int) {
35+
guard count > 0 else { return }
36+
37+
value.foo()
38+
39+
// Assuming that T is B<U> for some U, this constructs B<A<U>>,
40+
// which will be T in the recursive call; i.e. we should have a
41+
// different metadata/wtable pair each time. In order to construct
42+
// that, we have to read the associated metadata and wtable for T.Assoc.
43+
rundown(value: B<T.Assoc>(), count: count - 1)
44+
}
45+
46+
DispatchQueue.concurrentPerform(iterations: 5) { n in
47+
rundown(value: B<A<()>>(), count: 1000)
48+
}

0 commit comments

Comments
 (0)