From ca3f33cc1e2f6dd12eeeb4ea54e880ff4a456b94 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 13 May 2025 07:49:52 -0700 Subject: [PATCH 1/4] [clang][modules] Invalidate module cache when SDKSettings.json changes --- clang/lib/Serialization/ASTWriter.cpp | 59 +++++++++++++++---- clang/test/Modules/sdk-settings-json-dep.m | 49 +++++++++++++++ clang/tools/clang-scan-deps/ClangScanDeps.cpp | 12 +++- 3 files changed, 105 insertions(+), 15 deletions(-) create mode 100644 clang/test/Modules/sdk-settings-json-dep.m 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 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> &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(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 static auto toJSONStrings(llvm::json::OStream &JOS, Container &&Strings) { return [&JOS, Strings = std::forward(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)); From 12f3bbc99ce79765539b80ca10ac1c9dc173d484 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 13 May 2025 08:30:07 -0700 Subject: [PATCH 2/4] Avoid opening file with content validation disabled --- clang/lib/Serialization/ASTWriter.cpp | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 48595ae8fabfa..8ed2140cb95bd 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1775,8 +1775,9 @@ struct InputFileEntry { InputFileEntry(FileEntryRef File) : File(File) {} - void trySetContentHash(Preprocessor &PP, - std::optional MemBuff) { + void trySetContentHash( + Preprocessor &PP, + llvm::function_ref()> GetMemBuff) { ContentHash[0] = 0; ContentHash[1] = 0; @@ -1785,6 +1786,7 @@ struct InputFileEntry { .ValidateASTInputFilesContent) return; + auto MemBuff = GetMemBuff(); if (!MemBuff) { PP.Diag(SourceLocation(), diag::err_module_unable_to_hash_content) << File.getName(); @@ -1869,7 +1871,7 @@ void ASTWriter::WriteInputFiles(SourceManager &SourceMgr) { !IsSLocFileEntryAffecting[IncludeFileID.ID]; Entry.IsModuleMap = isModuleMap(File.getFileCharacteristic()); - Entry.trySetContentHash(*PP, Cache->getBufferIfLoaded()); + Entry.trySetContentHash(*PP, [&] { return Cache->getBufferIfLoaded(); }); if (Entry.IsSystemFile) SystemFiles.push_back(Entry); @@ -1892,10 +1894,14 @@ void ASTWriter::WriteInputFiles(SourceManager &SourceMgr) { Entry.BufferOverridden = false; Entry.IsTopLevel = true; Entry.IsModuleMap = false; - auto Convert = [](const ErrorOr> &MB) { - return MB ? std::optional((*MB)->getMemBufferRef()) : std::nullopt; - }; - Entry.trySetContentHash(*PP, Convert(FM.getBufferForFile(Entry.File))); + std::unique_ptr MB; + Entry.trySetContentHash(*PP, [&] -> std::optional { + if (auto MBOrErr = FM.getBufferForFile(Entry.File)) { + MB = std::move(*MBOrErr); + return MB->getMemBufferRef(); + } + return std::nullopt; + }); SystemFiles.push_back(Entry); } } From 406357f715b6e8de150b449a6d22718431ac8439 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 13 May 2025 09:50:29 -0700 Subject: [PATCH 3/4] Check for error messages --- clang/test/Modules/sdk-settings-json-dep.m | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clang/test/Modules/sdk-settings-json-dep.m b/clang/test/Modules/sdk-settings-json-dep.m index e7cbe760a1eb6..196f4219bd989 100644 --- a/clang/test/Modules/sdk-settings-json-dep.m +++ b/clang/test/Modules/sdk-settings-json-dep.m @@ -46,4 +46,8 @@ // 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 +// 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' From ef761924a06b9cbf7247b19e39744b5c0873d9ac Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 13 May 2025 09:52:57 -0700 Subject: [PATCH 4/4] Don't omit `()` in lambda with explicit return type --- clang/lib/Serialization/ASTWriter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 8ed2140cb95bd..fa0efc9ce8a41 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1895,7 +1895,7 @@ void ASTWriter::WriteInputFiles(SourceManager &SourceMgr) { Entry.IsTopLevel = true; Entry.IsModuleMap = false; std::unique_ptr MB; - Entry.trySetContentHash(*PP, [&] -> std::optional { + Entry.trySetContentHash(*PP, [&]() -> std::optional { if (auto MBOrErr = FM.getBufferForFile(Entry.File)) { MB = std::move(*MBOrErr); return MB->getMemBufferRef();