[clang] [clang][Modules] Use strict comparison at build-session validation boundary (PR #197203)

[Cyndy Ishida via cfe-commits]

https://github.com/cyndyishida created https://github.com/llvm/llvm-project/pull/197203

The relocation check used `IFVT >= session` while input validation used `IFVT > session`, silently skipping the relocation check when both landed in the same truncated second. Share a single helper with the strict comparison.  This was noticed when the accompanying test was flaky on green dragon macOS bot. 

Also:

* Drop the test's `sleep 1` timing dependency by future-dating the session timestamp directly.
* Remove the dead `touch %t/session.timestamp` lines in the ClangScanDeps test (the scanner ignores the file's mtime).

hopefully resolves: rdar://173816745

>From f4aeea072214309fa8e233feac01f6266f9fac02 Mon Sep 17 00:00:00 2001
From: Cyndy Ishida <cyndy_ishida at apple.com>
Date: Tue, 12 May 2026 07:03:07 -0700
Subject: [PATCH] [clang][Modules] Use strict comparison at build-session
 validation boundary

The relocation check used `IFVT >= session` while input validation used `IFVT > session`, silently skipping the relocation check when both landed in the same truncated second. Share a single helper with the strict comparison.

Also:

* Drop the test's `sleep 1` timing dependency by future-dating the
    session timestamp directly.
* Remove the dead `touch %t/session.timestamp` lines in the
    ClangScanDeps test (the scanner ignores the file's mtime).

hopefully resolves: rdar://173816745
---
 clang/lib/Serialization/ASTReader.cpp         | 30 +++++++++++++------
 ...ild-session-validation-relocated-modules.c |  4 +--
 ...ild-session-validation-relocated-modules.c |  8 ++---
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index dfd714dd53814..0f834859e982a 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3179,6 +3179,18 @@ ASTReader::ASTReadResult ASTReader::ReadOptionsBlock(
   }
 }
 
+/// Returns {build-session validation applies, MF was validated this session}.
+static std::pair<bool, bool>
+wasValidatedInBuildSession(const ModuleFile &MF,
+                           const HeaderSearchOptions &HSOpts) {
+  const bool EnablesBSValidation =
+      HSOpts.ModulesValidateOncePerBuildSession && MF.Kind == MK_ImplicitModule;
+  const bool WasValidated =
+      EnablesBSValidation &&
+      MF.InputFilesValidationTimestamp > HSOpts.BuildSessionTimestamp;
+  return {EnablesBSValidation, WasValidated};
+}
+
 ASTReader::RelocationResult
 ASTReader::getModuleForRelocationChecks(ModuleFile &F, bool DirectoryCheck) {
   // Don't emit module relocation errors if we have -fno-validate-pch.
@@ -3201,12 +3213,13 @@ ASTReader::getModuleForRelocationChecks(ModuleFile &F, bool DirectoryCheck) {
   // When only validating modules once per build session,
   // Skip check if the timestamp is up to date or module was built in same build
   // session.
-  if (HSOpts.ModulesValidateOncePerBuildSession && IsImplicitModule) {
-    if (F.InputFilesValidationTimestamp >= HSOpts.BuildSessionTimestamp)
-      return {std::nullopt, IgnoreError};
-    if (static_cast<uint64_t>(F.ModTime) >= HSOpts.BuildSessionTimestamp)
-      return {std::nullopt, IgnoreError};
-  }
+  auto [EnablesBSValidation, WasValidated] =
+      wasValidatedInBuildSession(F, HSOpts);
+  if (WasValidated)
+    return {std::nullopt, IgnoreError};
+  if (EnablesBSValidation &&
+      static_cast<uint64_t>(F.ModTime) >= HSOpts.BuildSessionTimestamp)
+    return {std::nullopt, IgnoreError};
 
   Diag(diag::remark_module_check_relocation) << F.ModuleName << F.FileName;
 
@@ -3294,9 +3307,8 @@ ASTReader::ReadControlBlock(ModuleFile &F,
         F.InputFilesValidationStatus = ValidateSystemInputs
                                            ? InputFilesValidation::AllFiles
                                            : InputFilesValidation::UserFiles;
-        if (HSOpts.ModulesValidateOncePerBuildSession &&
-            F.InputFilesValidationTimestamp > HSOpts.BuildSessionTimestamp &&
-            F.Kind == MK_ImplicitModule) {
+        auto [_, WasValidated] = wasValidatedInBuildSession(F, HSOpts);
+        if (WasValidated) {
           N = ForceValidateUserInputs ? NumUserInputs : 0;
           F.InputFilesValidationStatus =
               ForceValidateUserInputs
diff --git a/clang/test/ClangScanDeps/build-session-validation-relocated-modules.c b/clang/test/ClangScanDeps/build-session-validation-relocated-modules.c
index 47919229512d0..5a0744718685b 100644
--- a/clang/test/ClangScanDeps/build-session-validation-relocated-modules.c
+++ b/clang/test/ClangScanDeps/build-session-validation-relocated-modules.c
@@ -12,14 +12,12 @@
 // RUN: split-file %s %t
 // RUN: sed -e "s|DIR|%/t|g" %t/compile-commands.json.in > %t/compile-commands.json
 
-// RUN: touch %t/session.timestamp
 // RUN: clang-scan-deps -format experimental-full -j 1 \
 // RUN:   -compilation-database %t/compile-commands.json -o %t/deps1.json 
 // RUN: cat %t/deps1.json | FileCheck %s --check-prefix=DEPS1 
 
 // Model update where same framework appears in earlier search path.
 // This can occur on an incremental build where dependency relationships are updated.
-// RUN: touch %t/session.timestamp
 // RUN: sleep 1
 // RUN: mkdir %t/preferred_frameworks/
 // RUN: cp -r %t/fallback_frameworks/MovedDep.framework %t/preferred_frameworks/
@@ -46,6 +44,8 @@
 }
 ]
 
+//--- session.timestamp
+
 //--- fallback_frameworks/MovedDep.framework/Modules/module.modulemap
 framework module MovedDep { header "MovedDep.h" }
 //--- fallback_frameworks/MovedDep.framework/Headers/MovedDep.h
diff --git a/clang/test/Modules/build-session-validation-relocated-modules.c b/clang/test/Modules/build-session-validation-relocated-modules.c
index 173d35528fb4f..5a1a249896040 100644
--- a/clang/test/Modules/build-session-validation-relocated-modules.c
+++ b/clang/test/Modules/build-session-validation-relocated-modules.c
@@ -26,11 +26,9 @@
 // RUN:   -fbuild-session-file=%t/session.timestamp -fmodules-validate-once-per-build-session \
 // RUN:   -Xclang -fno-modules-check-relocated -Rmodule-validation 2>&1 | FileCheck %s --check-prefix=NO_RELOC
 
-// Ensure future new timestamp doesn't have same time as older one.
-// RUN: sleep 1
-
-// Now remove the disablement and check.
-// RUN: touch %t/session.timestamp
+// Force session.timestamp into the future so it is strictly greater than any
+// cached pcm or timestamp mtime, regardless of wall-clock resolution.
+// RUN: %python -c "import os, time; t = time.time() + 3600; os.utime('%t/session.timestamp', (t, t))"
 // RUN: %clang -fmodules -fimplicit-module-maps  -fsyntax-only %t/tu1.c \
 // RUN:   -fmodules-cache-path=%t/cache -F%t/preferred_frameworks -F%t/fallback_frameworks \
 // RUN:   -fbuild-session-file=%t/session.timestamp -fmodules-validate-once-per-build-session \