aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorjensmassberg <87519353+jensmassberg@users.noreply.github.com>2024-06-06 17:32:50 +0200
committerGitHub <noreply@github.com>2024-06-06 17:32:50 +0200
commit09f19c7396ecf26623d08c4288b35a60e950fcd8 (patch)
tree848ccc08cee0542a561656465972ff2c9a092558
parentdf168427b314f057c739eaccb21f361d3628f03b (diff)
downloadllvm-09f19c7396ecf26623d08c4288b35a60e950fcd8.zip
llvm-09f19c7396ecf26623d08c4288b35a60e950fcd8.tar.gz
llvm-09f19c7396ecf26623d08c4288b35a60e950fcd8.tar.bz2
[clang] Fix handling of adding a file with the same name as an existing dir to VFS (#94461)
When trying to add a file to clang's VFS via `addFile` and a directory of the same name already exists, we run into a [out-of-bound access](https://github.com/llvm/llvm-project/blob/145815c180fc82c5a55bf568d01d98d250490a55/llvm/lib/Support/Path.cpp#L244). The problem is that the file name is [recognised as existing path]( https://github.com/llvm/llvm-project/blob/145815c180fc82c5a55bf568d01d98d250490a55/llvm/lib/Support/VirtualFileSystem.cpp#L896) and thus continues to process the next part of the path which doesn't exist. This patch adds a check if we have reached the last part of the filename and return false in that case. This we reject to add a file if a directory of the same name already exists. This is in sync with [this check](https://github.com/llvm/llvm-project/blob/145815c180fc82c5a55bf568d01d98d250490a55/llvm/lib/Support/VirtualFileSystem.cpp#L903) that rejects adding a path if a file of the same name already exists.
-rw-r--r--llvm/lib/Support/VirtualFileSystem.cpp61
-rw-r--r--llvm/unittests/Support/VirtualFileSystemTest.cpp5
2 files changed, 36 insertions, 30 deletions
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index fcefdef9..7360901 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -867,21 +867,16 @@ bool InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime,
// Any intermediate directories we create should be accessible by
// the owner, even if Perms says otherwise for the final path.
const auto NewDirectoryPerms = ResolvedPerms | sys::fs::owner_all;
+
+ StringRef Name = *I;
while (true) {
- StringRef Name = *I;
- detail::InMemoryNode *Node = Dir->getChild(Name);
+ Name = *I;
++I;
+ if (I == E)
+ break;
+ detail::InMemoryNode *Node = Dir->getChild(Name);
if (!Node) {
- if (I == E) {
- // End of the path.
- Dir->addChild(
- Name, MakeNode({Dir->getUniqueID(), Path, Name, ModificationTime,
- std::move(Buffer), ResolvedUser, ResolvedGroup,
- ResolvedType, ResolvedPerms}));
- return true;
- }
-
- // Create a new directory. Use the path up to here.
+ // This isn't the last element, so we create a new directory.
Status Stat(
StringRef(Path.str().begin(), Name.end() - Path.str().begin()),
getDirectoryID(Dir->getUniqueID(), Name),
@@ -891,27 +886,33 @@ bool InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime,
Name, std::make_unique<detail::InMemoryDirectory>(std::move(Stat))));
continue;
}
+ // Creating file under another file.
+ if (!isa<detail::InMemoryDirectory>(Node))
+ return false;
+ Dir = cast<detail::InMemoryDirectory>(Node);
+ }
+ detail::InMemoryNode *Node = Dir->getChild(Name);
+ if (!Node) {
+ Dir->addChild(Name,
+ MakeNode({Dir->getUniqueID(), Path, Name, ModificationTime,
+ std::move(Buffer), ResolvedUser, ResolvedGroup,
+ ResolvedType, ResolvedPerms}));
+ return true;
+ }
+ if (isa<detail::InMemoryDirectory>(Node))
+ return ResolvedType == sys::fs::file_type::directory_file;
- if (auto *NewDir = dyn_cast<detail::InMemoryDirectory>(Node)) {
- Dir = NewDir;
- } else {
- assert((isa<detail::InMemoryFile>(Node) ||
- isa<detail::InMemoryHardLink>(Node)) &&
- "Must be either file, hardlink or directory!");
-
- // Trying to insert a directory in place of a file.
- if (I != E)
- return false;
+ assert((isa<detail::InMemoryFile>(Node) ||
+ isa<detail::InMemoryHardLink>(Node)) &&
+ "Must be either file, hardlink or directory!");
- // Return false only if the new file is different from the existing one.
- if (auto Link = dyn_cast<detail::InMemoryHardLink>(Node)) {
- return Link->getResolvedFile().getBuffer()->getBuffer() ==
- Buffer->getBuffer();
- }
- return cast<detail::InMemoryFile>(Node)->getBuffer()->getBuffer() ==
- Buffer->getBuffer();
- }
+ // Return false only if the new file is different from the existing one.
+ if (auto *Link = dyn_cast<detail::InMemoryHardLink>(Node)) {
+ return Link->getResolvedFile().getBuffer()->getBuffer() ==
+ Buffer->getBuffer();
}
+ return cast<detail::InMemoryFile>(Node)->getBuffer()->getBuffer() ==
+ Buffer->getBuffer();
}
bool InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime,
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index e9fd967..9e9b4fb 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1138,6 +1138,11 @@ TEST_F(InMemoryFileSystemTest, DuplicatedFile) {
ASSERT_FALSE(FS.addFile("/a/b", 0, MemoryBuffer::getMemBuffer("a")));
ASSERT_TRUE(FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a")));
ASSERT_FALSE(FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("b")));
+ ASSERT_TRUE(FS.addFile("/b/c/d", 0, MemoryBuffer::getMemBuffer("a")));
+ ASSERT_FALSE(FS.addFile("/b/c", 0, MemoryBuffer::getMemBuffer("a")));
+ ASSERT_TRUE(FS.addFile(
+ "/b/c", 0, MemoryBuffer::getMemBuffer(""), /*User=*/std::nullopt,
+ /*Group=*/std::nullopt, sys::fs::file_type::directory_file));
}
TEST_F(InMemoryFileSystemTest, DirectoryIteration) {