[clang][modules] Invalidate module cache when SDKSettings.json changes #139751
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-modules Author: Jan Svoboda (jansvoboda11) ChangesThis PR adds the Full diff: https://github.com/llvm/llvm-project/pull/139751.diff 3 Files Affected:
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index cccf53de25882..48595ae8fabfa 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1774,6 +1774,27 @@ struct InputFileEntry {
uint32_t ContentHash[2];
InputFileEntry(FileEntryRef File) : File(File) {}
+
+ void trySetContentHash(Preprocessor &PP,
+ std::optional<llvm::MemoryBufferRef> MemBuff) {
+ ContentHash[0] = 0;
+ ContentHash[1] = 0;
+
+ if (!PP.getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .ValidateASTInputFilesContent)
+ return;
+
+ 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
@@ -1848,25 +1869,37 @@ void ASTWriter::WriteInputFiles(SourceManager &SourceMgr) {
!IsSLocFileEntryAffecting[IncludeFileID.ID];
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, 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;
+ auto Convert = [](const ErrorOr<std::unique_ptr<MemoryBuffer>> &MB) {
+ return MB ? std::optional((*MB)->getMemBufferRef()) : std::nullopt;
+ };
+ Entry.trySetContentHash(*PP, Convert(FM.getBufferForFile(Entry.File)));
+ 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));
diff --git a/clang/test/Modules/sdk-settings-json-dep.m b/clang/test/Modules/sdk-settings-json-dep.m
new file mode 100644
index 0000000000000..e7cbe760a1eb6
--- /dev/null
+++ b/clang/test/Modules/sdk-settings-json-dep.m
@@ -0,0 +1,49 @@
+// 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
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index dae2b9a9fe683..3b42267f4d5f4 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -346,7 +346,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);
};
}
@@ -498,7 +501,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));
|
|
@llvm/pr-subscribers-clang Author: Jan Svoboda (jansvoboda11) ChangesThis PR adds the Full diff: https://github.com/llvm/llvm-project/pull/139751.diff 3 Files Affected:
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index cccf53de25882..48595ae8fabfa 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1774,6 +1774,27 @@ struct InputFileEntry {
uint32_t ContentHash[2];
InputFileEntry(FileEntryRef File) : File(File) {}
+
+ void trySetContentHash(Preprocessor &PP,
+ std::optional<llvm::MemoryBufferRef> MemBuff) {
+ ContentHash[0] = 0;
+ ContentHash[1] = 0;
+
+ if (!PP.getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .ValidateASTInputFilesContent)
+ return;
+
+ 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
@@ -1848,25 +1869,37 @@ void ASTWriter::WriteInputFiles(SourceManager &SourceMgr) {
!IsSLocFileEntryAffecting[IncludeFileID.ID];
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, 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;
+ auto Convert = [](const ErrorOr<std::unique_ptr<MemoryBuffer>> &MB) {
+ return MB ? std::optional((*MB)->getMemBufferRef()) : std::nullopt;
+ };
+ Entry.trySetContentHash(*PP, Convert(FM.getBufferForFile(Entry.File)));
+ 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));
diff --git a/clang/test/Modules/sdk-settings-json-dep.m b/clang/test/Modules/sdk-settings-json-dep.m
new file mode 100644
index 0000000000000..e7cbe760a1eb6
--- /dev/null
+++ b/clang/test/Modules/sdk-settings-json-dep.m
@@ -0,0 +1,49 @@
+// 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
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index dae2b9a9fe683..3b42267f4d5f4 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -346,7 +346,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);
};
}
@@ -498,7 +501,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));
|
benlangmuir
approved these changes
May 13, 2025
jansvoboda11
added a commit
to swiftlang/llvm-project
that referenced
this pull request
May 13, 2025
…n-invalidation [clang][modules] Test for llvm#139751
jansvoboda11
added a commit
to swiftlang/llvm-project
that referenced
this pull request
May 14, 2025
llvm#139751) This PR adds the `%sdk/SDKSettings.json` file to the PCM input file table, so that the PCM gets invalidated when the file changes. This is necessary for availability checks to work correctly.
jansvoboda11
added a commit
to swiftlang/llvm-project
that referenced
this pull request
May 14, 2025
The input file section in module files only stored files loaded into the `SourceManager`. When rebuilding the module cache, include-trees also include other files, like the SDKSettings.json file. If we don't invalidate the module cache when that file changes, the corresponding include-trees won't agree with the primary TU include-tree on the file contents. This was fixed in llvm#139751 and this PR adds an include tree test. I intentionally suppress reporting of this new file in tests, so that I don't have go updating ~all of them. This file is getting reported by the scanning C API, so that the build system is given the ability to eventually act on this file being out-of-date. rdar://149868539
jansvoboda11
added a commit
to swiftlang/llvm-project
that referenced
this pull request
May 14, 2025
This fixes the Index/Store/handle-prebuilt-module.m test that started failing after llvm#139751.
jansvoboda11
added a commit
to swiftlang/llvm-project
that referenced
this pull request
May 14, 2025
This fixes the Index/Store/handle-prebuilt-module.m test that started failing after llvm#139751.
jansvoboda11
added a commit
to swiftlang/llvm-project
that referenced
this pull request
May 19, 2025
llvm#139751) This PR adds the `%sdk/SDKSettings.json` file to the PCM input file table, so that the PCM gets invalidated when the file changes. This is necessary for availability checks to work correctly.
jansvoboda11
added a commit
to swiftlang/llvm-project
that referenced
this pull request
May 19, 2025
The input file section in module files only stored files loaded into the `SourceManager`. When rebuilding the module cache, include-trees also include other files, like the SDKSettings.json file. If we don't invalidate the module cache when that file changes, the corresponding include-trees won't agree with the primary TU include-tree on the file contents. This was fixed in llvm#139751 and this PR adds an include tree test. I intentionally suppress reporting of this new file in tests, so that I don't have go updating ~all of them. This file is getting reported by the scanning C API, so that the build system is given the ability to eventually act on this file being out-of-date. rdar://149868539
jansvoboda11
added a commit
to swiftlang/llvm-project
that referenced
this pull request
May 19, 2025
This fixes the Index/Store/handle-prebuilt-module.m test that started failing after llvm#139751.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the
%sdk/SDKSettings.jsonfile to the PCM input file table, so that the PCM gets invalidated when the file changes. This is necessary for availability checks to work correctly.