From 8efc7414f0f71a2674c45a17c717f6304ca5fe35 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 20 Aug 2024 09:33:10 -0700 Subject: [PATCH] [Module] Use stricter module hash Use a stricter module hash for modules as moving towards more explicit modules. Previously, a normalized target triple without deployment target is used as part of the module hash, with the assumption that deployment target should not change binary module generated. In reality, the binary module compilation also needs to provide the typecheck for all interface to ensure the underlying C/ObjC module that built with the correct deployment target does provide the interfaces to pass typecheck. Using a stricter module hash can avoid the potentially confusing error when importing the module. This commit adds following to module hash to distinguish swiftmodules. * Deployment target * -application-extension rdar://134301179 --- lib/Frontend/ModuleInterfaceLoader.cpp | 33 +++++++++---- test/ScanDependencies/module_hash.swift | 2 +- test/ScanDependencies/module_hash_macos.swift | 47 +++++++++++++++++++ 3 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 test/ScanDependencies/module_hash_macos.swift diff --git a/lib/Frontend/ModuleInterfaceLoader.cpp b/lib/Frontend/ModuleInterfaceLoader.cpp index 818c011a736..a0168460a27 100644 --- a/lib/Frontend/ModuleInterfaceLoader.cpp +++ b/lib/Frontend/ModuleInterfaceLoader.cpp @@ -2071,8 +2071,24 @@ StringRef InterfaceSubContextDelegateImpl::computeCachedOutputPath( std::string InterfaceSubContextDelegateImpl::getCacheHash(StringRef useInterfacePath, StringRef sdkPath) { - auto normalizedTargetTriple = - getTargetSpecificModuleTriple(genericSubInvocation.getLangOptions().Target); + // When doing dependency scanning for explicit module, use strict context hash + // to ensure sound module hash. + bool useStrictCacheHash = + genericSubInvocation.getFrontendOptions().RequestedAction == + FrontendOptions::ActionType::ScanDependencies; + + // Include the normalized target triple when not using strict hash. + // Otherwise, use the full target to ensure soundness of the hash. In + // practice, .swiftinterface files will be in target-specific subdirectories + // and would have target-specific pieces #if'd out. However, it doesn't hurt + // to include it, and it guards against mistakenly reusing cached modules + // across targets. Note that this normalization explicitly doesn't include the + // minimum deployment target (e.g. the '12.0' in 'ios12.0'). + auto targetToHash = useStrictCacheHash + ? genericSubInvocation.getLangOptions().Target + : getTargetSpecificModuleTriple( + genericSubInvocation.getLangOptions().Target); + std::string sdkBuildVersion = getSDKBuildVersion(sdkPath); const auto ExtraArgs = genericSubInvocation.getClangImporterOptions() .getReducedExtraArgsForSwiftModuleDependency(); @@ -2089,13 +2105,8 @@ InterfaceSubContextDelegateImpl::getCacheHash(StringRef useInterfacePath, // anyways. useInterfacePath, - // Include the normalized target triple. In practice, .swiftinterface - // files will be in target-specific subdirectories and would have - // target-specific pieces #if'd out. However, it doesn't hurt to include - // it, and it guards against mistakenly reusing cached modules across - // targets. Note that this normalization explicitly doesn't include the - // minimum deployment target (e.g. the '12.0' in 'ios12.0'). - normalizedTargetTriple.str(), + // The target triple to hash. + targetToHash.str(), // The SDK path is going to affect how this module is imported, so // include it. @@ -2121,6 +2132,10 @@ InterfaceSubContextDelegateImpl::getCacheHash(StringRef useInterfacePath, // module. llvm::hash_combine_range(ExtraArgs.begin(), ExtraArgs.end()), + /// Application extension. + unsigned( + genericSubInvocation.getLangOptions().EnableAppExtensionRestrictions), + // Whether or not OSSA modules are enabled. // // If OSSA modules are enabled, we use a separate namespace of modules to diff --git a/test/ScanDependencies/module_hash.swift b/test/ScanDependencies/module_hash.swift index 66f548ddbe3..88e17a18394 100644 --- a/test/ScanDependencies/module_hash.swift +++ b/test/ScanDependencies/module_hash.swift @@ -14,7 +14,7 @@ // RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \ // RUN: -o %t/deps-3.json -Xcc -fapplication-extension -I %t/include -/// Check module hash for the swiftmodule. 1 and 2 should match, but not 3. +/// Check module hash for the swiftmodule. They should all not match. // RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-1.json Library modulePath > %t/path-1 // RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-2.json Library modulePath > %t/path-2 // RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-3.json Library modulePath > %t/path-3 diff --git a/test/ScanDependencies/module_hash_macos.swift b/test/ScanDependencies/module_hash_macos.swift new file mode 100644 index 00000000000..8cac2f76a75 --- /dev/null +++ b/test/ScanDependencies/module_hash_macos.swift @@ -0,0 +1,47 @@ +// REQUIRES: OS=macosx, objc_interop +// RUN: %empty-directory(%t) +// RUN: split-file %s %t + +/// Different version should not match. +// RUN: %target-swift-frontend -target %module-target-triple -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \ +// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \ +// RUN: -o %t/deps-now.json -I %t/include +// RUN: %target-swift-frontend -target %module-target-future -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \ +// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \ +// RUN: -o %t/deps-future.json -I %t/include +// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-now.json Library modulePath > %t/path-now +// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-future.json Library modulePath > %t/path-future +// RUN: not diff %t/path-now %t/path-future + +/// Application extension should not match. +// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \ +// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \ +// RUN: -o %t/deps.json -I %t/include +// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \ +// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \ +// RUN: -o %t/deps-ae.json -I %t/include -application-extension +// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps.json Library modulePath > %t/path +// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-ae.json Library modulePath > %t/path-ae +// RUN: not diff %t/path %t/path-ae + +//--- main.swift +import Library + +//--- include/Library.swiftinterface +// swift-interface-format-version: 1.0 +// swift-module-flags: -module-name Library -O -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -user-module-version 1.0 +import Swift +@_exported import A +@inlinable +public func test() { + foo() +} + +//--- include/a.h +void foo(void); + +//--- include/module.modulemap +module A { + header "a.h" + export * +}