Skip to content

Fix global accessor and class linker errors in package. #73686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -4151,6 +4151,13 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
/// is built resiliently.
bool isResilient() const;

/// True if the decl is resilient AND also its defining module does
/// _not_ allow non-resilient access; the module can allow such access
/// if package optimization is enabled so its client modules within the
/// same package can have a direct access to this decl even if it's
/// resilient.
bool isStrictlyResilient() const;

/// Returns whether this decl is accessed non/resiliently at the _use_ site
/// in \p accessingModule, depending on \p expansion.
///
Expand Down Expand Up @@ -6015,6 +6022,13 @@ class AbstractStorageDecl : public ValueDecl {
/// property from the given module?
bool isResilient(ModuleDecl *M, ResilienceExpansion expansion) const;

/// True if the decl is resilient AND also its defining module does
/// _not_ allow non-resilient access; the module can allow such access
/// if package optimization is enabled so its client modules within the
/// same package can have a direct access to this decl even if it's
/// resilient.
bool isStrictlyResilient() const;

/// True if the storage can be referenced by a keypath directly.
/// Otherwise, its override must be referenced.
bool isValidKeyPathComponent() const;
Expand Down
9 changes: 9 additions & 0 deletions include/swift/AST/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,15 @@ class ModuleDecl
return getResilienceStrategy() != ResilienceStrategy::Default;
}

/// True if this module is resilient AND also does _not_ allow
/// non-resilient access; the module can allow such access if
/// package optimization is enabled so its client modules within
/// the same package can have a direct access to decls in this
/// module even if it's built resiliently.
bool isStrictlyResilient() const {
return isResilient() && !allowNonResilientAccess();
}

/// Look up a (possibly overloaded) value set at top-level scope
/// (but with the specified access path, which may come from an import decl)
/// within the current module.
Expand Down
8 changes: 8 additions & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3032,6 +3032,10 @@ bool Decl::isOutermostPrivateOrFilePrivateScope() const {
!isInPrivateOrLocalContext(this);
}

bool AbstractStorageDecl::isStrictlyResilient() const {
return isResilient() && !getModuleContext()->allowNonResilientAccess();
}

bool AbstractStorageDecl::isResilient() const {
// Check for an explicit @_fixed_layout attribute.
if (getAttrs().hasAttribute<FixedLayoutAttr>())
Expand Down Expand Up @@ -5142,6 +5146,10 @@ bool NominalTypeDecl::isResilient() const {
return getModuleContext()->isResilient();
}

bool NominalTypeDecl::isStrictlyResilient() const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: strict is vague when used as an API name. A better alternative could be something like isNonResilientAccessDisallowed()

return isResilient() && !getModuleContext()->allowNonResilientAccess();
}

DestructorDecl *NominalTypeDecl::getValueTypeDestructor() {
if (!isa<StructDecl>(this) && !isa<EnumDecl>(this)) {
return nullptr;
Expand Down
18 changes: 7 additions & 11 deletions lib/SIL/IR/SILDeclRef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -460,12 +460,8 @@ static LinkageLimit getLinkageLimit(SILDeclRef constant) {
case Kind::EnumElement:
return Limit::OnDemand;

case Kind::GlobalAccessor: {
auto varDecl = cast<VarDecl>(d);
return varDecl->isResilient() &&
!varDecl->getModuleContext()->allowNonResilientAccess() ?
Limit::NeverPublic : Limit::None;
}
case Kind::GlobalAccessor:
return cast<VarDecl>(d)->isStrictlyResilient() ? Limit::NeverPublic : Limit::None;

case Kind::DefaultArgGenerator:
// If the default argument is to be serialized, only use non-ABI public
Expand Down Expand Up @@ -511,7 +507,7 @@ static LinkageLimit getLinkageLimit(SILDeclRef constant) {
return Limit::AlwaysEmitIntoClient;

// FIXME: This should always be true.
if (d->getModuleContext()->isResilient())
if (d->getModuleContext()->isStrictlyResilient())
return Limit::NeverPublic;

break;
Expand Down Expand Up @@ -1532,10 +1528,10 @@ SubclassScope SILDeclRef::getSubclassScope() const {
// FIXME: This is too narrow. Any class with resilient metadata should
// probably have this, at least for method overrides that don't add new
// vtable entries.
bool isResilientClass = classType->isResilient();
bool isStrictResilientClass = classType->isStrictlyResilient();

if (auto *CD = dyn_cast<ConstructorDecl>(decl)) {
if (isResilientClass)
if (isStrictResilientClass)
return SubclassScope::NotApplicable;
// Initializing entry points do not appear in the vtable.
if (kind == SILDeclRef::Kind::Initializer)
Expand Down Expand Up @@ -1566,14 +1562,14 @@ SubclassScope SILDeclRef::getSubclassScope() const {
// In the resilient case, we're going to be making symbols _less_
// visible, so make sure we stop now; final methods can always be
// called directly.
if (isResilientClass)
if (isStrictResilientClass)
return SubclassScope::Internal;
}

assert(decl->getEffectiveAccess() <= classType->getEffectiveAccess() &&
"class must be as visible as its members");

if (isResilientClass) {
if (isStrictResilientClass) {
// The symbol should _only_ be reached via the vtable, so we're
// going to make it hidden.
return SubclassScope::Resilient;
Expand Down
6 changes: 1 addition & 5 deletions lib/SIL/IR/SILSymbolVisitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ class SILSymbolVisitorImpl : public ASTVisitor<SILSymbolVisitorImpl> {

void visitVarDecl(VarDecl *VD) {
// Variables inside non-resilient modules have some additional symbols.
if (!VD->isResilient()) {
if (!VD->isStrictlyResilient()) {
// Non-global variables might have an explicit initializer symbol in
// non-resilient modules.
if (VD->getAttrs().hasAttribute<HasInitialValueAttr>() &&
Expand All @@ -589,25 +589,21 @@ class SILSymbolVisitorImpl : public ASTVisitor<SILSymbolVisitorImpl> {
// Stored property initializers for public properties are public.
addFunction(declRef);
}

// Statically/globally stored variables have some special handling.
if (VD->hasStorage() && isGlobalOrStaticVar(VD)) {
if (!shouldSkipVisit(getDeclLinkage(VD))) {
Visitor.addGlobalVar(VD);
}

if (VD->isLazilyInitializedGlobal())
addFunction(SILDeclRef(VD, SILDeclRef::Kind::GlobalAccessor));
}

// Wrapped non-static member properties may have a backing initializer.
auto initInfo = VD->getPropertyWrapperInitializerInfo();
if (initInfo.hasInitFromWrappedValue() && !VD->isStatic()) {
addFunction(SILDeclRef(
VD, SILDeclRef::Kind::PropertyWrapperBackingInitializer));
}
}

visitAbstractStorageDecl(VD);
}

Expand Down
150 changes: 150 additions & 0 deletions test/IRGen/package_bypass_resilience_class.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t

// RUN: %target-build-swift %t/Core.swift \
// RUN: -module-name=Core -package-name Pkg \
// RUN: -Xfrontend -experimental-allow-non-resilient-access \
// RUN: -enable-library-evolution -O -wmo \
// RUN: -emit-ir -o %t/Core.ir \
// RUN: -emit-tbd -emit-tbd-path %t/libCore.tbd \
// RUN: -Xfrontend -tbd-install_name=libCore.dylib -Xfrontend -validate-tbd-against-ir=all

// RUN: %FileCheck %s --check-prefix=CHECK-IR < %t/Core.ir
// RUN: %FileCheck %s --check-prefix=CHECK-TBD < %t/libCore.tbd

//--- Core.swift

final public class Pub {}

package class Foo {
package var myFoo: Pub?
}

final package class Bar {
package var myBar: Pub?
}

// key path getter for Core.Foo.myFoo
// CHECK-IR-DAG: define linkonce_odr hidden swiftcc void @"$s4Core3FooC02myB0AA3PubCSgvpACTK"

// key path setter for Core.Foo.myFoo
// CHECK-IR-DAG: define linkonce_odr hidden swiftcc void @"$s4Core3FooC02myB0AA3PubCSgvpACTk"

// variable initialization expression of Core.Foo.myFoo
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc {{i32|i64}} @"$s4Core3FooC02myB0AA3PubCSgvpfi"() #0 {

// Core.Foo.myFoo.getter
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc {{i32|i64}} @"$s4Core3FooC02myB0AA3PubCSgvg"(ptr swiftself %0)

// merged Core.Foo.myFoo.getter
// CHECK-IR-DAG: define internal swiftcc {{i32|i64}} @"$s4Core3FooC02myB0AA3PubCSgvgTm"(ptr swiftself %0)

// Core.Foo.myFoo.setter
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc void @"$s4Core3FooC02myB0AA3PubCSgvs"({{i32|i64}} %0, ptr swiftself %1) #1 {

// merged Core.Foo.myFoo.setter
// CHECK-IR-DAG: define internal swiftcc void @"$s4Core3FooC02myB0AA3PubCSgvsTm"({{i32|i64}} %0, ptr swiftself %1)

// Core.Foo.myFoo.modify
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc { ptr, ptr } @"$s4Core3FooC02myB0AA3PubCSgvM"

// Core.Foo.myFoo.modify
// CHECK-IR-DAG: define internal swiftcc void @"$s4Core3FooC02myB0AA3PubCSgvM.resume.0"

// type metadata accessor for Core.Foo
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc %swift.metadata_response @"$s4Core3FooCMa"

// method lookup function for Core.Foo
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc ptr @"$s4Core3FooCMu"(ptr %0, ptr %1)

// dispatch thunk of Core.Foo.myFoo.getter
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc {{i32|i64}} @"$s4Core3FooC02myB0AA3PubCSgvgTj"(ptr swiftself %0)

// dispatch thunk of Core.Foo.myFoo.setter
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc void @"$s4Core3FooC02myB0AA3PubCSgvsTj"({{i32|i64}} %0, ptr swiftself %1)

// dispatch thunk of Core.Foo.myFoo.modify
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc { ptr, ptr } @"$s4Core3FooC02myB0AA3PubCSgvMTj"

// Core.Foo.deinit
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc ptr @"$s4Core3FooCfd"(ptr readonly returned swiftself %0)

// Core.Foo.__deallocating_deinit
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc void @"$s4Core3FooCfD"(ptr swiftself %0)


// Core.Bar.myBar
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc {{i32|i64}} @"$s4Core3BarC02myB0AA3PubCSgvpfi"()
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc {{i32|i64}} @"$s4Core3BarC02myB0AA3PubCSgvg"(ptr swiftself %0)
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc void @"$s4Core3BarC02myB0AA3PubCSgvs"({{i32|i64}} %0, ptr swiftself %1)
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc { ptr, ptr } @"$s4Core3BarC02myB0AA3PubCSgvM"
// CHECK-IR-DAG: define internal swiftcc void @"$s4Core3BarC02myB0AA3PubCSgvM.resume.0"

// Core.Bar
// type metadata accessor for Core.Bar
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc %swift.metadata_response @"$s4Core3BarCMa"({{i32|i64}} %0)

// method lookup function for Core.Bar
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc ptr @"$s4Core3BarCMu"(ptr %0, ptr %1)

// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc ptr @"$s4Core3BarCfd"(ptr readonly returned swiftself %0)
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc void @"$s4Core3BarCfD"(ptr swiftself %0)

// Core.Pub
// type metadata accessor for Core.Pub
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc %swift.metadata_response @"$s4Core3PubCMa"({{i32|i64}} %0)

// method lookup function for Core.Pub
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc ptr @"$s4Core3PubCMu"(ptr %0, ptr %1)

// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc ptr @"$s4Core3PubCfd"(ptr readnone returned swiftself %0)
// CHECK-IR-DAG: define {{(dllexport |protected )?}}swiftcc void @"$s4Core3PubCfD"(ptr swiftself %0)


// property descriptor for Core.Foo.myFoo
// CHECK-TBD-DAG: s4Core3FooC02myB0AA3PubCSgvpMV
// method descriptor for Core.Foo.myFoo.getter
// CHECK-TBD-DAG: s4Core3FooC02myB0AA3PubCSgvgTq
// method descriptor for Core.Foo.myFoo.setter
// CHECK-TBD-DAG: s4Core3FooC02myB0AA3PubCSgvsTq
// method descriptor for Core.Foo.myFoo.modify
// CHECK-TBD-DAG: s4Core3FooC02myB0AA3PubCSgvMTq
// dispatch thunk of Core.Foo.myFoo.getter
// CHECK-TBD-DAG: s4Core3FooC02myB0AA3PubCSgvgTj
// dispatch thunk of Core.Foo.myFoo.setter
// CHECK-TBD-DAG: s4Core3FooC02myB0AA3PubCSgvsTj
// dispatch thunk of Core.Foo.myFoo.modify
// CHECK-TBD-DAG: s4Core3FooC02myB0AA3PubCSgvMTj
// type metadata accessor for Core.Foo
// CHECK-TBD-DAG: s4Core3FooCMa
// method lookup function for Core.Foo
// CHECK-TBD-DAG: s4Core3FooCMu
// nominal type descriptor for Core.Foo
// CHECK-TBD-DAG: s4Core3FooCMn
// class metadata base offset for Core.Foo
// CHECK-TBD-DAG: s4Core3FooCMo

// CHECK-TBD-DAG: s4Core3FooC02myB0AA3PubCSgvpfi
// CHECK-TBD-DAG: s4Core3FooC02myB0AA3PubCSgvg
// CHECK-TBD-DAG: s4Core3FooC02myB0AA3PubCSgvs
// CHECK-TBD-DAG: s4Core3FooCfd
// CHECK-TBD-DAG: s4Core3FooCfD

// CHECK-TBD-DAG: s4Core3BarC02myB0AA3PubCSgvpMV
// CHECK-TBD-DAG: s4Core3BarC02myB0AA3PubCSgvpfi
// CHECK-TBD-DAG: s4Core3BarC02myB0AA3PubCSgvg
// CHECK-TBD-DAG: s4Core3BarC02myB0AA3PubCSgvs
// CHECK-TBD-DAG: s4Core3BarC02myB0AA3PubCSgvM
// CHECK-TBD-DAG: s4Core3BarCMa
// CHECK-TBD-DAG: s4Core3BarCMu
// CHECK-TBD-DAG: s4Core3BarCMn
// CHECK-TBD-DAG: s4Core3BarCMo
// CHECK-TBD-DAG: s4Core3BarCfd
// CHECK-TBD-DAG: s4Core3BarCfD

// CHECK-TBD-DAG: s4Core3PubCMa
// CHECK-TBD-DAG: s4Core3PubCMu
// CHECK-TBD-DAG: s4Core3PubCMn
// CHECK-TBD-DAG: s4Core3PubCMo
// CHECK-TBD-DAG: s4Core3PubCfd
// CHECK-TBD-DAG: s4Core3PubCfD
21 changes: 21 additions & 0 deletions test/IRGen/package_bypass_resilience_global_accessor.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// RUN: %empty-directory(%t)

// RUN: %target-build-swift -module-name=File -package-name Pkg -I%t -emit-sil %s | %FileCheck %s --check-prefix=CHECK-SIL
// RUN: %target-build-swift -module-name=File -package-name Pkg -I%t -emit-sil %s -enable-library-evolution | %FileCheck %s --check-prefix=CHECK-SIL-HID
// RUN: %target-build-swift -module-name=File -package-name Pkg -I%t -emit-sil %s -enable-library-evolution -Xfrontend -experimental-allow-non-resilient-access | %FileCheck %s --check-prefix=CHECK-SIL
// RUN: %target-build-swift -module-name=File -package-name Pkg -I%t -emit-ir %s | %FileCheck %s --check-prefix=CHECK-IR
// RUN: %target-build-swift -module-name=File -package-name Pkg -I%t -emit-ir %s -enable-library-evolution | %FileCheck %s --check-prefix=CHECK-IR-HID
// RUN: %target-build-swift -module-name=File -package-name Pkg -I%t -emit-ir %s -enable-library-evolution -Xfrontend -experimental-allow-non-resilient-access | %FileCheck %s --check-prefix=CHECK-IR

public struct S {
public static var x = "hello world"
}

// S.x.unsafeMutableAddressor
// CHECK-SIL: sil [global_init] @$s4File1SV1xSSvau : $@convention(thin) () -> Builtin.RawPointer {
// S.x.unsafeMutableAddressor
// CHECK-SIL-HID: sil hidden [global_init] @$s4File1SV1xSSvau : $@convention(thin) () -> Builtin.RawPointer {

// CHECK-IR: define{{( dllexport)?}}{{( protected)?}} swiftcc ptr @"$s4File1SV1xSSvau"()
// CHECK-IR-HID: define hidden swiftcc ptr @"$s4File1SV1xSSvau"()