Merge pull request #10679 from swiftlang/jan_svoboda/release-6.2-sdk-settings-json

🍒[clang][modules] Invalidate module cache when SDKSettings.json changes

Author: akyrtzi

clang/lib/Index/IndexingAction.cpp

@@ -919,6 +919,10 @@ class ModuleFileIndexDependencyCollector : public IndexDependencyProvider {
           // undesirable dependency on an intermediate build byproduct.
           if (FE->getName().ends_with("module.modulemap"))
             return;
+          // Ignore SDKSettings.json, they are not important to track for
+          // indexing.
+          if (FE->getName().ends_with("SDKSettings.json"))
+            return;
 
           visitor(*FE, isSystem);
         });

clang/lib/Serialization/ASTWriter.cpp

@@ -1771,6 +1771,29 @@ struct InputFileEntry {
   uint32_t ContentHash[2];
 
   InputFileEntry(FileEntryRef File) : File(File) {}
+
+  void trySetContentHash(
+      Preprocessor &PP,
+      llvm::function_ref<std::optional<llvm::MemoryBufferRef>()> GetMemBuff) {
+    ContentHash[0] = 0;
+    ContentHash[1] = 0;
+
+    if (!PP.getHeaderSearchInfo()
+             .getHeaderSearchOpts()
+             .ValidateASTInputFilesContent)
+      return;
+
+    auto MemBuff = GetMemBuff();
+    if (!MemBuff) {
+      PP.Diag(SourceLocation(), diag::err_module_unable_to_hash_content)
+          << File.getName();
+      return;
+    }
+
+    uint64_t Hash = xxh3_64bits(MemBuff->getBuffer());
+    ContentHash[0] = uint32_t(Hash);
+    ContentHash[1] = uint32_t(Hash >> 32);
+  }
 };
 
 } // namespace
@@ -1843,25 +1866,41 @@ void ASTWriter::WriteInputFiles(SourceManager &SourceMgr,
     Entry.IsTopLevel = getAffectingIncludeLoc(SourceMgr, File).isInvalid();
     Entry.IsModuleMap = isModuleMap(File.getFileCharacteristic());
 
-    uint64_t ContentHash = 0;
-    if (PP->getHeaderSearchInfo()
-            .getHeaderSearchOpts()
-            .ValidateASTInputFilesContent) {
-      auto MemBuff = Cache->getBufferIfLoaded();
-      if (MemBuff)
-        ContentHash = xxh3_64bits(MemBuff->getBuffer());
-      else
-        PP->Diag(SourceLocation(), diag::err_module_unable_to_hash_content)
-            << Entry.File.getName();
-    }
-    Entry.ContentHash[0] = uint32_t(ContentHash);
-    Entry.ContentHash[1] = uint32_t(ContentHash >> 32);
+    Entry.trySetContentHash(*PP, [&] { return Cache->getBufferIfLoaded(); });
+
     if (Entry.IsSystemFile)
       SystemFiles.push_back(Entry);
     else
       UserFiles.push_back(Entry);
   }
 
+  // FIXME: Make providing input files not in the SourceManager more flexible.
+  // The SDKSettings.json file is necessary for correct evaluation of
+  // availability annotations.
+  StringRef Sysroot = PP->getHeaderSearchInfo().getHeaderSearchOpts().Sysroot;
+  if (!Sysroot.empty()) {
+    SmallString<128> SDKSettingsJSON = Sysroot;
+    llvm::sys::path::append(SDKSettingsJSON, "SDKSettings.json");
+    FileManager &FM = PP->getFileManager();
+    if (auto FE = FM.getOptionalFileRef(SDKSettingsJSON)) {
+      InputFileEntry Entry(*FE);
+      Entry.IsSystemFile = true;
+      Entry.IsTransient = false;
+      Entry.BufferOverridden = false;
+      Entry.IsTopLevel = true;
+      Entry.IsModuleMap = false;
+      std::unique_ptr<MemoryBuffer> MB;
+      Entry.trySetContentHash(*PP, [&]() -> std::optional<MemoryBufferRef> {
+        if (auto MBOrErr = FM.getBufferForFile(Entry.File)) {
+          MB = std::move(*MBOrErr);
+          return MB->getMemBufferRef();
+        }
+        return std::nullopt;
+      });
+      SystemFiles.push_back(Entry);
+    }
+  }
+
   // User files go at the front, system files at the back.
   auto SortedFiles = llvm::concat<InputFileEntry>(std::move(UserFiles),
                                                   std::move(SystemFiles));

clang/test/ClangScanDeps/modules-include-tree-sdk-settings.c

@@ -0,0 +1,37 @@
+// This test checks that the module cache gets invalidated when the
+// SDKSettings.json file changes. This prevents "file changed during build"
+// errors when the TU does get rescanned and recompiled.
+
+// REQUIRES: ondisk_cas
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- sdk/SDKSettings.json
+{
+  "Version": "11.0",
+  "MaximumDeploymentTarget": "11.0.99"
+}
+
+//--- module.modulemap
+module M { header "M.h" }
+//--- M.h
+//--- tu.c
+#include "M.h"
+
+// RUN: clang-scan-deps -format experimental-include-tree-full -cas-path %t/cas -o %t/deps_clean.json \
+// RUN:   -- %clang -target x86_64-apple-macos11 -isysroot %t/sdk \
+// RUN:      -c %t/tu.c -o %t/tu.o -fmodules -fmodules-cache-path=%t/cache
+
+// RUN: sleep 1
+// RUN: echo " " >> %t/sdk/SDKSettings.json
+// RUN: echo " " >> %t/tu.c
+
+// RUN: clang-scan-deps -format experimental-include-tree-full -cas-path %t/cas -o %t/deps_incremental.json \
+// RUN:   -- %clang -target x86_64-apple-macos11 -isysroot %t/sdk \
+// RUN:      -c %t/tu.c -o %t/tu.o -fmodules -fmodules-cache-path=%t/cache
+
+// RUN: %deps-to-rsp %t/deps_incremental.json --module-name M > %t/M.rsp
+// RUN: %deps-to-rsp %t/deps_incremental.json --tu-index 0 > %t/tu.rsp
+// RUN: %clang @%t/M.rsp
+// RUN: %clang @%t/tu.rsp

clang/test/Modules/sdk-settings-json-dep.m

@@ -0,0 +1,53 @@
+// This test checks that the module cache gets invalidated when the SDKSettings.json file changes.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- AppleTVOS15.0.sdk/SDKSettings-old.json
+{
+  "DisplayName": "tvOS 15.0",
+  "Version": "15.0",
+  "CanonicalName": "appletvos15.0",
+  "MaximumDeploymentTarget": "15.0.99",
+  "PropertyConditionFallbackNames": []
+}
+//--- AppleTVOS15.0.sdk/SDKSettings-new.json
+{
+  "DisplayName": "tvOS 15.0",
+  "Version": "15.0",
+  "CanonicalName": "appletvos15.0",
+  "MaximumDeploymentTarget": "15.0.99",
+  "PropertyConditionFallbackNames": [],
+  "VersionMap": {
+    "iOS_tvOS": {
+      "13.2": "13.1"
+    },
+    "tvOS_iOS": {
+      "13.1": "13.2"
+    }
+  }
+}
+//--- module.modulemap
+module M { header "M.h" }
+//--- M.h
+void foo(void) __attribute__((availability(iOS, obsoleted = 13.2)));
+void test() { foo(); }
+
+//--- tu.m
+#include "M.h"
+
+// Compiling for tvOS 13.1 without "VersionMap" should succeed, since by default iOS 13.2 gets mapped to tvOS 13.2,
+// and \c foo is therefore **not** deprecated.
+// RUN: cp %t/AppleTVOS15.0.sdk/SDKSettings-old.json %t/AppleTVOS15.0.sdk/SDKSettings.json
+// RUN: %clang -target x86_64-apple-tvos13.1 -isysroot %t/AppleTVOS15.0.sdk \
+// RUN:   -fsyntax-only %t/tu.m -o %t/tu.o -fmodules -Xclang -fdisable-module-hash -fmodules-cache-path=%t/cache
+
+// Compiling for tvOS 13.1 with "VersionMap" saying it maps to iOS 13.2 should fail, since \c foo is now deprecated.
+// RUN: sleep 1
+// RUN: cp %t/AppleTVOS15.0.sdk/SDKSettings-new.json %t/AppleTVOS15.0.sdk/SDKSettings.json
+// RUN: not %clang -target x86_64-apple-tvos13.1 -isysroot %t/AppleTVOS15.0.sdk \
+// RUN:   -fsyntax-only %t/tu.m -o %t/tu.o -fmodules -Xclang -fdisable-module-hash -fmodules-cache-path=%t/cache 2>&1 \
+// RUN:     | FileCheck %s
+// CHECK: M.h:2:15: error: 'foo' is unavailable: obsoleted in tvOS 13.1
+// CHECK: M.h:1:6: note: 'foo' has been explicitly marked unavailable here
+// CHECK: tu.m:1:10: fatal error: could not build module 'M'

clang/tools/c-index-test/core_main.cpp

@@ -850,7 +850,10 @@ static int scanDeps(ArrayRef<const char *> Args, std::string WorkingDirectory,
         llvm::outs() << "      " << ModuleName << "\n";
       llvm::outs() << "    file-deps:\n";
       for (const auto &FileName : ArrayRef(FileDeps.Strings, FileDeps.Count))
-        llvm::outs() << "      " << FileName << "\n";
+        // Not reporting SDKSettings.json so that test checks can remain
+        // (mostly) platform-agnostic.
+        if (!StringRef(FileName).ends_with("SDKSettings.json"))
+          llvm::outs() << "      " << FileName << "\n";
       llvm::outs() << "    build-args:";
       for (const auto &Arg :
            ArrayRef(BuildArguments.Strings, BuildArguments.Count))

clang/tools/clang-scan-deps/ClangScanDeps.cpp

@@ -597,7 +597,10 @@ template <typename Container>
 static auto toJSONStrings(llvm::json::OStream &JOS, Container &&Strings) {
   return [&JOS, Strings = std::forward<Container>(Strings)] {
     for (StringRef Str : Strings)
-      JOS.value(Str);
+      // Not reporting SDKSettings.json so that test checks can remain (mostly)
+      // platform-agnostic.
+      if (!Str.ends_with("SDKSettings.json"))
+        JOS.value(Str);
   };
 }
 
@@ -743,7 +746,12 @@ class FullDeps {
                                toJSONStrings(JOS, MD.getBuildArguments()));
             JOS.attribute("context-hash", StringRef(MD.ID.ContextHash));
             JOS.attributeArray("file-deps", [&] {
-              MD.forEachFileDep([&](StringRef FileDep) { JOS.value(FileDep); });
+              MD.forEachFileDep([&](StringRef FileDep) {
+                // Not reporting SDKSettings.json so that test checks can remain
+                // (mostly) platform-agnostic.
+                if (!FileDep.ends_with("SDKSettings.json"))
+                  JOS.value(FileDep);
+              });
             });
             JOS.attributeArray("link-libraries",
                                toJSONSorted(JOS, MD.LinkLibraries));