Author: Volodymyr Sapsai Date: 2026-01-21T21:31:53-08:00 New Revision: f2a3079a1b48033a92d0a7d9f03251ebeb4a0c30
URL: https://github.com/llvm/llvm-project/commit/f2a3079a1b48033a92d0a7d9f03251ebeb4a0c30 DIFF: https://github.com/llvm/llvm-project/commit/f2a3079a1b48033a92d0a7d9f03251ebeb4a0c30.diff LOG: [Modules] Fix spurious errors about module input file changing after .pcm file was built. (#176537) For incremental multi-process/multi-thread compilation utilizing a build session fix errors like > fatal error: file '/path/to/Frmwrk.framework/Headers/Header.h' has been > modified since the module file > '/path/to/ModuleCache.noindex/XXX/Frmwrk-YYY.pcm' was built: mtime changed > (was aaa, now bbb) Another symptom of the bug is when you check Frmwrk.pcm the header's mtime is correct, so it is confusing where "was aaa" is even coming from. The main problem is that in case of a signature mismatch `ModuleManager::addModule` returns `OutOfDate` but keeps .pcm buffer in `InMemoryModuleCache`. So if later on it tries to use such a module, it would have an outdated buffer in `InMemoryModuleCache`. If another process/thread happens to rebuild such module and write a new validation timestamp, the original process would skip the input file validation and would keep using the outdated .pcm buffer hitting errors about unexpected input modifications. I want to note that in the current implementation the validation timestamp can be still incorrect. When we are writing AST we don't account for the case when an input file is modified after its mtime and size are captured. We are relying on the input files not being modified while building a module using these files. rdar://165237499 Added: clang/test/ClangScanDeps/build-session-validation-outdated-module.c Modified: clang/lib/Serialization/ModuleManager.cpp Removed: ################################################################################ diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 236fe20fdad7c..353121c9b5184 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -179,6 +179,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, ModCache.getModuleTimestamp(NewModule->FileName); // Load the contents of the module + std::unique_ptr<llvm::MemoryBuffer> NewFileBuffer = nullptr; if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) { // The buffer was already provided for us. NewModule->Buffer = &getModuleCache().getInMemoryModuleCache().addBuiltPCM( @@ -215,8 +216,8 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, return Missing; } - NewModule->Buffer = &getModuleCache().getInMemoryModuleCache().addPCM( - FileName, std::move(*Buf)); + NewFileBuffer = std::move(*Buf); + NewModule->Buffer = NewFileBuffer.get(); } // Initialize the stream. @@ -228,6 +229,10 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, ExpectedSignature, ErrorStr)) return OutOfDate; + if (NewFileBuffer) + getModuleCache().getInMemoryModuleCache().addPCM(FileName, + std::move(NewFileBuffer)); + // We're keeping this module. Store it everywhere. Module = Modules[*Entry] = NewModule.get(); diff --git a/clang/test/ClangScanDeps/build-session-validation-outdated-module.c b/clang/test/ClangScanDeps/build-session-validation-outdated-module.c new file mode 100644 index 0000000000000..7f2296aa4ae14 --- /dev/null +++ b/clang/test/ClangScanDeps/build-session-validation-outdated-module.c @@ -0,0 +1,122 @@ +// Test build session validation with parallel compilation for incremental builds. + +// RUN: rm -rf %t +// RUN: split-file %s %t +// Use the same command line arguments to ensure .pcm files are in the same location. +// RUN: echo "-fsyntax-only -I %/t/include" > %t/ctx.rsp +// RUN: echo "-fmodules -fmodules-cache-path=%/t/module-cache" >> %t/ctx.rsp +// RUN: echo "-fbuild-session-file=%/t/session.timestamp" >> %t/ctx.rsp +// RUN: echo "-fmodules-validate-once-per-build-session" >> %t/ctx.rsp +// +// Add a build session file to avoid errors that it's missing. Doesn't do anything as we start with a clean build. +// RUN: touch %t/session.timestamp +// Generate headers that take noticeable time to parse. The purpose is to tweak the timing, so can reproduce the error. +// RUN: %python %t/generate-expensive-header.py A 2000000 > %t/include/expensive-header.h +// RUN: %python %t/generate-expensive-header.py B 500000 > %t/include/less-expensive-header.h +// +// Create SignatureExpectation.pcm with a specific Target signature. +// RUN: cp %t/version1.h %t/include/target.h +// RUN: sed "s|DIR|%/t|g" %t/prepare-expected-signature.json.template > %t/prepare-expected-signature.json +// RUN: clang-scan-deps -compilation-database %t/prepare-expected-signature.json > /dev/null +// +// Built a diff erent Target.pcm with a diff erent signature. +// RUN: cp %t/version2.h %t/include/target.h +// Update the build session to rebuild Target. +// RUN: touch %t/session.timestamp +// RUN: sed "s|DIR|%/t|g" %t/create-outdated-module.json.template > %t/create-outdated-module.json +// RUN: clang-scan-deps -compilation-database %t/create-outdated-module.json > /dev/null +// +// Simulate the incremental build. +// RUN: cp %t/version3.h %t/include/target.h +// RUN: touch %t/session.timestamp +// Scan 2 files in parallel. +// RUN: sed "s|DIR|%/t|g" %t/incremental.json.template > %t/incremental.json +// RUN: clang-scan-deps -compilation-database %t/incremental.json > /dev/null + +//--- include/module.modulemap +module Target { + header "target.h" + export * +} + +module SignatureExpectation { + header "signature-expectation.h" + textual header "expensive-header.h" + export * +} + +//--- version1.h +// empty + +//--- version2.h +void some_func(void); + +//--- version3.h +void some_func(void); +#define TARGET_MACRO 1 + +//--- generate-expensive-header.py +import sys + +num_items = int(sys.argv[2]) +suffix = sys.argv[1] +for i in range(num_items): + if i == 0: + print(f"#define MACRO_{suffix}_{i} 0") + else: + print(f"#define MACRO_{suffix}_{i} MACRO_{suffix}_{i-1} + 1") + +//--- include/signature-expectation.h +// Expensive header is used to slow down the build of SignatureExpectation module. +// Without the slow down we are able to validate and rebuild Target before another thread can do it. +#include <expensive-header.h> +#include <target.h> + +//--- prepare-expected-signature.c +#include <signature-expectation.h> +//--- prepare-expected-signature.json.template +[{ + "file": "DIR/prepare-expected-signature.c", + "directory": "DIR", + "command": "clang @DIR/ctx.rsp DIR/prepare-expected-signature.c" +}] + +//--- create-outdated-module.c +#include <target.h> +//--- create-outdated-module.json.template +[{ + "file": "DIR/create-outdated-module.c", + "directory": "DIR", + "command": "clang @DIR/ctx.rsp DIR/create-outdated-module.c" +}] + +//--- use-outdated-module.c +// At first load SignatureExpectation module, so it would load Target but +// reject it due to a signature mismatch. +// After this we rebuild SignatureExpectation and try to use already loaded +// Target buffer when encounter corresponding include in the source code. +// The problem is that it corresponds to version2.h and not version3.h. +#include <signature-expectation.h> + +#ifndef TARGET_MACRO +# error Macro is missing, probably using outdated Target module +# include <missing macro> +#endif + +//--- rebuild-outdated-module.c +// We aim for another thread to load the outdated module Target with +// the wrong signature before we rebuild it. +#include <less-expensive-header.h> +#include <target.h> + +//--- incremental.json.template +[{ + "file": "DIR/use-outdated-module.c", + "directory": "DIR", + "command": "clang @DIR/ctx.rsp DIR/use-outdated-module.c" +}, +{ + "file": "DIR/rebuild-outdated-module.c", + "directory": "DIR", + "command": "clang @DIR/ctx.rsp DIR/rebuild-outdated-module.c" +}] _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
