Skip to content

Commit d75993b

Browse files
xymusxymus
committed
SILGen: Limit references to NSError in inlinable code optimization
Revisit the optimization that provides a fast path for instances of `NSError` when erasing the `Error` type in `emitExistentialErasure`. It generated references to `NSError` when the `Foundation` module was loaded, no matter how it was imported. This lead to deserialization failures at reading the swiftmodule when that reference was added to inlinable code while `Foundation` was not a public dependency. Fix this crash by limiting the optimization to all non-inlinable code and only inlinable code from a file with a public dependency on `Foundation`. This is the similar check we apply to user written inlinable code. rdar://142438679
1 parent a619daf commit d75993b

File tree

4 files changed

+160
-1
lines changed

4 files changed

+160
-1
lines changed

‎lib/SILGen/SILGenConvert.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -656,9 +656,10 @@ ManagedValue SILGenFunction::emitExistentialErasure(
656656
// If we're erasing to the 'Error' type, we might be able to get an NSError
657657
// representation more efficiently.
658658
auto &ctx = getASTContext();
659+
auto *nsErrorDecl = ctx.getNSErrorDecl();
659660
if (ctx.LangOpts.EnableObjCInterop && conformances.size() == 1 &&
660661
conformances[0].getRequirement() == ctx.getErrorDecl() &&
661-
ctx.getNSErrorDecl()) {
662+
nsErrorDecl && referenceAllowed(nsErrorDecl)) {
662663
// If the concrete type is NSError or a subclass thereof, just erase it
663664
// directly.
664665
auto nsErrorType = ctx.getNSErrorType()->getCanonicalType();

‎lib/SILGen/SILGenFunction.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,26 @@ DeclName SILGenModule::getMagicFunctionName(SILDeclRef ref) {
210210
llvm_unreachable("Unhandled SILDeclRefKind in switch.");
211211
}
212212

213+
bool SILGenFunction::referenceAllowed(ValueDecl *decl) {
214+
// Use in any non-fragile functions.
215+
if (FunctionDC->getResilienceExpansion() == ResilienceExpansion::Maximal)
216+
return true;
217+
218+
// Don't reference an implementation-only dependency.
219+
auto *otherMod = decl->getDeclContext()->getParentModule();
220+
if (SF->getRestrictedImportKind(otherMod) != RestrictedImportKind::None)
221+
return false;
222+
223+
auto *thisMod = FunctionDC->getParentModule();
224+
if (thisMod->getResilienceStrategy() != ResilienceStrategy::Resilient)
225+
return true;
226+
227+
// Don't inline a ref to a non-public import from a resilient module.
228+
ImportAccessLevel accessLevelRestriction = SF->getImportAccessLevel(otherMod);
229+
return !accessLevelRestriction ||
230+
accessLevelRestriction->accessLevel == AccessLevel::Public;
231+
}
232+
213233
SILDebugLocation SILGenFunction::getSILDebugLocation(
214234
SILBuilder &B, SILLocation Loc,
215235
std::optional<SILLocation> CurDebugLocOverride, bool ForMetaInstruction) {

‎lib/SILGen/SILGenFunction.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,6 +789,15 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
789789
bool isEmittingTopLevelCode() { return IsEmittingTopLevelCode; }
790790
void stopEmittingTopLevelCode() { IsEmittingTopLevelCode = false; }
791791

792+
/// Can the generated code reference \c decl safely?
793+
///
794+
/// Checks that the module defining \c decl is as visible to clients as the
795+
/// code referencing it, preventing an inlinable function to reference an
796+
/// implementation-only dependency and similar. This applies similar checks
797+
/// as the exportability checker does to source code for decls referenced by
798+
/// generated code.
799+
bool referenceAllowed(ValueDecl *decl);
800+
792801
std::optional<SILAccessEnforcement>
793802
getStaticEnforcement(VarDecl *var = nullptr);
794803
std::optional<SILAccessEnforcement>

‎test/Serialization/generated-ref-to-nserror.swift

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t --leading-lines
3+
4+
/// Build a minimal version of Foundation defining only NSError
5+
// RUN: %target-swift-frontend -emit-module %t/Foundation.swift -o %t/Foundation.swiftmodule
6+
7+
/// Enabled existential to NSError optimization
8+
// CHECK-OPTIMIZED: $NSError
9+
10+
/// Public import or non-resilient modules
11+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
12+
// RUN: -swift-version 5 -enable-library-evolution \
13+
// RUN: %t/inlinable-public.swift > %t/main.sil
14+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
15+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
16+
// RUN: %t/inlinable-public.swift > %t/main.sil
17+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
18+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
19+
// RUN: %t/inlinable-internal.swift > %t/main.sil
20+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
21+
22+
/// Any non-inlinable uses
23+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
24+
// RUN: %t/non-inlinable-public.swift > %t/main.sil
25+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
26+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
27+
// RUN: %t/non-inlinable-ioi.swift > %t/main.sil
28+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
29+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
30+
// RUN: %t/non-inlinable-internal.swift > %t/main.sil
31+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
32+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
33+
// RUN: %t/non-inlinable-not-imported-fileA.swift \
34+
// RUN: %t/non-inlinable-not-imported-fileB.swift > %t/main.sil
35+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
36+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
37+
// RUN: -swift-version 5 -enable-library-evolution \
38+
// RUN: %t/non-inlinable-not-imported-fileA.swift \
39+
// RUN: %t/non-inlinable-not-imported-fileB.swift > %t/main.sil
40+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
41+
42+
/// Disabled existential to NSError optimization
43+
// CHECK-NOT-OPTIMIZED-NOT: $NSError
44+
45+
/// Implementation-only import
46+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
47+
// RUN: %t/inlinable-ioi.swift > %t/main.sil
48+
// RUN: %FileCheck --check-prefix CHECK-NOT-OPTIMIZED --input-file %t/main.sil %s
49+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
50+
// RUN: -swift-version 5 -enable-library-evolution \
51+
// RUN: %t/inlinable-ioi.swift > %t/main.sil
52+
// RUN: %FileCheck --check-prefix CHECK-NOT-OPTIMIZED --input-file %t/main.sil %s
53+
54+
/// Not imported from the same file
55+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
56+
// RUN: %t/inlinable-not-imported-fileA.swift \
57+
// RUN: %t/inlinable-not-imported-fileB.swift > %t/main.sil
58+
// RUN: %FileCheck --check-prefix CHECK-NOT-OPTIMIZED --input-file %t/main.sil %s
59+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
60+
// RUN: -swift-version 5 -enable-library-evolution \
61+
// RUN: %t/inlinable-not-imported-fileA.swift \
62+
// RUN: %t/inlinable-not-imported-fileB.swift > %t/main.sil
63+
// RUN: %FileCheck --check-prefix CHECK-NOT-OPTIMIZED --input-file %t/main.sil %s
64+
65+
/// Internal import from resilient module
66+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
67+
// RUN: -swift-version 5 -enable-library-evolution \
68+
// RUN: %t/inlinable-internal.swift > %t/main.sil
69+
// RUN: %FileCheck --check-prefix CHECK-NOT-OPTIMIZED --input-file %t/main.sil %s
70+
71+
//--- Foundation.swift
72+
73+
class NSError {}
74+
75+
//--- inlinable-public.swift
76+
public import Foundation
77+
78+
@inlinable public func foo<E: Error>(e: E) -> Error {
79+
return e
80+
}
81+
82+
//--- inlinable-ioi.swift
83+
@_implementationOnly import Foundation
84+
85+
@inlinable public func foo<E: Error>(e: E) -> Error {
86+
return e
87+
}
88+
89+
//--- inlinable-internal.swift
90+
internal import Foundation
91+
92+
@inlinable public func foo<E: Error>(e: E) -> Error {
93+
return e
94+
}
95+
96+
//--- inlinable-not-imported-fileA.swift
97+
@inlinable public func foo<E: Error>(e: E) -> Error {
98+
return e
99+
}
100+
//--- inlinable-not-imported-fileB.swift
101+
import Foundation
102+
103+
//--- non-inlinable-public.swift
104+
public import Foundation
105+
106+
public func foo<E: Error>(e: E) -> Error {
107+
return e
108+
}
109+
110+
//--- non-inlinable-ioi.swift
111+
@_implementationOnly import Foundation
112+
113+
public func foo<E: Error>(e: E) -> Error {
114+
return e
115+
}
116+
117+
//--- non-inlinable-internal.swift
118+
internal import Foundation
119+
120+
public func foo<E: Error>(e: E) -> Error {
121+
return e
122+
}
123+
124+
//--- non-inlinable-not-imported-fileA.swift
125+
public func foo<E: Error>(e: E) -> Error {
126+
return e
127+
}
128+
//--- non-inlinable-not-imported-fileB.swift
129+
import Foundation

0 commit comments

Comments
 (0)