Merge pull request #71563 from xymus/sdk-version-in-hash

xymus · web-flow · commit 2875980d01ee · 2024-02-12T16:45:55.000-08:00
Serialization: Apply the SDK build version as part of the swiftmodule cache hash
diff --git a/include/swift/AST/ModuleLoader.h b/include/swift/AST/ModuleLoader.h
@@ -209,13 +209,15 @@ class ModuleInterfaceChecker {
 struct InterfaceSubContextDelegate {
   virtual std::error_code runInSubContext(StringRef moduleName,
                                           StringRef interfacePath,
+                                          StringRef sdkPath,
                                           StringRef outputPath,
                                           SourceLoc diagLoc,
     llvm::function_ref<std::error_code(ASTContext&, ModuleDecl*,
                                        ArrayRef<StringRef>,
                                        ArrayRef<StringRef>, StringRef)> action) = 0;
   virtual std::error_code runInSubCompilerInstance(StringRef moduleName,
                                                    StringRef interfacePath,
+                                                   StringRef sdkPath,
                                                    StringRef outputPath,
                                                    SourceLoc diagLoc,
                                                    bool silenceErrors,
diff --git a/include/swift/Frontend/ModuleInterfaceLoader.h b/include/swift/Frontend/ModuleInterfaceLoader.h
@@ -693,13 +693,15 @@ struct InterfaceSubContextDelegateImpl: InterfaceSubContextDelegate {
 
   std::error_code runInSubContext(StringRef moduleName,
                                   StringRef interfacePath,
+                                  StringRef sdkPath,
                                   StringRef outputPath,
                                   SourceLoc diagLoc,
     llvm::function_ref<std::error_code(ASTContext&, ModuleDecl*,
                                        ArrayRef<StringRef>, ArrayRef<StringRef>,
                                        StringRef)> action) override;
   std::error_code runInSubCompilerInstance(StringRef moduleName,
                                            StringRef interfacePath,
+                                           StringRef sdkPath,
                                            StringRef outputPath,
                                            SourceLoc diagLoc,
                                            bool silenceErrors,
@@ -710,9 +712,10 @@ struct InterfaceSubContextDelegateImpl: InterfaceSubContextDelegate {
   /// includes a hash of relevant key data.
   StringRef computeCachedOutputPath(StringRef moduleName,
                                     StringRef UseInterfacePath,
+                                    StringRef sdkPath,
                                     llvm::SmallString<256> &OutPath,
                                     StringRef &CacheHash);
-  std::string getCacheHash(StringRef useInterfacePath);
+  std::string getCacheHash(StringRef useInterfacePath, StringRef sdkPath);
 };
 }
 
diff --git a/lib/Frontend/ModuleInterfaceBuilder.cpp b/lib/Frontend/ModuleInterfaceBuilder.cpp
@@ -345,7 +345,7 @@ bool ImplicitModuleInterfaceBuilder::buildSwiftModuleInternal(
     }
 
     SubError = (bool)subASTDelegate.runInSubCompilerInstance(
-        moduleName, interfacePath, OutPath, diagnosticLoc,
+        moduleName, interfacePath, sdkPath, OutPath, diagnosticLoc,
         silenceInterfaceDiagnostics,
         [&](SubCompilerInstanceInfo &info) {
           auto EBuilder = ExplicitModuleInterfaceBuilder(
diff --git a/lib/Frontend/ModuleInterfaceBuilder.h b/lib/Frontend/ModuleInterfaceBuilder.h
@@ -41,6 +41,7 @@ class ImplicitModuleInterfaceBuilder {
   DiagnosticEngine *diags;
   InterfaceSubContextDelegate &subASTDelegate;
   const StringRef interfacePath;
+  const StringRef sdkPath;
   const StringRef moduleName;
   const StringRef moduleCachePath;
   const StringRef prebuiltCachePath;
@@ -84,15 +85,16 @@ class ImplicitModuleInterfaceBuilder {
 public:
   ImplicitModuleInterfaceBuilder(
       SourceManager &sourceMgr, DiagnosticEngine *diags,
-      InterfaceSubContextDelegate &subASTDelegate, StringRef interfacePath,
+      InterfaceSubContextDelegate &subASTDelegate,
+      StringRef interfacePath, StringRef sdkPath,
       StringRef moduleName, StringRef moduleCachePath,
       StringRef backupInterfaceDir, StringRef prebuiltCachePath,
       StringRef ABIDescriptorPath, bool disableInterfaceFileLock = false,
       bool silenceInterfaceDiagnostics = false,
       SourceLoc diagnosticLoc = SourceLoc(),
       DependencyTracker *tracker = nullptr)
       : sourceMgr(sourceMgr), diags(diags), subASTDelegate(subASTDelegate),
-        interfacePath(interfacePath), moduleName(moduleName),
+        interfacePath(interfacePath), sdkPath(sdkPath), moduleName(moduleName),
         moduleCachePath(moduleCachePath), prebuiltCachePath(prebuiltCachePath),
         backupInterfaceDir(backupInterfaceDir),
         ABIDescriptorPath(ABIDescriptorPath),
diff --git a/lib/Frontend/ModuleInterfaceLoader.cpp b/lib/Frontend/ModuleInterfaceLoader.cpp
@@ -1064,6 +1064,7 @@ class ModuleInterfaceLoaderImpl {
     llvm::SmallString<256> cachedOutputPath;
     StringRef CacheHash;
     astDelegate.computeCachedOutputPath(moduleName, interfacePath,
+                                        ctx.SearchPathOpts.getSDKPath(),
                                         cachedOutputPath, CacheHash);
 
     // Try to find the right module for this interface, either alongside it,
@@ -1144,7 +1145,9 @@ class ModuleInterfaceLoaderImpl {
       Identifier realName = ctx.getRealModuleName(ctx.getIdentifier(moduleName));
       ImplicitModuleInterfaceBuilder builder(
         ctx.SourceMgr, diagsToUse,
-        astDelegate, interfacePath, realName.str(), cacheDir,
+        astDelegate, interfacePath,
+        ctx.SearchPathOpts.getSDKPath(),
+        realName.str(), cacheDir,
         prebuiltCacheDir, backupInterfaceDir, StringRef(),
         Opts.disableInterfaceLock,
         ctx.IgnoreAdjacentModules, diagnosticLoc,
@@ -1177,7 +1180,9 @@ class ModuleInterfaceLoaderImpl {
       // Set up a builder if we need to build the module. It'll also set up
       // the genericSubInvocation we'll need to use to compute the cache paths.
       ImplicitModuleInterfaceBuilder fallbackBuilder(
-        ctx.SourceMgr, &ctx.Diags, astDelegate, backupPath, moduleName, cacheDir,
+        ctx.SourceMgr, &ctx.Diags, astDelegate, backupPath,
+        ctx.SearchPathOpts.getSDKPath(),
+        moduleName, cacheDir,
         prebuiltCacheDir, backupInterfaceDir, StringRef(),
         Opts.disableInterfaceLock,
         ctx.IgnoreAdjacentModules, diagnosticLoc,
@@ -1390,6 +1395,7 @@ bool ModuleInterfaceLoader::buildSwiftModuleFromSwiftInterface(
       SerializeDependencyHashes, TrackSystemDependencies,
       RequireOSSAModules, RequireNCGenerics);
   ImplicitModuleInterfaceBuilder builder(SourceMgr, &Diags, astDelegate, InPath,
+                                         SearchPathOpts.getSDKPath(),
                                          ModuleName, CacheDir, PrebuiltCacheDir,
                                          BackupInterfaceDir, ABIOutputPath,
                                          LoaderOpts.disableInterfaceLock,
@@ -1409,6 +1415,7 @@ bool ModuleInterfaceLoader::buildSwiftModuleFromSwiftInterface(
   assert(failed);
   assert(!backInPath.empty());
   ImplicitModuleInterfaceBuilder backupBuilder(SourceMgr, &Diags, astDelegate, backInPath,
+                                               SearchPathOpts.getSDKPath(),
                                                ModuleName, CacheDir, PrebuiltCacheDir,
                                                BackupInterfaceDir, ABIOutputPath,
                                                LoaderOpts.disableInterfaceLock,
@@ -1865,13 +1872,14 @@ InterfaceSubContextDelegateImpl::InterfaceSubContextDelegateImpl(
 StringRef InterfaceSubContextDelegateImpl::computeCachedOutputPath(
                              StringRef moduleName,
                              StringRef useInterfacePath,
+                             StringRef sdkPath,
                              llvm::SmallString<256> &OutPath,
                              StringRef &CacheHash) {
   OutPath = genericSubInvocation.getClangModuleCachePath();
   llvm::sys::path::append(OutPath, moduleName);
   OutPath.append("-");
   auto hashStart = OutPath.size();
-  OutPath.append(getCacheHash(useInterfacePath));
+  OutPath.append(getCacheHash(useInterfacePath, sdkPath));
   CacheHash = OutPath.str().substr(hashStart);
   OutPath.append(".");
   auto OutExt = file_types::getExtension(file_types::TY_SwiftModuleFile);
@@ -1890,9 +1898,11 @@ StringRef InterfaceSubContextDelegateImpl::computeCachedOutputPath(
 /// with dead entries -- when other factors change, such as the contents of
 /// the .swiftinterface input or its dependencies.
 std::string
-InterfaceSubContextDelegateImpl::getCacheHash(StringRef useInterfacePath) {
+InterfaceSubContextDelegateImpl::getCacheHash(StringRef useInterfacePath,
+                                              StringRef sdkPath) {
   auto normalizedTargetTriple =
       getTargetSpecificModuleTriple(genericSubInvocation.getLangOptions().Target);
+  std::string sdkBuildVersion = getSDKBuildVersion(sdkPath);
 
   llvm::hash_code H = hash_combine(
       // Start with the compiler version (which will be either tag names or
@@ -1918,6 +1928,10 @@ InterfaceSubContextDelegateImpl::getCacheHash(StringRef useInterfacePath) {
       // include it.
       genericSubInvocation.getSDKPath(),
 
+      // The SDK build version may identify differences in headers
+      // that affects references serialized in the cached file.
+      sdkBuildVersion,
+
       // Whether or not we're tracking system dependencies affects the
       // invalidation behavior of this cache item.
       genericSubInvocation.getFrontendOptions().shouldTrackSystemDependencies(),
@@ -1934,11 +1948,12 @@ InterfaceSubContextDelegateImpl::getCacheHash(StringRef useInterfacePath) {
 std::error_code
 InterfaceSubContextDelegateImpl::runInSubContext(StringRef moduleName,
                                                  StringRef interfacePath,
+                                                 StringRef sdkPath,
                                                  StringRef outputPath,
                                                  SourceLoc diagLoc,
     llvm::function_ref<std::error_code(ASTContext&, ModuleDecl*, ArrayRef<StringRef>,
                             ArrayRef<StringRef>, StringRef)> action) {
-  return runInSubCompilerInstance(moduleName, interfacePath, outputPath,
+  return runInSubCompilerInstance(moduleName, interfacePath, sdkPath, outputPath,
                                   diagLoc, /*silenceErrors=*/false,
                                   [&](SubCompilerInstanceInfo &info){
     return action(info.Instance->getASTContext(),
@@ -1952,6 +1967,7 @@ InterfaceSubContextDelegateImpl::runInSubContext(StringRef moduleName,
 std::error_code
 InterfaceSubContextDelegateImpl::runInSubCompilerInstance(StringRef moduleName,
                                                           StringRef interfacePath,
+                                                          StringRef sdkPath,
                                                           StringRef outputPath,
                                                           SourceLoc diagLoc,
                                                           bool silenceErrors,
@@ -1981,8 +1997,8 @@ InterfaceSubContextDelegateImpl::runInSubCompilerInstance(StringRef moduleName,
   // Calculate output path of the module.
   llvm::SmallString<256> buffer;
   StringRef CacheHash;
-  auto hashedOutput = computeCachedOutputPath(moduleName, interfacePath, buffer,
-                                              CacheHash);
+  auto hashedOutput = computeCachedOutputPath(moduleName, interfacePath,
+                                              sdkPath, buffer, CacheHash);
   // If no specific output path is given, use the hashed output path.
   if (outputPath.empty()) {
     outputPath = hashedOutput;
diff --git a/lib/Serialization/ScanningLoaders.cpp b/lib/Serialization/ScanningLoaders.cpp
@@ -154,11 +154,13 @@ SwiftModuleScanner::scanInterfaceFile(Twine moduleInterfacePath,
   // name for the module, which includes an appropriate hash.
   auto newExt = file_types::getExtension(file_types::TY_SwiftModuleFile);
   auto realModuleName = Ctx.getRealModuleName(moduleName);
+  StringRef sdkPath = Ctx.SearchPathOpts.getSDKPath();
   llvm::SmallString<32> modulePath = realModuleName.str();
   llvm::sys::path::replace_extension(modulePath, newExt);
   llvm::Optional<ModuleDependencyInfo> Result;
   std::error_code code = astDelegate.runInSubContext(
-      realModuleName.str(), moduleInterfacePath.str(), StringRef(), SourceLoc(),
+      realModuleName.str(), moduleInterfacePath.str(), sdkPath,
+      StringRef(), SourceLoc(),
       [&](ASTContext &Ctx, ModuleDecl *mainMod, ArrayRef<StringRef> BaseArgs,
           ArrayRef<StringRef> PCMArgs, StringRef Hash) {
         assert(mainMod);
diff --git a/test/ModuleInterface/ModuleCache/module-cache-sdk-build-version.swift b/test/ModuleInterface/ModuleCache/module-cache-sdk-build-version.swift
@@ -0,0 +1,71 @@
+/// ProductBuildVersion of the SDK is tracked as part of the module cache hash.
+
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/ModuleCache)
+// RUN: %empty-directory(%t/sdk/System/Library/CoreServices/)
+// RUN: split-file %s %t
+
+/// Setup an "old" SDK.
+// RUN: cp %t/SystemVersion.plist.old %t/sdk/System/Library/CoreServices/SystemVersion.plist
+
+/// Build Lib against the old SDK.
+// RUN: %target-swift-frontend -emit-module -sdk %t/sdk %t/Lib.swift \
+// RUN:   -enable-library-evolution -swift-version 5 \
+// RUN:   -emit-module-path %t/Lib.swiftmodule \
+// RUN:   -emit-module-interface-path %t/Lib.swiftinterface
+
+/// Baseline check, we should read the adjacent swiftmodule.
+// RUN: %target-swift-frontend -typecheck -verify -sdk %t/sdk -I %t \
+// RUN:   %t/Client_NoRebuild.swift \
+// RUN:   -Rmodule-interface-rebuild -module-cache-path %t/ModuleCache
+
+/// Keep only the swiftinterface.
+// RUN: rm %t/Lib.swiftmodule
+
+/// Build client, which should trigger a build from swiftinterface.
+// RUN: %target-swift-frontend -typecheck -verify -sdk %t/sdk -I %t \
+// RUN:   %t/Client.swift \
+// RUN:   -Rmodule-interface-rebuild -module-cache-path %t/ModuleCache
+
+/// Update SDK.
+// RUN: cp %t/SystemVersion.plist.new %t/sdk/System/Library/CoreServices/SystemVersion.plist
+
+/// Build client, which should trigger a build from swiftinterface.
+// RUN: %target-swift-frontend -typecheck -verify -sdk %t/sdk -I %t \
+// RUN:   %t/Client.swift \
+// RUN:   -Rmodule-interface-rebuild -module-cache-path %t/ModuleCache
+
+/// Baseline check, we should reused the newly cached swiftmodule.
+// RUN: %target-swift-frontend -typecheck -verify -sdk %t/sdk -I %t \
+// RUN:   %t/Client_NoRebuild.swift \
+// RUN:   -Rmodule-interface-rebuild -module-cache-path %t/ModuleCache
+
+//--- SystemVersion.plist.old
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>ProductBuildVersion</key>
+	<string>10A100</string>
+</dict>
+</plist>
+
+//--- SystemVersion.plist.new
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>ProductBuildVersion</key>
+	<string>10A200</string>
+</dict>
+</plist>
+
+//--- Lib.swift
+public func foo() {}
+
+//--- Client.swift
+import Lib // expected-remark {{rebuilding module 'Lib' from interface}}
+
+//--- Client_NoRebuild.swift
+import Lib
+

Original file line number	Diff line number	Diff line change
`@@ -345,7 +345,7 @@ bool ImplicitModuleInterfaceBuilder::buildSwiftModuleInternal(`
`345`	`345`	`}`
`346`	`346`
`347`	`347`	`SubError = (bool)subASTDelegate.runInSubCompilerInstance(`
`348`		`- moduleName, interfacePath, OutPath, diagnosticLoc,`
	`348`	`+ moduleName, interfacePath, sdkPath, OutPath, diagnosticLoc,`
`349`	`349`	`silenceInterfaceDiagnostics,`
`350`	`350`	`[&](SubCompilerInstanceInfo &info) {`
`351`	`351`	`auto EBuilder = ExplicitModuleInterfaceBuilder(`