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 module with a public dependency on
`Foundation`. This is the similar check we apply to user written
inlinable code, however here we use the module-wide dependency instead
of per file imports.

rdar://142438679
This commit is contained in:
Alexis Laferrière
2025-02-18 16:44:28 -08:00
parent a619daf12b
commit db7126c390
4 changed files with 200 additions and 1 deletions

View File

@@ -656,9 +656,10 @@ ManagedValue SILGenFunction::emitExistentialErasure(
// If we're erasing to the 'Error' type, we might be able to get an NSError
// representation more efficiently.
auto &ctx = getASTContext();
auto *nsErrorDecl = ctx.getNSErrorDecl();
if (ctx.LangOpts.EnableObjCInterop && conformances.size() == 1 &&
conformances[0].getRequirement() == ctx.getErrorDecl() &&
ctx.getNSErrorDecl()) {
nsErrorDecl && referenceAllowed(nsErrorDecl)) {
// If the concrete type is NSError or a subclass thereof, just erase it
// directly.
auto nsErrorType = ctx.getNSErrorType()->getCanonicalType();

View File

@@ -30,6 +30,7 @@
#include "swift/AST/FileUnit.h"
#include "swift/AST/GenericEnvironment.h"
#include "swift/AST/Initializer.h"
#include "swift/AST/ImportCache.h"
#include "swift/AST/ParameterList.h"
#include "swift/AST/PropertyWrappers.h"
#include "swift/AST/SourceFile.h"
@@ -210,6 +211,35 @@ DeclName SILGenModule::getMagicFunctionName(SILDeclRef ref) {
llvm_unreachable("Unhandled SILDeclRefKind in switch.");
}
bool SILGenFunction::referenceAllowed(ValueDecl *decl) {
// Use in any non-fragile functions.
if (FunctionDC->getResilienceExpansion() == ResilienceExpansion::Maximal)
return true;
// Allow same-module references.
auto *targetMod = decl->getDeclContext()->getParentModule();
auto *thisMod = FunctionDC->getParentModule();
if (thisMod == targetMod)
return true;
ModuleDecl::ImportFilter filter = {
ModuleDecl::ImportFilterKind::Exported,
ModuleDecl::ImportFilterKind::Default,
ModuleDecl::ImportFilterKind::SPIOnly};
if (thisMod->getResilienceStrategy() != ResilienceStrategy::Resilient)
filter |= ModuleDecl::ImportFilterKind::InternalOrBelow;
// Look through public module local imports and their reexports.
llvm::SmallVector<ImportedModule, 8> imports;
thisMod->getImportedModules(imports, filter);
auto &importCache = getASTContext().getImportCache();
for (auto &import : imports) {
if (importCache.isImportedBy(targetMod, import.importedModule))
return true;
}
return false;
}
SILDebugLocation SILGenFunction::getSILDebugLocation(
SILBuilder &B, SILLocation Loc,
std::optional<SILLocation> CurDebugLocOverride, bool ForMetaInstruction) {

View File

@@ -789,6 +789,15 @@ public:
bool isEmittingTopLevelCode() { return IsEmittingTopLevelCode; }
void stopEmittingTopLevelCode() { IsEmittingTopLevelCode = false; }
/// Can the generated code reference \c decl safely?
///
/// Checks that the module defining \c decl is as visible to clients as the
/// code referencing it, preventing an inlinable function to reference an
/// implementation-only dependency and similar. This applies similar checks
/// as the exportability checker does to source code for decls referenced by
/// generated code.
bool referenceAllowed(ValueDecl *decl);
std::optional<SILAccessEnforcement>
getStaticEnforcement(VarDecl *var = nullptr);
std::optional<SILAccessEnforcement>

View File

@@ -0,0 +1,159 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t --leading-lines
// REQUIRES: objc_interop
/// Build a minimal version of Foundation defining only NSError
// RUN: %target-swift-frontend -emit-module %t/Foundation.swift \
// RUN: -o %t/Foundation.swiftmodule
// RUN: %target-swift-frontend -emit-module %t/FoundationExporter.swift -I%t \
// RUN: -o %t/FoundationExporter.swiftmodule
/// Enabled existential to NSError optimization
// CHECK-OPTIMIZED: $NSError
/// Optimize Foundation itself
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
// RUN: -swift-version 5 -enable-library-evolution \
// RUN: %t/Foundation.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
/// Public import or non-resilient modules
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
// RUN: -swift-version 5 -enable-library-evolution \
// RUN: %t/inlinable-public.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
// RUN: %t/inlinable-public.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
// RUN: %t/inlinable-internal.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
/// Foundation is imported from a different file
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
// RUN: %t/inlinable-not-imported-fileA.swift \
// RUN: %t/inlinable-not-imported-fileB.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
// RUN: -swift-version 5 -enable-library-evolution \
// RUN: %t/inlinable-not-imported-fileA.swift \
// RUN: %t/inlinable-not-imported-fileB.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
/// Foundation is imported via a transitive dependency
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
// RUN: %t/inlinable-imported-transitive.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
/// Any non-inlinable uses
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
// RUN: %t/non-inlinable-public.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
// RUN: %t/non-inlinable-ioi.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
// RUN: %t/non-inlinable-internal.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
// RUN: %t/non-inlinable-not-imported-fileA.swift \
// RUN: %t/non-inlinable-not-imported-fileB.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
// RUN: -swift-version 5 -enable-library-evolution \
// RUN: %t/non-inlinable-not-imported-fileA.swift \
// RUN: %t/non-inlinable-not-imported-fileB.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
/// Disabled existential to NSError optimization
// CHECK-NOT-OPTIMIZED-NOT: $NSError
/// Implementation-only import
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
// RUN: %t/inlinable-ioi.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-NOT-OPTIMIZED --input-file %t/main.sil %s
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
// RUN: -swift-version 5 -enable-library-evolution \
// RUN: %t/inlinable-ioi.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-NOT-OPTIMIZED --input-file %t/main.sil %s
/// Internal import from resilient module
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
// RUN: -swift-version 5 -enable-library-evolution \
// RUN: %t/inlinable-internal.swift > %t/main.sil
// RUN: %FileCheck --check-prefix CHECK-NOT-OPTIMIZED --input-file %t/main.sil %s
//--- Foundation.swift
class NSError {}
@inlinable public func foo<E: Error>(e: E) -> Error {
return e
}
//--- FoundationExporter.swift
@_exported import Foundation
//--- inlinable-public.swift
public import Foundation
@inlinable public func foo<E: Error>(e: E) -> Error {
return e
}
//--- inlinable-ioi.swift
@_implementationOnly import Foundation
@inlinable public func foo<E: Error>(e: E) -> Error {
return e
}
//--- inlinable-internal.swift
internal import Foundation
@inlinable public func foo<E: Error>(e: E) -> Error {
return e
}
//--- inlinable-not-imported-fileA.swift
@inlinable public func foo<E: Error>(e: E) -> Error {
return e
}
//--- inlinable-not-imported-fileB.swift
import Foundation
//--- inlinable-imported-transitive.swift
public import FoundationExporter
@inlinable public func foo<E: Error>(e: E) -> Error {
return e
}
//--- non-inlinable-public.swift
public import Foundation
public func foo<E: Error>(e: E) -> Error {
return e
}
//--- non-inlinable-ioi.swift
@_implementationOnly import Foundation
public func foo<E: Error>(e: E) -> Error {
return e
}
//--- non-inlinable-internal.swift
internal import Foundation
public func foo<E: Error>(e: E) -> Error {
return e
}
//--- non-inlinable-not-imported-fileA.swift
public func foo<E: Error>(e: E) -> Error {
return e
}
//--- non-inlinable-not-imported-fileB.swift
import Foundation