[clang] [llvm] [llvm][vfs] Allow root directory remap. Fix assertion failure. (PR #83766)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 3 23:52:17 PST 2024
https://github.com/yelleyee updated https://github.com/llvm/llvm-project/pull/83766
>From f133ce3ee5f26f18182218949cc19ba4ff5ecd92 Mon Sep 17 00:00:00 2001
From: yelleyee <feelyeling at gmail.com>
Date: Mon, 4 Mar 2024 05:32:13 +0000
Subject: [PATCH] [llvm][vfs] Allow root directory remap
Add a new feature allowing root(/) directory remap by breaking the
assumption that remap root must be a directory.
Now the config below will be allowed.
Directory-remap config of root(/) is not allowed in previous impl.
Config list below will cause assertion failure in
`uniqueOverlayTree`.
'name': '/',
'type': 'directory-remap',
'external-contents': '/xxxx/xxx'
---
clang/test/VFS/Inputs/system-root-remap.yaml | 12 ++++++
clang/test/VFS/system-root-remap.c | 14 +++++++
llvm/lib/Support/VirtualFileSystem.cpp | 32 ++++++++++----
.../Support/VirtualFileSystemTest.cpp | 42 +++++++++++++++++++
4 files changed, 92 insertions(+), 8 deletions(-)
create mode 100644 clang/test/VFS/Inputs/system-root-remap.yaml
create mode 100644 clang/test/VFS/system-root-remap.c
diff --git a/clang/test/VFS/Inputs/system-root-remap.yaml b/clang/test/VFS/Inputs/system-root-remap.yaml
new file mode 100644
index 00000000000000..43d25fcc952236
--- /dev/null
+++ b/clang/test/VFS/Inputs/system-root-remap.yaml
@@ -0,0 +1,12 @@
+{
+ 'version': 0,
+ 'use-external-names': false,
+ 'redirecting-with': 'redirect-only'
+ 'roots': [
+ {
+ 'name': '/',
+ 'type': 'directory-remap',
+ 'external-contents': 'EXTERNAL_DIR'
+ }
+ ]
+}
diff --git a/clang/test/VFS/system-root-remap.c b/clang/test/VFS/system-root-remap.c
new file mode 100644
index 00000000000000..f4168b75f29909
--- /dev/null
+++ b/clang/test/VFS/system-root-remap.c
@@ -0,0 +1,14 @@
+// RUN: mkdir -p %t
+// RUN: cd %t
+// RUN: sed -e "s at INPUT_DIR@%{/S:regex_replacement}/Inputs at g" -e "s at EXTERNAL_DIR@%{/t:regex_replacement}@g" %S/Inputs/system-root-remap.yaml > %t.yaml
+// RUN: mkdir -p %t%t
+// RUN: cp %S/Inputs/actual_header.h %t%t/not_real.h
+// RUN: mkdir -p %t%S
+// RUN: cp %s %t%S
+// RUN: %clang_cc1 -Werror -I . -vfsoverlay %t.yaml -fsyntax-only -working-directory=%t %s
+
+#include "not_real.h"
+
+void foo(void) {
+ bar();
+}
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index 051dd2a67d120f..3a94afea9a0629 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -54,8 +54,8 @@
using namespace llvm;
using namespace llvm::vfs;
-using llvm::sys::fs::file_t;
using llvm::sys::fs::file_status;
+using llvm::sys::fs::file_t;
using llvm::sys::fs::file_type;
using llvm::sys::fs::kInvalidFile;
using llvm::sys::fs::perms;
@@ -1056,7 +1056,7 @@ llvm::ErrorOr<Status> InMemoryFileSystem::status(const Twine &Path) {
llvm::ErrorOr<std::unique_ptr<File>>
InMemoryFileSystem::openFileForRead(const Twine &Path) {
- auto Node = lookupNode(Path,/*FollowFinalSymlink=*/true);
+ auto Node = lookupNode(Path, /*FollowFinalSymlink=*/true);
if (!Node)
return Node.getError();
@@ -1224,7 +1224,6 @@ static bool isFileNotFound(std::error_code EC,
} // anonymous namespace
-
RedirectingFileSystem::RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> FS)
: ExternalFS(std::move(FS)) {
if (ExternalFS)
@@ -1350,7 +1349,8 @@ std::error_code RedirectingFileSystem::isLocal(const Twine &Path_,
return ExternalFS->isLocal(Path, Result);
}
-std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl<char> &Path) const {
+std::error_code
+RedirectingFileSystem::makeAbsolute(SmallVectorImpl<char> &Path) const {
// is_absolute(..., Style::windows_*) accepts paths with both slash types.
if (llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::posix) ||
llvm::sys::path::is_absolute(Path,
@@ -1747,6 +1747,10 @@ class llvm::vfs::RedirectingFileSystemParser {
void uniqueOverlayTree(RedirectingFileSystem *FS,
RedirectingFileSystem::Entry *SrcE,
RedirectingFileSystem::Entry *NewParentE = nullptr) {
+ if (NewParentE) {
+ assert(NewParentE->getKind() == RedirectingFileSystem::EK_Directory &&
+ "NewParentE must be a directory entry or nullptr");
+ }
StringRef Name = SrcE->getName();
switch (SrcE->getKind()) {
case RedirectingFileSystem::EK_Directory: {
@@ -1756,13 +1760,26 @@ class llvm::vfs::RedirectingFileSystemParser {
// is parsed. This only leads to redundant walks, ignore it.
if (!Name.empty())
NewParentE = lookupOrCreateEntry(FS, Name, NewParentE);
+ if (NewParentE->getKind() != RedirectingFileSystem::EK_Directory) {
+ // Found non directory entry, no need to generate the left nodes.
+ break;
+ }
for (std::unique_ptr<RedirectingFileSystem::Entry> &SubEntry :
llvm::make_range(DE->contents_begin(), DE->contents_end()))
uniqueOverlayTree(FS, SubEntry.get(), NewParentE);
break;
}
case RedirectingFileSystem::EK_DirectoryRemap: {
- assert(NewParentE && "Parent entry must exist");
+ // Root DirectoryRemap:
+ // name: "/"
+ // external-name: "xxxx"
+ if (!NewParentE) {
+ auto *DR = cast<RedirectingFileSystem::DirectoryRemapEntry>(SrcE);
+ FS->Roots.push_back(
+ std::make_unique<RedirectingFileSystem::DirectoryRemapEntry>(
+ Name, DR->getExternalContentsPath(), DR->getUseName()));
+ break;
+ }
auto *DR = cast<RedirectingFileSystem::DirectoryRemapEntry>(SrcE);
auto *DE = cast<RedirectingFileSystem::DirectoryEntry>(NewParentE);
DE->addContent(
@@ -2776,9 +2793,8 @@ void JSONWriter::write(ArrayRef<YAMLVFSEntry> Entries,
if (!Entries.empty()) {
const YAMLVFSEntry &Entry = Entries.front();
- startDirectory(
- Entry.IsDirectory ? Entry.VPath : path::parent_path(Entry.VPath)
- );
+ startDirectory(Entry.IsDirectory ? Entry.VPath
+ : path::parent_path(Entry.VPath));
StringRef RPath = Entry.RPath;
if (UseOverlayRelative) {
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index d4abbb4345873c..0a77d8b13eca0e 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1715,6 +1715,48 @@ TEST_F(VFSFromYAMLTest, MappedRoot) {
EXPECT_EQ(0, NumDiagnostics);
}
+TEST_F(VFSFromYAMLTest, MappedSysRoot) {
+ IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
+ Lower->addDirectory("//root/foo/bar");
+ Lower->addRegularFile("//root/foo/bar/a");
+ IntrusiveRefCntPtr<vfs::FileSystem> FS =
+ getFromYAMLString("{ 'roots': [\n"
+ "{\n"
+ " 'type': 'directory-remap',\n"
+ " 'name': '/',\n"
+ " 'external-contents': '//root/foo/bar'\n"
+ "}\n"
+ "]\n"
+ "}",
+ Lower);
+ ASSERT_NE(FS.get(), nullptr);
+
+ IntrusiveRefCntPtr<vfs::OverlayFileSystem> O(
+ new vfs::OverlayFileSystem(Lower));
+ O->pushOverlay(FS);
+
+ // file
+ ErrorOr<vfs::Status> S = O->status("/a");
+ ASSERT_FALSE(S.getError());
+ EXPECT_EQ("//root/foo/bar/a", S->getName());
+ EXPECT_TRUE(S->ExposesExternalVFSPath);
+
+ ErrorOr<vfs::Status> SLower = O->status("//root/foo/bar/a");
+ EXPECT_EQ("//root/foo/bar/a", SLower->getName());
+ EXPECT_TRUE(S->equivalent(*SLower));
+ EXPECT_FALSE(SLower->ExposesExternalVFSPath);
+
+ // file after opening
+ auto OpenedF = O->openFileForRead("/a");
+ ASSERT_FALSE(OpenedF.getError());
+ auto OpenedS = (*OpenedF)->status();
+ ASSERT_FALSE(OpenedS.getError());
+ EXPECT_EQ("//root/foo/bar/a", OpenedS->getName());
+ EXPECT_TRUE(OpenedS->ExposesExternalVFSPath);
+
+ EXPECT_EQ(0, NumDiagnostics);
+}
+
TEST_F(VFSFromYAMLTest, RemappedDirectoryOverlay) {
IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
Lower->addDirectory("//root/foo");
More information about the cfe-commits
mailing list