Serialization: apply the SDK build version as part of the cache hash

xymus · xymus · commit 86ea594f21f7 · 2024-02-12T12:35:03.000-08:00
The SDK build version is a decent heuristic for expected changes in the
SDK. Any change in SDK, to clang headers in particular, can break
references from cached swiftmodules.

Track the SDK build version as part of the swiftmodule cache hash. This
will ensure we rebuild from swiftinterfaces on SDK updates.

rdar://122655978
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(`