[clang] Make PCH's respect any VFS specified. (PR #106577)

Neil Henning via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 29 11:38:50 PDT 2024


https://github.com/sheredom updated https://github.com/llvm/llvm-project/pull/106577

>From 774ca72fd61c98a830930dab9c60e16fcc6782a9 Mon Sep 17 00:00:00 2001
From: Neil Henning <neil.henning at epicgames.com>
Date: Thu, 29 Aug 2024 13:15:49 +0100
Subject: [PATCH] Make PCH's respect any VFS specified.

We want to be able to generate a PCH against one file-system path, and
then re-use that PCH when the file-system path is different (but the
sources are the same). We also do not know when generating the PCH what
the destination file-system path will be, so what we want to be able to
do is:

- When generating a PCH map the original directory to some fake
  directory. You could imagine `D:/Foo` being mapped to `Z:/Foo` for
  instance.
- Then when consuming a PCH, we want to be able to use a different
  mapping to map `Z:/Foo` to `D:/Some/Other/Machines/Foo` for instance.

This will let us generate and share PCHs to speed up compile time for
our users.

To enable this we've made PCH generation respect any specified
vfsoverlay, such that it will remap the paths in the PCH accordingly.

We've also made `-verify-pch` respect the
`-fvalidate-ast-input-files-content` option so that we can force
verification of inputs.
---
 clang/lib/Frontend/FrontendActions.cpp        |  3 ++-
 clang/lib/Serialization/ASTWriter.cpp         | 21 +++++++++++++--
 clang/test/PCH/verify_no_timestamp.c          |  8 ++++++
 .../Inputs/vfsoverlay-directory-remap.yaml    |  9 +++++++
 clang/test/VFS/remap-to-fake.c                | 26 +++++++++++++++++++
 5 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/PCH/verify_no_timestamp.c
 create mode 100644 clang/test/VFS/Inputs/vfsoverlay-directory-remap.yaml
 create mode 100644 clang/test/VFS/remap-to-fake.c

diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index 9f5d09e33ce244..9a60aaf880e96b 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -347,7 +347,8 @@ void VerifyPCHAction::ExecuteAction() {
       DisableValidationForModuleKind::None,
       /*AllowASTWithCompilerErrors*/ false,
       /*AllowConfigurationMismatch*/ true,
-      /*ValidateSystemInputs*/ true));
+      /*ValidateSystemInputs*/ true,
+      CI.getHeaderSearchOpts().ValidateASTInputFilesContent));
 
   Reader->ReadAST(getCurrentFile(),
                   Preamble ? serialization::MK_Preamble
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 5cfb98c2a1060a..bba060d01338a8 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1115,13 +1115,13 @@ void ASTWriter::WriteBlockInfoBlock() {
 }
 
 /// Prepares a path for being written to an AST file by converting it
-/// to an absolute path and removing nested './'s.
+/// to an absolute path and removing nested './'s and '../'s.
 ///
 /// \return \c true if the path was changed.
 static bool cleanPathForOutput(FileManager &FileMgr,
                                SmallVectorImpl<char> &Path) {
   bool Changed = FileMgr.makeAbsolutePath(Path);
-  return Changed | llvm::sys::path::remove_dots(Path);
+  return Changed | llvm::sys::path::remove_dots(Path, true);
 }
 
 /// Adjusts the given filename to only write out the portion of the
@@ -4772,6 +4772,23 @@ bool ASTWriter::PreparePathForOutput(SmallVectorImpl<char> &Path) {
     Changed = true;
   }
 
+  // If we are generating a normal PCH (EG. not a C++ module).
+  if (!WritingModule) {
+    // Use the vfs overlay if it exists to translate paths.
+    auto &FileSys =
+        Context->getSourceManager().getFileManager().getVirtualFileSystem();
+
+    if (auto *RFS = dyn_cast<llvm::vfs::RedirectingFileSystem>(&FileSys)) {
+      if (auto Result = RFS->lookupPath(PathStr)) {
+        if (std::optional<StringRef> Redirect = Result->getExternalRedirect()) {
+          PathStr = *Redirect;
+          Path.assign(PathStr.data(), PathStr.data() + PathStr.size());
+          Changed = true;
+        }
+      }
+    }
+  }
+
   return Changed;
 }
 
diff --git a/clang/test/PCH/verify_no_timestamp.c b/clang/test/PCH/verify_no_timestamp.c
new file mode 100644
index 00000000000000..8aca76cf77c449
--- /dev/null
+++ b/clang/test/PCH/verify_no_timestamp.c
@@ -0,0 +1,8 @@
+// RUN: echo 'int SomeFunc() { return 42; }' > %t.h
+// RUN: %clang_cc1 -Werror -fno-pch-timestamp -fvalidate-ast-input-files-content -emit-pch -o "%t.pch" %t.h
+
+// Now change the source file, which should cause the verifier to fail with content mismatch.
+// RUN: echo 'int SomeFunc() { return 13; }' > %t.h
+// RUN: not %clang_cc1 -fno-pch-timestamp -fvalidate-ast-input-files-content -verify-pch %t.pch 2>&1 | FileCheck %s -DT=%t
+
+// CHECK: fatal error: file '[[T]].h' has been modified since the precompiled header '[[T]].pch' was built: content changed
diff --git a/clang/test/VFS/Inputs/vfsoverlay-directory-remap.yaml b/clang/test/VFS/Inputs/vfsoverlay-directory-remap.yaml
new file mode 100644
index 00000000000000..3cb279a08fae14
--- /dev/null
+++ b/clang/test/VFS/Inputs/vfsoverlay-directory-remap.yaml
@@ -0,0 +1,9 @@
+{
+  'version': 0,
+  'roots': [
+    { 'name': 'FROM_DIR',
+      'type': 'directory-remap',
+      'external-contents': 'TO_DIR'
+    }
+  ]
+}
diff --git a/clang/test/VFS/remap-to-fake.c b/clang/test/VFS/remap-to-fake.c
new file mode 100644
index 00000000000000..fbf14117c7e8cc
--- /dev/null
+++ b/clang/test/VFS/remap-to-fake.c
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/From
+// RUN: mkdir -p %t/To
+// RUN: echo '#pragma once' > %t/From/B.h
+// RUN: echo 'int SomeFunc() { return 13; }' >> %t/From/B.h
+// RUN: echo '#pragma once' > %t/To/B.h
+// RUN: echo 'int SomeFunc() { return 13; }' >> %t/To/B.h
+// RUN: sed -e "s at FROM_DIR@%{/t:regex_replacement}/From at g" -e "s at TO_DIR@%{/t:regex_replacement}/Fake at g" %S/Inputs/vfsoverlay-directory-remap.yaml > %t/to-fake.yaml
+// RUN: sed -e "s at FROM_DIR@%{/t:regex_replacement}/Fake at g" -e "s at TO_DIR@%{/t:regex_replacement}/To at g" %S/Inputs/vfsoverlay-directory-remap.yaml > %t/from-fake.yaml
+
+// RUN: %clang_cc1 -Werror -fno-pch-timestamp -fvalidate-ast-input-files-content -ivfsoverlay %t/to-fake.yaml -emit-pch -o "%t.pch" %t/From/B.h
+
+// Remove the `From` directory as we don't want to accidentally find that source if the PCH hasn't remapped using the VFS!
+// RUN: rm -rf %t/From
+
+// The PCH will be invalid because the `Fake` directory does not exist.
+// RUN: not %clang_cc1 -fno-pch-timestamp -fvalidate-ast-input-files-content -verify-pch %t.pch
+
+// But if we specify the correct VFS overlay it'll verify clean.
+// RUN: %clang_cc1 -fno-pch-timestamp -fvalidate-ast-input-files-content -verify-pch -ivfsoverlay %t/from-fake.yaml %t.pch
+
+// RUN: %clang_cc1 -fno-pch-timestamp -fvalidate-ast-input-files-content -Werror -I %t/To -ivfsoverlay %t/from-fake.yaml -include-pch "%t.pch" -emit-llvm -C %s -o %t.o
+
+#include "B.h"
+
+int UseSomeFunc() { return SomeFunc(); }



More information about the cfe-commits mailing list