[clang][modules] Derive mtime from PCM timestamps, not PCM files

[jansvoboda11]

#137363 was supposed to be NFC for the CrossProcessModuleCache (a.k.a normal implicit module builds), but accidentally passed the wrong path to sys::fs::status. Then, #141358 removed the correct path that should've been passed instead. (The variable was flagged as unused.) None of our existing regression tests caught this regression, we only found out due to a SourceKit-LSP benchmark getting slower.

This PR re-implements the original behavior, adds new remark to Clang for PCM input file validation, and uses it to create more reliable tests of the -fmodules-validate-once-per-build-session flag.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Jan Svoboda (jansvoboda11)

Changes

#137363 was supposed to be NFC for the CrossProcessModuleCache (a.k.a normal implicit module builds), but accidentally passed the wrong path to sys::fs::status. Then, #141358 removed the correct path that should've been passes there instead. (The variable was flagged as unused.) None of our existing regression tests caught this regression, we only found out due to a SourceKit-LSP benchmark getting slower.

This PR re-implements the original behavior, add new remark to Clang for PCM input file validation, and uses it to create more reliable tests of the -fmodules-validate-once-per-build-session flag.

---

Patch is 20.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/162965.diff

5 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticGroups.td (+1)
• (modified) clang/include/clang/Basic/DiagnosticSerializationKinds.td (+4)
• (modified) clang/lib/Serialization/ASTReader.cpp (+4)
• (modified) clang/lib/Serialization/ModuleCache.cpp (+3-1)
• (modified) clang/test/Modules/fmodules-validate-once-per-build-session.c (+125-110)

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index ef3f59f4f2263..8aa3489a2a62b 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def MissingFieldInitializers : DiagGroup<"missing-field-initializers",
 def ModuleLock : DiagGroup<"module-lock">;
 def ModuleBuild : DiagGroup<"module-build">;
 def ModuleImport : DiagGroup<"module-import">;
+def ModuleValidation : DiagGroup<"module-validation">;
 def ModuleConflict : DiagGroup<"module-conflict">;
 def ModuleFileExtension : DiagGroup<"module-file-extension">;
 def ModuleIncludeDirectiveTranslation : DiagGroup<"module-include-translation">;
diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index fc3585ffe8549..b80aff385e01f 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -86,6 +86,10 @@ def remark_module_import : Remark<
   "importing module '%0'%select{| into '%3'}2 from '%1'">,
   ShowInSystemHeader,
   InGroup<ModuleImport>;
+def remark_module_validation : Remark<
+  "validating %0 input files in module '%1' from '%2'">,
+  ShowInSystemHeader,
+  InGroup<ModuleValidation>;
 
 def err_imported_module_not_found : Error<
     "module '%0' in precompiled file '%1' %select{(imported by precompiled file '%2') |}4"
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 868f0cc8b1da7..c9104f5bc2ed0 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3190,6 +3190,10 @@ ASTReader::ReadControlBlock(ModuleFile &F,
             F.Kind == MK_ImplicitModule)
           N = ForceValidateUserInputs ? NumUserInputs : 0;
 
+        if (N != 0)
+          Diag(diag::remark_module_validation)
+              << N << F.ModuleName << F.FileName;
+
         for (unsigned I = 0; I < N; ++I) {
           InputFile IF = getInputFile(F, I+1, Complain);
           if (!IF.getFile() || IF.isOutOfDate())
diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp
index 96687277ebafd..9850956380423 100644
--- a/clang/lib/Serialization/ModuleCache.cpp
+++ b/clang/lib/Serialization/ModuleCache.cpp
@@ -115,8 +115,10 @@ class CrossProcessModuleCache : public ModuleCache {
   }
 
   std::time_t getModuleTimestamp(StringRef ModuleFilename) override {
+    std::string TimestampFilename =
+        serialization::ModuleFile::getTimestampFilename(ModuleFilename);
     llvm::sys::fs::file_status Status;
-    if (llvm::sys::fs::status(ModuleFilename, Status) != std::error_code{})
+    if (llvm::sys::fs::status(TimestampFilename, Status) != std::error_code{})
       return 0;
     return llvm::sys::toTimeT(Status.getLastModificationTime());
   }
diff --git a/clang/test/Modules/fmodules-validate-once-per-build-session.c b/clang/test/Modules/fmodules-validate-once-per-build-session.c
index d9d79b001e30c..fa6823747c970 100644
--- a/clang/test/Modules/fmodules-validate-once-per-build-session.c
+++ b/clang/test/Modules/fmodules-validate-once-per-build-session.c
@@ -1,119 +1,134 @@
-#include "foo.h"
-#include "bar.h"
-
-// Clear the module cache.
-// RUN: rm -rf %t
-// RUN: mkdir -p %t/Inputs
-// RUN: mkdir -p %t/modules-to-compare
+// This tests the behavior of -fmodules-validate-once-per-build-session with
+// different combinations of flags and states of the module cache.
 
-// ===
-// Create a module.  We will use -I or -isystem to determine whether to treat
-// foo.h as a system header.
-// RUN: echo 'void meow(void);' > %t/Inputs/foo.h
-// RUN: echo 'void woof(void);' > %t/Inputs/bar.h
-// RUN: echo 'module Foo { header "foo.h" }' > %t/Inputs/module.modulemap
-// RUN: echo 'extern module Bar "bar.modulemap"' >> %t/Inputs/module.modulemap
-// RUN: echo 'module Bar { header "bar.h" }' > %t/Inputs/bar.modulemap
+// Note: The `sleep 1` commands sprinkled throughout this test make the strict
+//       comparisons of epoch mtimes work as expected. Some may be unnecessary,
+//       but make the intent clearer.
 
-// ===
-// Compile the module.
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp
-// RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-before.pcm
-// RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-before.pcm
-// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-before-user.pcm
-// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-before-user.pcm
-// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-before-user-no-force.pcm
-// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-before-user-no-force.pcm
-
-// ===
-// Use it, and make sure that we did not recompile it.
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-use-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp
-// RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-after.pcm
-// RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-after.pcm
-// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-after-user.pcm
-// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-after-user.pcm
-// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
-// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: echo "-fsyntax-only -fmodules -fmodules-cache-path=%t/module-cache" > %t/ctx.rsp
+// RUN: echo "-fbuild-session-file=%t/module-cache/session.timestamp"      >> %t/ctx.rsp
+// RUN: echo "-fmodules-validate-once-per-build-session"                   >> %t/ctx.rsp
+// RUN: echo "-Rmodule-build -Rmodule-validation"                          >> %t/ctx.rsp
 
-// RUN: diff %t/modules-to-compare/Foo-before.pcm %t/modules-to-compare/Foo-after.pcm
-// RUN: diff %t/modules-to-compare/Bar-before.pcm %t/modules-to-compare/Bar-after.pcm
-// RUN: diff %t/modules-to-compare/Foo-before-user.pcm %t/modules-to-compare/Foo-after-user.pcm
-// RUN: diff %t/modules-to-compare/Bar-before-user.pcm %t/modules-to-compare/Bar-after-user.pcm
-// RUN: diff %t/modules-to-compare/Foo-before-user-no-force.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
-// RUN: diff %t/modules-to-compare/Bar-before-user-no-force.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
+//--- include/foo.h
+//--- include/module.modulemap
+module Foo { header "foo.h" }
 
-// ===
-// Change the sources.
+//--- clean.c
+// Clean module cache. Modules will get compiled regardless of validation settings.
+// RUN: mkdir %t/module-cache
 // RUN: sleep 1
-// RUN: echo 'void meow2(void);' > %t/Inputs/foo.h
-// RUN: echo 'module Bar { header "bar.h" export * }' > %t/Inputs/bar.modulemap
+// RUN: touch %t/module-cache/session.timestamp
+// RUN: sleep 1
+// RUN: %clang @%t/ctx.rsp %t/clean.c -DCTX=1 \
+// RUN:   -isystem %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/clean.c
+// RUN: %clang @%t/ctx.rsp %t/clean.c -DCTX=2 \
+// RUN:   -I %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/clean.c
+// RUN: %clang @%t/ctx.rsp %t/clean.c -DCTX=3 \
+// RUN:   -I %t/include -fmodules-validate-system-headers -Xclang -fno-modules-force-validate-user-headers \
+// RUN:     2>&1 | FileCheck %t/clean.c
+#include "foo.h"
+// CHECK: building module 'Foo'
 
-// ===
-// Use the module, and make sure that we did not recompile it if foo.h or
-// module.modulemap are system files or user files with force validation disabled,
-// even though the sources changed.
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp
-// RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-after.pcm
-// RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-after.pcm
-// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-after-user.pcm
-// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-after-user.pcm
-// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
-// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
+//--- no-change-same-session.c
+// Populated module cache in the same build session with unchanged inputs.
+// Validation only happens when it's forced for user headers. No compiles.
+// RUN: sleep 1
+// RUN: %clang @%t/ctx.rsp %t/no-change-same-session.c -DCTX=1 \
+// RUN:   -isystem %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-same-session.c --check-prefix=CHECK-NO-VALIDATION-OR-BUILD --allow-empty
+// RUN: %clang @%t/ctx.rsp %t/no-change-same-session.c -DCTX=2 \
+// RUN:   -I %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-same-session.c  --check-prefix=CHECK-VALIDATION-ONLY
+// RUN: %clang @%t/ctx.rsp %t/no-change-same-session.c -DCTX=3 \
+// RUN:   -I %t/include -fmodules-validate-system-headers -Xclang -fno-modules-force-validate-user-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-same-session.c --check-prefix=CHECK-NO-VALIDATION-OR-BUILD --allow-empty
+#include "foo.h"
+// CHECK-NO-VALIDATION-OR-BUILD-NOT: validating {{[0-9]+}} input files in module 'Foo'
+// CHECK-NO-VALIDATION-OR-BUILD-NOT: building module 'Foo'
+// CHECK-VALIDATION-ONLY: validating {{[0-9]+}} input files in module 'Foo'
+// CHECK-VALIDATION-ONLY-NOT: building module 'Foo'
 
-// RUN: diff %t/modules-to-compare/Foo-before.pcm %t/modules-to-compare/Foo-after.pcm
-// RUN: diff %t/modules-to-compare/Bar-before.pcm %t/modules-to-compare/Bar-after.pcm
-// When foo.h is an user header, we will validate it by default.
-// RUN: not diff %t/modules-to-compare/Foo-before-user.pcm %t/modules-to-compare/Foo-after-user.pcm
-// RUN: not diff %t/modules-to-compare/Bar-before-user.pcm %t/modules-to-compare/Bar-after-user.pcm
-// When foo.h is an user header, we will not validate it if force validation is turned off.
-// RUN: diff %t/modules-to-compare/Foo-before-user-no-force.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
-// RUN: diff %t/modules-to-compare/Bar-before-user-no-force.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
+//--- change-same-session.c
+// Populated module cache in the same build session with changed inputs.
+// Validation only happens when it's forced for user headers and results in compilation.
+// RUN: sleep 1
+// RUN: touch %t/include/foo.h
+// RUN: sleep 1
+// RUN: %clang @%t/ctx.rsp %t/change-same-session.c -DCTX=1 \
+// RUN:   -isystem %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/change-same-session.c --check-prefix=CHECK-NO-VALIDATION-OR-BUILD --allow-empty
+// RUN: %clang @%t/ctx.rsp %t/change-same-session.c -DCTX=2 \
+// RUN:   -I %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/change-same-session.c --check-prefix=CHECK-VALIDATION-AND-BUILD
+// RUN: %clang @%t/ctx.rsp %t/change-same-session.c -DCTX=3 \
+// RUN:   -I %t/include -fmodules-validate-system-headers -Xclang -fno-modules-force-validate-user-headers \
+// RUN:     2>&1 | FileCheck %t/change-same-session.c --check-prefix=CHECK-NO-VALIDATION-OR-BUILD --allow-empty
+#include "foo.h"
+// CHECK-NO-VALIDATION-OR-BUILD-NOT: validating {{[0-9]+}} input files in module 'Foo'
+// CHECK-NO-VALIDATION-OR-BUILD-NOT: building module 'Foo'
+// CHECK-VALIDATION-AND-BUILD: validating {{[0-9]+}} input files in module 'Foo'
+// CHECK-VALIDATION-AND-BUILD: building module 'Foo'
 
-// ===
-// Recompile the module if the today's date is before 01 January 2100.
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=4102441200 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=4102441200 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=4102441200 -fmodules-validate-once-per-build-session %s
-// RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp
-// RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-after.pcm
-// RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-after.pcm
-// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-after-user.pcm
-// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-after-user.pcm
-// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
-// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
+//--- change-new-session.c
+// Populated module cache in a new build session with changed inputs.
+// All configurations validate and recompile.
+// RUN: sleep 1
+// RUN: touch %t/include/foo.h
+// RUN: sleep 1
+// RUN: touch %t/module-cache/session.timestamp
+// RUN: sleep 1
+// RUN: %clang @%t/ctx.rsp %t/change-new-session.c -DCTX=1 \
+// RUN:   -isystem %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/change-new-session.c --check-prefixes=CHECK,CHECK-VALIDATE-ONCE
+// NOTE: Forced user headers validation causes redundant validation of the just-built module.
+// RUN: %clang @%t/ctx.rsp %t/change-new-session.c -DCTX=2 \
+// RUN:   -I %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/change-new-session.c --check-prefixes=CHECK,CHECK-FORCE-VALIDATE-TWICE
+// RUN: %clang @%t/ctx.rsp %t/change-new-session.c -DCTX=3 \
+// RUN:   -I %t/include -fmodules-validate-system-headers -Xclang -fno-modules-force-validate-user-headers \
+// RUN:     2>&1 | FileCheck %t/change-new-session.c --check-prefixes=CHECK,CHECK-VALIDATE-ONCE
+#include "foo.h"
+// CHECK: validating {{[0-9]+}} input files in module 'Foo'
+// CHECK: building module 'Foo'
+// CHECK-VALIDATE-ONCE-NOT: validating {{[0-9]+}} input files in module 'Foo'
+// CHECK-FORCE-VALIDATE-TWICE: validating {{[0-9]+}} input files in module 'Foo'
 
-// RUN: not diff %t/modules-to-compare/Foo-before.pcm %t/modules-to-compare/Foo-after.pcm
-// RUN: not diff %t/modules-to-compare/Bar-before.pcm %t/modules-to-compare/Bar-after.pcm
-// RUN: not diff %t/modules-to-compare/Foo-before-user.pcm %t/modules-to-compare/Foo-after-user.pcm
-// RUN: not diff %t/modules-to-compare/Bar-before-user.pcm %t/modules-to-compare/Bar-after-user.pcm
-// RUN: not diff %t/modules-to-compare/Foo-before-user-no-force.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
-// RUN: not diff %t/modules-to-compare/Bar-before-user-no-force.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
+//--- no-change-new-session-twice.c
+// Populated module cache in a new build session with unchanged inputs.
+// At first, all configurations validate but don't recompile.
+// RUN: sleep 1
+// RUN: touch %t/module-cache/session.timestamp
+// RUN: sleep 1
+// RUN: %clang @%t/ctx.rsp %t/no-change-new-session-twice.c -DCTX=1 \
+// RUN:   -isystem %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-new-session-twice.c --check-prefix=CHECK-ONCE
+// RUN: %clang @%t/ctx.rsp %t/no-change-new-session-twice.c -DCTX=2 \
+// RUN:   -I %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-new-session-twice.c --check-prefix=CHECK-ONCE
+// RUN: %clang @%t/ctx.rsp %t/no-change-new-session-twice.c -DCTX=3 \
+// RUN:   -I %t/include -fmodules-validate-system-headers -Xclang -fno-modules-force-validate-user-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-new-session-twice.c --check-prefix=CHECK-ONCE
+//
+// Then, only the forced user header validation performs redundant validation (but no compilation).
+// All other configurations do not validate and do not compile.
+// RUN: sleep 1
+// RUN: %clang @%t/ctx.rsp %t/no-change-new-session-twice.c -DCTX=1 \
+// RUN:   -isystem %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-new-session-twice.c --check-prefix=CHECK-NOT-TWICE --allow-empty
+// NOTE: Forced user headers validation causes redundant validation of the just-validated module.
+// RUN: %clang @%t/ctx.rsp %t/no-c...
[truncated]

[jansvoboda11]

This is what fails without the fix to ModuleCache.cpp. We'd use the mtime of the PCM from the old build session instead of the mtime of the PCM .timestamp file from the current build session. That would trigger unnecessary validation.