Skip to content

Commit ef7dfad

Browse files
committed
Register Dependency Arcs for Extension Members
This was a hole in the existing dependency tracking infrastructure. David managed to discover a way to exploit this bug to get a miscompile in rdar://74583179. There, members of extensions were not counted towards the interface hash of a type and so mutating them could lead to e.g. the wrong declaration being selected in an overload set. To start tracking extensions, we need to add two new kinds of arcs: 1) A nominal arc to the extension 2) A member arc to the extension Unfortunately, extensions are also unique in Swift in that they do not have a name to allow us to unique them. Luckily, we do have a way of identifying extensions: their fingerprint. These arcs are therefore emitted with the extended nominal type and the fingerprint of the extension as their context. This effectively invents a new nominal type for every extension.
1 parent d488f31 commit ef7dfad

File tree

3 files changed

+44
-9
lines changed

3 files changed

+44
-9
lines changed

include/swift/AST/FineGrainedDependencies.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -426,14 +426,14 @@ class DependencyKey {
426426
private:
427427
const NodeKind kind;
428428
const DeclAspect aspect;
429-
const NominalTypeDecl *context;
429+
const DeclContext *context;
430430
StringRef name;
431431

432432
private:
433433
// A private copy constructor so our clients are forced to use the
434434
// move-only builder interface.
435435
explicit Builder(NodeKind kind, DeclAspect aspect,
436-
const NominalTypeDecl *context, StringRef name)
436+
const DeclContext *context, StringRef name)
437437
: kind(kind), aspect(aspect), context(context), name(name) {}
438438

439439
public:

lib/AST/FrontendSourceFileDepGraphFactory.cpp

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,18 @@ using namespace fine_grained_dependencies;
5656
// MARK: Helpers for key construction that must be in frontend
5757
//==============================================================================
5858

59-
static std::string mangleTypeAsContext(const NominalTypeDecl *NTD) {
59+
static std::string identifierForContext(const DeclContext *DC) {
60+
if (!DC) return "";
61+
6062
Mangle::ASTMangler Mangler;
61-
return !NTD ? "" : Mangler.mangleTypeAsContextUSR(NTD);
63+
if (const auto *context = dyn_cast<NominalTypeDecl>(DC)) {
64+
return Mangler.mangleTypeAsContextUSR(context);
65+
}
66+
67+
const auto *ext = cast<ExtensionDecl>(DC);
68+
auto fp = ext->getBodyFingerprint().getValueOr(Fingerprint::ZERO());
69+
auto typeStr = Mangler.mangleTypeAsContextUSR(ext->getExtendedNominal());
70+
return (typeStr + "@" + fp.getRawValue()).str();
6271
}
6372

6473
//==============================================================================
@@ -69,7 +78,7 @@ DependencyKey DependencyKey::Builder::build() && {
6978
return DependencyKey{
7079
kind,
7180
aspect,
72-
context ? mangleTypeAsContext(context) : "",
81+
identifierForContext(context),
7382
name.str()
7483
};
7584
}
@@ -98,10 +107,15 @@ DependencyKey::Builder DependencyKey::Builder::withContext(const Decl *D) && {
98107
switch (kind) {
99108
case NodeKind::nominal:
100109
case NodeKind::potentialMember:
101-
case NodeKind::member:
110+
case NodeKind::member: {
102111
/// nominal and potential member dependencies are created from a Decl and
103112
/// use the context field.
104-
return Builder{kind, aspect, cast<NominalTypeDecl>(D), name};
113+
const DeclContext *context = dyn_cast<NominalTypeDecl>(D);
114+
if (!context) {
115+
context = cast<ExtensionDecl>(D);
116+
}
117+
return Builder{kind, aspect, context, name};
118+
}
105119
case NodeKind::topLevel:
106120
case NodeKind::dynamicLookup:
107121
case NodeKind::externalDepend:
@@ -403,6 +417,7 @@ void FrontendSourceFileDepGraphFactory::addAllDefinedDecls() {
403417
addAllDefinedDeclsOfAGivenType<NodeKind::topLevel>(declFinder.topNominals);
404418
addAllDefinedDeclsOfAGivenType<NodeKind::topLevel>(declFinder.topValues);
405419
addAllDefinedDeclsOfAGivenType<NodeKind::nominal>(declFinder.allNominals);
420+
addAllDefinedDeclsOfAGivenType<NodeKind::nominal>(declFinder.extensions);
406421
addAllDefinedDeclsOfAGivenType<NodeKind::potentialMember>(
407422
declFinder.potentialMemberHolders);
408423
addAllDefinedDeclsOfAGivenType<NodeKind::member>(
@@ -460,11 +475,23 @@ class UsedDeclEnumerator {
460475
return;
461476
}
462477

463-
auto key =
478+
auto nominalKey =
479+
DependencyKey::Builder(NodeKind::nominal, DeclAspect::interface)
480+
.withContext(subject->getSelfNominalTypeDecl())
481+
.build();
482+
enumerateUse(nominalKey, enumerator);
483+
484+
auto declKey =
464485
DependencyKey::Builder(NodeKind::nominal, DeclAspect::interface)
465486
.withContext(subject->getAsDecl())
466487
.build();
467-
enumerateUse(key, enumerator);
488+
489+
// If the subject of this dependency is not the nominal type itself,
490+
// record another arc for the extension this member came from.
491+
if (nominalKey != declKey) {
492+
assert(isa<ExtensionDecl>(subject));
493+
enumerateUse(declKey, enumerator);
494+
}
468495
});
469496
}
470497

@@ -571,6 +598,7 @@ void ModuleDepGraphFactory::addAllDefinedDecls() {
571598
addAllDefinedDeclsOfAGivenType<NodeKind::topLevel>(declFinder.topNominals);
572599
addAllDefinedDeclsOfAGivenType<NodeKind::topLevel>(declFinder.topValues);
573600
addAllDefinedDeclsOfAGivenType<NodeKind::nominal>(declFinder.allNominals);
601+
addAllDefinedDeclsOfAGivenType<NodeKind::nominal>(declFinder.extensions);
574602
addAllDefinedDeclsOfAGivenType<NodeKind::potentialMember>(
575603
declFinder.potentialMemberHolders);
576604
addAllDefinedDeclsOfAGivenType<NodeKind::member>(

lib/AST/NameLookupRequests.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,13 @@ void DirectLookupRequest::writeDependencySink(
305305
evaluator::DependencyCollector &tracker,
306306
const TinyPtrVector<ValueDecl *> &result) const {
307307
auto &desc = std::get<0>(getStorage());
308+
// Add used members from the perspective of
309+
// 1) The decl context they are found in
310+
// 2) The decl context of the request
311+
// This gets us a dependency not just on `Foo.bar` but on `extension Foo.bar`.
312+
for (const auto *member : result) {
313+
tracker.addUsedMember(member->getDeclContext(), desc.Name.getBaseName());
314+
}
308315
tracker.addUsedMember(desc.DC, desc.Name.getBaseName());
309316
}
310317

0 commit comments

Comments
 (0)