https://github.com/necto created https://github.com/llvm/llvm-project/pull/196057
Relocatable PCH are expected to tolerate a change in the absolute path of input files. It is natural that the files with different absolute paths can often have different mtime, so mtime should not be used as a proxy for file change when loading a relocatable PCH. -- CPP-8274 >From 8217386967e94bbfc9cecb36a595c4af7ccfb31f Mon Sep 17 00:00:00 2001 From: Arseniy Zaostrovnykh <[email protected]> Date: Wed, 6 May 2026 14:27:32 +0200 Subject: [PATCH 1/2] [clang] relocatable PCH do not support files with different mtime --- clang/test/PCH/relocatable-pch-mtime.c | 48 ++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 clang/test/PCH/relocatable-pch-mtime.c diff --git a/clang/test/PCH/relocatable-pch-mtime.c b/clang/test/PCH/relocatable-pch-mtime.c new file mode 100644 index 0000000000000..e5213903b1c84 --- /dev/null +++ b/clang/test/PCH/relocatable-pch-mtime.c @@ -0,0 +1,48 @@ +// Test that a relocatable PCH fails mtime validation when the source files +// have been relocated (different sysroot) and their timestamps have changed. +// This demonstrates that the mtime check in ASTReader::getInputFile does not +// account for the fact that relocated files will naturally have different mtimes. + +// RUN: rm -rf %t +// RUN: mkdir -p %t/sysroot_orig/usr/include + +// Create a simple header in the original sysroot. +// RUN: echo '#ifndef TEST_H' > %t/sysroot_orig/usr/include/test.h +// RUN: echo '#define TEST_H' >> %t/sysroot_orig/usr/include/test.h +// RUN: echo 'int relocated_val = 42;' >> %t/sysroot_orig/usr/include/test.h +// RUN: echo '#endif' >> %t/sysroot_orig/usr/include/test.h + +// Generate a relocatable PCH against the original sysroot. +// RUN: %clang_cc1 -x c-header -relocatable-pch -emit-pch \ +// RUN: -isysroot %t/sysroot_orig \ +// RUN: -o %t/test.pch %t/sysroot_orig/usr/include/test.h + +// Baseline: loading with the original sysroot succeeds. +// RUN: %clang_cc1 -include-pch %t/test.pch \ +// RUN: -isysroot %t/sysroot_orig \ +// RUN: -fsyntax-only %s + +// Set up a new sysroot by moving the header there (simulating workspace relocation). +// RUN: mkdir -p %t/sysroot_new/usr/include +// RUN: mv %t/sysroot_orig/usr/include/test.h %t/sysroot_new/usr/include/test.h + +// Loading the relocatable PCH with the new sysroot and with mtime preserved works. +// RUN: %clang_cc1 -include-pch %t/test.pch \ +// RUN: -isysroot %t/sysroot_new \ +// RUN: -fsyntax-only %s + +// Advance mtime of the copied header by 1 hour to simulate relocation timestamp +// drift, without changing content. +// RUN: %python -c "import os,sys; t=os.path.getmtime(sys.argv[1])+3600; os.utime(sys.argv[1],(t,t))" \ +// RUN: %t/sysroot_new/usr/include/test.h + +// Loading the relocatable PCH with the new sysroot and mtime should fail because the +// stored mtime no longer matches. +// RUN: not %clang_cc1 -include-pch %t/test.pch \ +// RUN: -isysroot %t/sysroot_new \ +// RUN: -fsyntax-only %s 2>&1 | FileCheck %s + +// CHECK: fatal error: file '{{.*}}test.h' has been modified since the precompiled header '{{.*}}test.pch' was built +// CHECK: note: mtime changed + +int get_val(void) { return relocated_val; } >From 380711d986f11857b0edf3404b65fce8dab06f17 Mon Sep 17 00:00:00 2001 From: Arseniy Zaostrovnykh <[email protected]> Date: Wed, 6 May 2026 14:33:13 +0200 Subject: [PATCH 2/2] [clang] Accept mtime change for input files of relocatable PCH --- clang/lib/Serialization/ASTReader.cpp | 2 +- clang/test/PCH/relocatable-pch-mtime.c | 32 +++++++++++++++----------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 2c0b908314fa5..e20db4aba3ad9 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -2942,7 +2942,7 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) { auto HasInputFileChanged = [&]() { if (StoredSize != File->getSize()) return Change{Change::Size, StoredSize, File->getSize()}; - if (!shouldDisableValidationForFile(F) && StoredTime && + if (!shouldDisableValidationForFile(F) && !F.RelocatablePCH && StoredTime && StoredTime != File->getModificationTime()) { Change MTimeChange = {Change::ModTime, StoredTime, File->getModificationTime()}; diff --git a/clang/test/PCH/relocatable-pch-mtime.c b/clang/test/PCH/relocatable-pch-mtime.c index e5213903b1c84..f9667fabf9ca1 100644 --- a/clang/test/PCH/relocatable-pch-mtime.c +++ b/clang/test/PCH/relocatable-pch-mtime.c @@ -1,7 +1,7 @@ -// Test that a relocatable PCH fails mtime validation when the source files -// have been relocated (different sysroot) and their timestamps have changed. -// This demonstrates that the mtime check in ASTReader::getInputFile does not -// account for the fact that relocated files will naturally have different mtimes. +// Test that a relocatable PCH tolerates mtime changes after relocation. +// When a PCH is relocatable, input files are resolved against a new -isysroot, +// so their mtimes will naturally differ from the stored ones; we should not +// reject the PCH on that basis. // RUN: rm -rf %t // RUN: mkdir -p %t/sysroot_orig/usr/include @@ -26,23 +26,29 @@ // RUN: mkdir -p %t/sysroot_new/usr/include // RUN: mv %t/sysroot_orig/usr/include/test.h %t/sysroot_new/usr/include/test.h -// Loading the relocatable PCH with the new sysroot and with mtime preserved works. +// Advance mtime of the relocated header by 1 hour to simulate timestamp drift +// that naturally occurs when files are relocated, without changing content. +// RUN: %python -c "import os,sys; t=os.path.getmtime(sys.argv[1])+3600; os.utime(sys.argv[1],(t,t))" \ +// RUN: %t/sysroot_new/usr/include/test.h + +// Loading the relocatable PCH with the new sysroot must succeed even though +// the header mtime changed: mtime drift is an expected consequence of +// relocation and must not be treated as a modification. // RUN: %clang_cc1 -include-pch %t/test.pch \ // RUN: -isysroot %t/sysroot_new \ // RUN: -fsyntax-only %s -// Advance mtime of the copied header by 1 hour to simulate relocation timestamp -// drift, without changing content. -// RUN: %python -c "import os,sys; t=os.path.getmtime(sys.argv[1])+3600; os.utime(sys.argv[1],(t,t))" \ -// RUN: %t/sysroot_new/usr/include/test.h - -// Loading the relocatable PCH with the new sysroot and mtime should fail because the -// stored mtime no longer matches. +// Content changes must still be detected via the size check. Overwrite the +// header in the new sysroot with different content. +// RUN: echo '#ifndef TEST_H' > %t/sysroot_new/usr/include/test.h +// RUN: echo '#define TEST_H' >> %t/sysroot_new/usr/include/test.h +// RUN: echo 'int relocated_val = 99999;' >> %t/sysroot_new/usr/include/test.h +// RUN: echo '#endif' >> %t/sysroot_new/usr/include/test.h // RUN: not %clang_cc1 -include-pch %t/test.pch \ // RUN: -isysroot %t/sysroot_new \ // RUN: -fsyntax-only %s 2>&1 | FileCheck %s // CHECK: fatal error: file '{{.*}}test.h' has been modified since the precompiled header '{{.*}}test.pch' was built -// CHECK: note: mtime changed +// CHECK: size changed int get_val(void) { return relocated_val; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
