diff --git a/include/swift/Serialization/ModuleFormat.h b/include/swift/Serialization/ModuleFormat.h index 2a28e950d4cb0..492d3853abf21 100644 --- a/include/swift/Serialization/ModuleFormat.h +++ b/include/swift/Serialization/ModuleFormat.h @@ -54,7 +54,7 @@ const uint16_t VERSION_MAJOR = 0; /// in source control, you should also update the comment to briefly /// describe what change you made. The content of this comment isn't important; /// it just ensures a conflict if two people change the module format. -const uint16_t VERSION_MINOR = 397; // No resilience expansion in SILDeclRef +const uint16_t VERSION_MINOR = 398; // Private discriminators for type xrefs using DeclIDField = BCFixed<31>; @@ -1288,6 +1288,7 @@ namespace decls_block { using XRefTypePathPieceLayout = BCRecordLayout< XREF_TYPE_PATH_PIECE, IdentifierIDField, // name + IdentifierIDField, // private discriminator BCFixed<1> // restrict to protocol extension >; diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index 682b6045e37da..50a8c094773c0 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -1264,18 +1264,22 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) { case XREF_TYPE_PATH_PIECE: case XREF_VALUE_PATH_PIECE: { IdentifierID IID; + IdentifierID privateDiscriminator = 0; TypeID TID = 0; bool isType = (recordID == XREF_TYPE_PATH_PIECE); bool inProtocolExt = false; bool isStatic = false; if (isType) - XRefTypePathPieceLayout::readRecord(scratch, IID, inProtocolExt); + XRefTypePathPieceLayout::readRecord(scratch, IID, privateDiscriminator, + inProtocolExt); else XRefValuePathPieceLayout::readRecord(scratch, TID, IID, inProtocolExt, isStatic); DeclBaseName name = getDeclBaseName(IID); pathTrace.addValue(name); + if (privateDiscriminator) + pathTrace.addValue(getIdentifier(privateDiscriminator)); Type filterTy; if (!isType) { @@ -1290,9 +1294,14 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) { pathTrace.addType(filterTy); } - baseModule->lookupQualified(ModuleType::get(baseModule), name, - NL_QualifiedDefault | NL_KnownNoDependency, - /*typeResolver=*/nullptr, values); + if (privateDiscriminator) { + baseModule->lookupMember(values, baseModule, name, + getIdentifier(privateDiscriminator)); + } else { + baseModule->lookupQualified(ModuleType::get(baseModule), name, + NL_QualifiedDefault | NL_KnownNoDependency, + /*typeResolver=*/nullptr, values); + } filterValues(filterTy, nullptr, nullptr, isType, inProtocolExt, isStatic, None, values); break; @@ -1348,7 +1357,7 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) { switch (recordID) { case XREF_TYPE_PATH_PIECE: { IdentifierID IID; - XRefTypePathPieceLayout::readRecord(scratch, IID, None); + XRefTypePathPieceLayout::readRecord(scratch, IID, None, None); result = getIdentifier(IID); break; } @@ -1405,8 +1414,13 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) { // Fast path for nested types that avoids deserializing all // members of the parent type. IdentifierID IID; + IdentifierID privateDiscriminator; bool onlyInNominal = false; - XRefTypePathPieceLayout::readRecord(scratch, IID, onlyInNominal); + XRefTypePathPieceLayout::readRecord(scratch, IID, privateDiscriminator, + onlyInNominal); + if (privateDiscriminator) + goto giveUpFastPath; + Identifier memberName = getIdentifier(IID); pathTrace.addValue(memberName); @@ -1449,21 +1463,25 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) { pathTrace.removeLast(); } +giveUpFastPath: LLVM_FALLTHROUGH; } case XREF_VALUE_PATH_PIECE: case XREF_INITIALIZER_PATH_PIECE: { TypeID TID = 0; DeclBaseName memberName; + Identifier privateDiscriminator; Optional ctorInit; bool isType = false; bool inProtocolExt = false; bool isStatic = false; switch (recordID) { case XREF_TYPE_PATH_PIECE: { - IdentifierID IID; - XRefTypePathPieceLayout::readRecord(scratch, IID, inProtocolExt); + IdentifierID IID, discriminatorID; + XRefTypePathPieceLayout::readRecord(scratch, IID, discriminatorID, + inProtocolExt); memberName = getDeclBaseName(IID); + privateDiscriminator = getIdentifier(discriminatorID); isType = true; break; } @@ -1490,6 +1508,8 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) { } pathTrace.addValue(memberName); + if (!privateDiscriminator.empty()) + pathTrace.addPrivateDiscriminator(privateDiscriminator); Type filterTy; if (!isType) { @@ -1519,8 +1539,17 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) { getXRefDeclNameForError()); } - auto members = nominal->lookupDirect(memberName); - values.append(members.begin(), members.end()); + if (!privateDiscriminator.empty()) { + ModuleDecl *searchModule = M; + if (!searchModule) + searchModule = nominal->getModuleContext(); + searchModule->lookupMember(values, nominal, memberName, + privateDiscriminator); + + } else { + auto members = nominal->lookupDirect(memberName); + values.append(members.begin(), members.end()); + } filterValues(filterTy, M, genericSig, isType, inProtocolExt, isStatic, ctorInit, values); break; diff --git a/lib/Serialization/DeserializationErrors.h b/lib/Serialization/DeserializationErrors.h index b5424c322b8e1..0703ea49495fc 100644 --- a/lib/Serialization/DeserializationErrors.h +++ b/lib/Serialization/DeserializationErrors.h @@ -37,6 +37,7 @@ class XRefTracePath { Accessor, Extension, GenericParam, + PrivateDiscriminator, Unknown }; @@ -55,11 +56,12 @@ class XRefTracePath { : kind(K), data(llvm::PointerLikeTypeTraits::getAsVoidPointer(value)) {} - Identifier getAsIdentifier() const { + DeclBaseName getAsBaseName() const { switch (kind) { case Kind::Value: case Kind::Operator: - return getDataAs(); + case Kind::PrivateDiscriminator: + return getDataAs(); case Kind::Type: case Kind::OperatorFilter: case Kind::Accessor: @@ -73,7 +75,7 @@ class XRefTracePath { void print(raw_ostream &os) const { switch (kind) { case Kind::Value: - os << getDataAs(); + os << getDataAs(); break; case Kind::Type: os << "with type " << getDataAs(); @@ -137,6 +139,9 @@ class XRefTracePath { case Kind::GenericParam: os << "generic param #" << getDataAs(); break; + case Kind::PrivateDiscriminator: + os << "(in " << getDataAs() << ")"; + break; case Kind::Unknown: os << "unknown xref kind " << getDataAs(); break; @@ -180,17 +185,21 @@ class XRefTracePath { path.push_back({ PathPiece::Kind::GenericParam, index }); } + void addPrivateDiscriminator(Identifier name) { + path.push_back({ PathPiece::Kind::PrivateDiscriminator, name }); + } + void addUnknown(uintptr_t kind) { path.push_back({ PathPiece::Kind::Unknown, kind }); } - Identifier getLastName() const { + DeclBaseName getLastName() const { for (auto &piece : reversed(path)) { - Identifier result = piece.getAsIdentifier(); + DeclBaseName result = piece.getAsBaseName(); if (!result.empty()) return result; } - return Identifier(); + return DeclBaseName(); } void removeLast() { diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp index 1eca569696584..a020f9d08a8b4 100644 --- a/lib/Serialization/Serialization.cpp +++ b/lib/Serialization/Serialization.cpp @@ -1868,8 +1868,16 @@ void Serializer::writeCrossReference(const DeclContext *DC, uint32_t pathLen) { auto generic = cast(DC); abbrCode = DeclTypeAbbrCodes[XRefTypePathPieceLayout::Code]; + + Identifier discriminator; + if (generic->isOutermostPrivateOrFilePrivateScope()) { + auto *containingFile = cast(generic->getModuleScopeContext()); + discriminator = containingFile->getDiscriminatorForPrivateValue(generic); + } + XRefTypePathPieceLayout::emitRecord(Out, ScratchRecord, abbrCode, addDeclBaseNameRef(generic->getName()), + addDeclBaseNameRef(discriminator), false); break; } @@ -2019,8 +2027,17 @@ void Serializer::writeCrossReference(const Decl *D) { bool isProtocolExt = D->getDeclContext()->getAsProtocolExtensionContext(); if (auto type = dyn_cast(D)) { abbrCode = DeclTypeAbbrCodes[XRefTypePathPieceLayout::Code]; + + Identifier discriminator; + if (type->isOutermostPrivateOrFilePrivateScope()) { + auto *containingFile = + cast(type->getDeclContext()->getModuleScopeContext()); + discriminator = containingFile->getDiscriminatorForPrivateValue(type); + } + XRefTypePathPieceLayout::emitRecord(Out, ScratchRecord, abbrCode, addDeclBaseNameRef(type->getName()), + addDeclBaseNameRef(discriminator), isProtocolExt); return; } diff --git a/test/Serialization/Inputs/xref-private-type/Lib.swift b/test/Serialization/Inputs/xref-private-type/Lib.swift new file mode 100644 index 0000000000000..4292ce39e52a7 --- /dev/null +++ b/test/Serialization/Inputs/xref-private-type/Lib.swift @@ -0,0 +1,8 @@ +import LibCore + +public let lazyFoo = Foo() +public func testFoo(_: Foo = lazyFoo) {} +public let lazyBar = Bar() +public func testBar(_: Bar = lazyBar) {} +public let lazyBaz = Baz() +public func testBaz(_: Baz = lazyBaz) {} diff --git a/test/Serialization/Inputs/xref-private-type/LibCore.swift b/test/Serialization/Inputs/xref-private-type/LibCore.swift new file mode 100644 index 0000000000000..032dbdf15108d --- /dev/null +++ b/test/Serialization/Inputs/xref-private-type/LibCore.swift @@ -0,0 +1,24 @@ +private class TopLevelInternalClass {} +public struct Foo { + private var ref: TopLevelInternalClass + + public init() { self.ref = .init() } +} + +public struct Bar { + private class NestedInternalClass {} + + private var ref: NestedInternalClass + + public init() { self.ref = .init() } +} + +public struct Baz { + fileprivate class NestedInternalClass { + fileprivate class DoublyNestedInternalClass {} + } + + private var ref: NestedInternalClass.DoublyNestedInternalClass + + public init() { self.ref = .init() } +} diff --git a/test/Serialization/xref-private-type.swift b/test/Serialization/xref-private-type.swift new file mode 100755 index 0000000000000..2ad6c92b87ec8 --- /dev/null +++ b/test/Serialization/xref-private-type.swift @@ -0,0 +1,51 @@ +// RUN: %empty-directory(%t) +// RUN: %target-build-swift -swift-version 4 -O -force-single-frontend-invocation -emit-module-path %t/LibCore.swiftmodule %S/Inputs/xref-private-type/LibCore.swift +// RUN: %target-build-swift -swift-version 4 -O -I %t -force-single-frontend-invocation -emit-module-path %t/Lib.swiftmodule %S/Inputs/xref-private-type/Lib.swift +// RUN: %target-build-swift -swift-version 4 -O -I %t -emit-sil %s | %FileCheck %s + +import Lib + +// CHECK: sil{{.*}} @[[TESTSR6874:[^ ]+10testSR6874[^ ]+]] : +func testSR6874() { + // The important lines in this test are the strong_retains, which refer to + // private types defined in LibCore. Normally we shouldn't have references to + // non-public declarations in inlinable code, but because SIL passes can break + // apart non-resilient structs and enums we can end up in that situation. + // Worse, this can happen *across module boundaries.* + // + // In this test, the addressor for each global defined in Lib ends up + // referencing private types defined in LibCore. Using those globals in + // default argument position leads to the addressor getting inlined into + // calling code in Swift 4 and later. This results in an attempt to not just + // reference a private type, but to *resolve a cross-reference to a private + // type.* + // + // This is the situation in SR-6874 (simplified). I'm not sure of a simpler + // way to reliably trigger the issue. But if this test breaks, please try to + // find one. + // + // (We may want to revisit this whole thing later, as it violates the model. + // But it's also useful for performance in non-resilient code.) + + // CHECK: [[ADDR:%.+]] = global_addr @{{[^ ]+}}3Lib7lazyFoo + // CHECK: [[LOADED:%.+]] = load [[ADDR]] : $*Foo + // CHECK: [[REF:%.+]] = struct_extract [[LOADED]] : $Foo, #Foo.ref + // CHECK: strong_retain [[REF]] : $TopLevelInternalClass + // CHECK: apply {{%.+}}([[LOADED]]) + testFoo() + + // CHECK: [[ADDR:%.+]] = global_addr @{{[^ ]+}}3Lib7lazyBar + // CHECK: [[LOADED:%.+]] = load [[ADDR]] : $*Bar + // CHECK: [[REF:%.+]] = struct_extract [[LOADED]] : $Bar, #Bar.ref + // CHECK: strong_retain [[REF]] : $Bar.NestedInternalClass + // CHECK: apply {{%.+}}([[LOADED]]) + testBar() + + // CHECK: [[ADDR:%.+]] = global_addr @{{[^ ]+}}3Lib7lazyBaz + // CHECK: [[LOADED:%.+]] = load [[ADDR]] : $*Baz + // CHECK: [[REF:%.+]] = struct_extract [[LOADED]] : $Baz, #Baz.ref + // CHECK: strong_retain [[REF]] : $Baz.NestedInternalClass.DoublyNestedInternalClass + // CHECK: apply {{%.+}}([[LOADED]]) + testBaz() +} // CHECK: end sil function '[[TESTSR6874]]' +