[clang] Fix handling of adding a file with the same name as an existing dir to VFS

[jensmassberg]

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.

The problem is that the file name is recognised as existing path 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 that rejects adding a path if a file of the same name already exists.

[llvmbot]

@llvm/pr-subscribers-llvm-support

@llvm/pr-subscribers-clang

Author: None (jensmassberg)

Changes

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.

The problem is that the file name is recognised as existing path 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 that rejects adding a path if a file of the same name already exists.

---

Full diff: https://github.com/llvm/llvm-project/pull/94461.diff

2 Files Affected:

• (modified) llvm/lib/Support/VirtualFileSystem.cpp (+4)
• (modified) llvm/unittests/Support/VirtualFileSystemTest.cpp (+2)

diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index fcefdef992be5..defe7a7f45f44 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -893,6 +893,10 @@ bool InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime,
     }
 
     if (auto *NewDir = dyn_cast<detail::InMemoryDirectory>(Node)) {
+      // Trying to insert a file in place of a directory.
+      if (I == E)
+        return false;
+
       Dir = NewDir;
     } else {
       assert((isa<detail::InMemoryFile>(Node) ||
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index e9fd9671ea6ab..d12fd0ae2d503 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1138,6 +1138,8 @@ 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")));
 }
 
 TEST_F(InMemoryFileSystemTest, DirectoryIteration) {

[jensmassberg]

Thanks for the comments! I have refactored the code as suggested and added a new test case.

[kadircet]

thanks, lgtm!