llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Jan Svoboda (jansvoboda11) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/139751.diff 3 Files Affected: - (modified) clang/lib/Serialization/ASTWriter.cpp (+46-13) - (added) clang/test/Modules/sdk-settings-json-dep.m (+49) - (modified) clang/tools/clang-scan-deps/ClangScanDeps.cpp (+10-2) ``````````diff 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)); `````````` </details> https://github.com/llvm/llvm-project/pull/139751 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits