Skip to content

Commit 04ce7ab

Browse files
slavapestovslavapestov
authored
Merge pull request #21504 from slavapestov/sil-verifier-checks
Minor improvements to SIL verifier
2 parents 22f6a2d + d8cf20e commit 04ce7ab

File tree

12 files changed

+164
-56
lines changed

12 files changed

+164
-56
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4103,6 +4103,9 @@ ERROR(inlinable_decl_not_public,
41034103
"but %0 is %select{private|fileprivate|internal|%error|%error}1",
41044104
(DeclBaseName, AccessLevel))
41054105

4106+
ERROR(inlinable_resilient_deinit,
4107+
none, "deinitializer can only be '@inlinable' if the class is '@_fixed_layout'", ())
4108+
41064109
//------------------------------------------------------------------------------
41074110
// MARK: @_specialize diagnostics
41084111
//------------------------------------------------------------------------------

‎lib/SIL/SILDeclRef.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,11 @@ bool SILDeclRef::isTransparent() const {
444444

445445
/// True if the function should have its body serialized.
446446
IsSerialized_t SILDeclRef::isSerialized() const {
447+
// Native-to-foreign thunks are only referenced from the Objective-C
448+
// method table.
449+
if (isForeign)
450+
return IsNotSerialized;
451+
447452
DeclContext *dc;
448453
if (auto closure = getAbstractClosureExpr())
449454
dc = closure->getLocalContext();

‎lib/SIL/SILFunction.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -477,10 +477,16 @@ bool SILFunction::hasValidLinkageForFragileRef() const {
477477
if (hasValidLinkageForFragileInline())
478478
return true;
479479

480-
// If the containing module has been serialized
481-
if (getModule().isSerialized()) {
480+
// If the containing module has been serialized already, we no longer
481+
// enforce any invariants.
482+
if (getModule().isSerialized())
482483
return true;
483-
}
484+
485+
// If the function has a subclass scope that limits its visibility outside
486+
// the module despite its linkage, we cannot reference it.
487+
if (getClassSubclassScope() == SubclassScope::Resilient &&
488+
isAvailableExternally())
489+
return false;
484490

485491
// Otherwise, only public functions can be referenced.
486492
return hasPublicVisibility(getLinkage());

‎lib/SIL/SILVerifier.cpp

Lines changed: 86 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ namespace {
119119

120120
/// Verify invariants on a key path component.
121121
void verifyKeyPathComponent(SILModule &M,
122+
ResilienceExpansion expansion,
122123
llvm::function_ref<void(bool, StringRef)> require,
123124
CanType &baseTy,
124125
CanType leafTy,
@@ -211,13 +212,21 @@ void verifyKeyPathComponent(SILModule &M,
211212
switch (auto kind = component.getKind()) {
212213
case KeyPathPatternComponent::Kind::StoredProperty: {
213214
auto property = component.getStoredPropertyDecl();
215+
if (expansion == ResilienceExpansion::Minimal) {
216+
require(property->getEffectiveAccess() >= AccessLevel::Public,
217+
"Key path in serialized function cannot reference non-public "
218+
"property");
219+
}
220+
214221
auto fieldTy = baseTy->getTypeOfMember(M.getSwiftModule(), property)
215222
->getReferenceStorageReferent()
216223
->getCanonicalType();
217224
require(fieldTy == componentTy,
218225
"property decl should be a member of the base with the same type "
219226
"as the component");
220227
require(property->hasStorage(), "property must be stored");
228+
require(!property->isResilient(M.getSwiftModule(), expansion),
229+
"cannot access storage of resilient property");
221230
auto propertyTy = loweredBaseTy.getFieldType(property, M);
222231
require(propertyTy.getObjectType()
223232
== loweredComponentTy.getObjectType(),
@@ -247,6 +256,12 @@ void verifyKeyPathComponent(SILModule &M,
247256
// Getter should be <Sig...> @convention(thin) (@in_guaranteed Base) -> @out Result
248257
{
249258
auto getter = component.getComputedPropertyGetter();
259+
if (expansion == ResilienceExpansion::Minimal) {
260+
require(getter->hasValidLinkageForFragileRef(),
261+
"Key path in serialized function should not reference "
262+
"less visible getters");
263+
}
264+
250265
auto substGetterType = getter->getLoweredFunctionType()
251266
->substGenericArgs(M, patternSubs);
252267
require(substGetterType->getRepresentation() ==
@@ -286,6 +301,12 @@ void verifyKeyPathComponent(SILModule &M,
286301
// <Sig...> @convention(thin) (@in_guaranteed Result, @in Base) -> ()
287302

288303
auto setter = component.getComputedPropertySetter();
304+
if (expansion == ResilienceExpansion::Minimal) {
305+
require(setter->hasValidLinkageForFragileRef(),
306+
"Key path in serialized function should not reference "
307+
"less visible setters");
308+
}
309+
289310
auto substSetterType = setter->getLoweredFunctionType()
290311
->substGenericArgs(M, patternSubs);
291312

@@ -1456,8 +1477,13 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
14561477
}
14571478

14581479
void checkAllocGlobalInst(AllocGlobalInst *AGI) {
1480+
SILGlobalVariable *RefG = AGI->getReferencedGlobal();
1481+
if (auto *VD = RefG->getDecl()) {
1482+
require(!VD->isResilient(F.getModule().getSwiftModule(),
1483+
F.getResilienceExpansion()),
1484+
"cannot access storage of resilient global");
1485+
}
14591486
if (F.isSerialized()) {
1460-
SILGlobalVariable *RefG = AGI->getReferencedGlobal();
14611487
require(RefG->isSerialized()
14621488
|| hasPublicVisibility(RefG->getLinkage()),
14631489
"alloc_global inside fragile function cannot "
@@ -1469,6 +1495,11 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
14691495
SILGlobalVariable *RefG = GAI->getReferencedGlobal();
14701496
require(GAI->getType().getObjectType() == RefG->getLoweredType(),
14711497
"global_addr/value must be the type of the variable it references");
1498+
if (auto *VD = RefG->getDecl()) {
1499+
require(!VD->isResilient(F.getModule().getSwiftModule(),
1500+
F.getResilienceExpansion()),
1501+
"cannot access storage of resilient global");
1502+
}
14721503
if (F.isSerialized()) {
14731504
require(RefG->isSerialized()
14741505
|| hasPublicVisibility(RefG->getLinkage()),
@@ -2041,6 +2072,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
20412072
require(!structDecl->hasUnreferenceableStorage(),
20422073
"Cannot build a struct with unreferenceable storage from elements "
20432074
"using StructInst");
2075+
require(!structDecl->isResilient(F.getModule().getSwiftModule(),
2076+
F.getResilienceExpansion()),
2077+
"cannot access storage of resilient struct");
20442078
require(SI->getType().isObject(),
20452079
"StructInst must produce an object");
20462080

@@ -2232,8 +2266,14 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
22322266
void checkDeallocRefInst(DeallocRefInst *DI) {
22332267
require(DI->getOperand()->getType().isObject(),
22342268
"Operand of dealloc_ref must be object");
2235-
require(DI->getOperand()->getType().getClassOrBoundGenericClass(),
2236-
"Operand of dealloc_ref must be of class type");
2269+
auto *cd = DI->getOperand()->getType().getClassOrBoundGenericClass();
2270+
require(cd, "Operand of dealloc_ref must be of class type");
2271+
2272+
if (!DI->canAllocOnStack()) {
2273+
require(!cd->isResilient(F.getModule().getSwiftModule(),
2274+
F.getResilienceExpansion()),
2275+
"cannot directly deallocate resilient class");
2276+
}
22372277
}
22382278
void checkDeallocPartialRefInst(DeallocPartialRefInst *DPRI) {
22392279
require(DPRI->getInstance()->getType().isObject(),
@@ -2247,6 +2287,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
22472287
->getInstanceType()->getClassOrBoundGenericClass();
22482288
require(class2,
22492289
"Second operand of dealloc_partial_ref must be a class metatype");
2290+
require(!class2->isResilient(F.getModule().getSwiftModule(),
2291+
F.getResilienceExpansion()),
2292+
"cannot directly deallocate resilient class");
22502293
while (class1 != class2) {
22512294
class1 = class1->getSuperclassDecl();
22522295
require(class1, "First operand not superclass of second instance type");
@@ -2343,6 +2386,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
23432386
"result of struct_extract cannot be address");
23442387
StructDecl *sd = operandTy.getStructOrBoundGenericStruct();
23452388
require(sd, "must struct_extract from struct");
2389+
require(!sd->isResilient(F.getModule().getSwiftModule(),
2390+
F.getResilienceExpansion()),
2391+
"cannot access storage of resilient struct");
23462392
require(!EI->getField()->isStatic(),
23472393
"cannot get address of static property with struct_element_addr");
23482394
require(EI->getField()->hasStorage(),
@@ -2384,6 +2430,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
23842430
"must derive struct_element_addr from address");
23852431
StructDecl *sd = operandTy.getStructOrBoundGenericStruct();
23862432
require(sd, "struct_element_addr operand must be struct address");
2433+
require(!sd->isResilient(F.getModule().getSwiftModule(),
2434+
F.getResilienceExpansion()),
2435+
"cannot access storage of resilient struct");
23872436
require(EI->getType().isAddress(),
23882437
"result of struct_element_addr must be address");
23892438
require(!EI->getField()->isStatic(),
@@ -2413,6 +2462,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
24132462
SILType operandTy = EI->getOperand()->getType();
24142463
ClassDecl *cd = operandTy.getClassOrBoundGenericClass();
24152464
require(cd, "ref_element_addr operand must be a class instance");
2465+
require(!cd->isResilient(F.getModule().getSwiftModule(),
2466+
F.getResilienceExpansion()),
2467+
"cannot access storage of resilient class");
24162468

24172469
require(EI->getField()->getDeclContext() == cd,
24182470
"ref_element_addr field must be a member of the class");
@@ -2433,8 +2485,21 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
24332485
SILType operandTy = RTAI->getOperand()->getType();
24342486
ClassDecl *cd = operandTy.getClassOrBoundGenericClass();
24352487
require(cd, "ref_tail_addr operand must be a class instance");
2488+
require(!cd->isResilient(F.getModule().getSwiftModule(),
2489+
F.getResilienceExpansion()),
2490+
"cannot access storage of resilient class");
2491+
require(cd, "ref_tail_addr operand must be a class instance");
24362492
}
2437-
2493+
2494+
void checkDestructureStructInst(DestructureStructInst *DSI) {
2495+
SILType operandTy = DSI->getOperand()->getType();
2496+
StructDecl *sd = operandTy.getStructOrBoundGenericStruct();
2497+
require(sd, "must struct_extract from struct");
2498+
require(!sd->isResilient(F.getModule().getSwiftModule(),
2499+
F.getResilienceExpansion()),
2500+
"cannot access storage of resilient struct");
2501+
}
2502+
24382503
SILType getMethodSelfType(CanSILFunctionType ft) {
24392504
return fnConv.getSILType(ft->getParameters().back());
24402505
}
@@ -3867,8 +3932,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
38673932

38683933
// Find the set of enum elements for the type so we can verify
38693934
// exhaustiveness.
3870-
// FIXME: We also need to consider if the enum is resilient, in which case
3871-
// we're never guaranteed to be exhaustive.
38723935
llvm::DenseSet<EnumElementDecl*> unswitchedElts;
38733936
uDecl->getAllElements(unswitchedElts);
38743937

@@ -3954,8 +4017,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
39544017

39554018
// Find the set of enum elements for the type so we can verify
39564019
// exhaustiveness.
3957-
// FIXME: We also need to consider if the enum is resilient, in which case
3958-
// we're never guaranteed to be exhaustive.
39594020
llvm::DenseSet<EnumElementDecl*> unswitchedElts;
39604021
uDecl->getAllElements(unswitchedElts);
39614022

@@ -3979,9 +4040,12 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
39794040
}
39804041

39814042
// If the switch is non-exhaustive, we require a default.
3982-
require(unswitchedElts.empty() || SOI->hasDefault(),
3983-
"nonexhaustive switch_enum_addr must have a default "
3984-
"destination");
4043+
bool isExhaustive =
4044+
uDecl->isEffectivelyExhaustive(F.getModule().getSwiftModule(),
4045+
F.getResilienceExpansion());
4046+
require((isExhaustive && unswitchedElts.empty()) || SOI->hasDefault(),
4047+
"nonexhaustive switch_enum_addr must have a default destination");
4048+
39854049
if (SOI->hasDefault())
39864050
require(SOI->getDefaultBB()->args_empty(),
39874051
"switch_enum_addr default destination must take "
@@ -4242,7 +4306,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
42424306
break;
42434307
}
42444308

4245-
verifyKeyPathComponent(F.getModule(),
4309+
verifyKeyPathComponent(F.getModule(), F.getResilienceExpansion(),
42464310
[&](bool reqt, StringRef message) { _require(reqt, message); },
42474311
baseTy,
42484312
leafTy,
@@ -4879,15 +4943,16 @@ void SILProperty::verify(const SILModule &M) const {
48794943

48804944
if (auto &component = getComponent()) {
48814945
verifyKeyPathComponent(const_cast<SILModule&>(M),
4882-
require,
4883-
baseTy,
4884-
leafTy,
4885-
*component,
4886-
{},
4887-
canSig,
4888-
subs,
4889-
/*property descriptor*/true,
4890-
hasIndices);
4946+
ResilienceExpansion::Maximal,
4947+
require,
4948+
baseTy,
4949+
leafTy,
4950+
*component,
4951+
{},
4952+
canSig,
4953+
subs,
4954+
/*property descriptor*/true,
4955+
hasIndices);
48914956
// verifyKeyPathComponent updates baseTy to be the projected type of the
48924957
// component, which should be the same as the type of the declared storage
48934958
require(baseTy == leafTy,

‎lib/Sema/TypeCheckAttr.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1992,6 +1992,16 @@ void AttributeChecker::visitInlinableAttr(InlinableAttr *attr) {
19921992
access);
19931993
return;
19941994
}
1995+
1996+
// @inlinable cannot be applied to deinitializers in resilient classes.
1997+
if (auto *DD = dyn_cast<DestructorDecl>(D)) {
1998+
if (auto *CD = dyn_cast<ClassDecl>(DD->getDeclContext())) {
1999+
if (CD->isResilient()) {
2000+
diagnoseAndRemoveAttr(attr, diag::inlinable_resilient_deinit);
2001+
return;
2002+
}
2003+
}
2004+
}
19952005
}
19962006

19972007
void AttributeChecker::visitOptimizeAttr(OptimizeAttr *attr) {

‎test/ClangImporter/static_inline.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// RUN: %FileCheck < %t/static_inline.sil %s
77
// RUN: %target-swift-frontend -parse-as-library -module-name=static_inline -O -emit-ir %t/static_inline.sil -enable-objc-interop -import-objc-header %S/Inputs/static_inline.h | %FileCheck --check-prefix=CHECK-IR %s
88

9-
// CHECK: sil shared [serializable] [clang c_inline_func] @c_inline_func : $@convention(c) (Int32) -> Int32
9+
// CHECK: sil shared [clang c_inline_func] @c_inline_func : $@convention(c) (Int32) -> Int32
1010

1111
// CHECK-IR-LABEL: define{{.*}} i32 @"$s13static_inline6testit1xs5Int32VAE_tF"(i32)
1212
// CHECK-IR: = add {{.*}}, 27

‎test/IRGen/class_resilience.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import resilient_class
3737
sil @allocResilientOutsideParent : $@convention(thin) () -> () {
3838
bb0:
3939
%c = alloc_ref $ResilientOutsideParent
40-
dealloc_ref %c : $ResilientOutsideParent
40+
strong_release %c : $ResilientOutsideParent
4141
%result = tuple ()
4242
return %result : $()
4343
}

‎test/ParseableInterface/ObjC.swiftinterface

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,9 @@ public class SomeClassInlinable {
2929
@inlinable get { return 0 }
3030
@inlinable set {}
3131
}
32-
@objc @inlinable deinit {
33-
print("bye")
34-
}
32+
@objc deinit
3533
}
34+
3635
public class SomeNSObject : NSObject {
3736
@objc init?(_: Any)
3837
@objc func foo()
@@ -53,7 +52,5 @@ public class SomeNSObjectInlinable : NSObject {
5352
@inlinable get { return 0 }
5453
@inlinable set {}
5554
}
56-
@objc @inlinable deinit {
57-
print("bye")
58-
}
55+
@objc deinit
5956
}

‎test/SIL/Serialization/boxes.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// First parse this and then emit a *.sib. Then read in the *.sib, then recreate
22
// RUN: %empty-directory(%t)
33
// RUN: %target-sil-opt %s -emit-sib -o %t/tmp.sib -module-name boxes
4-
// RUN: %target-sil-opt %t/tmp.sib -o %t/tmp.2.sib -module-name boxes
4+
// RUN: %target-sil-opt %t/tmp.sib -emit-sib -o %t/tmp.2.sib -module-name boxes
55
// RUN: %target-sil-opt %t/tmp.2.sib -module-name boxes | %FileCheck %s
66

77
sil_stage canonical

0 commit comments

Comments
 (0)