Skip to content

Commit 09f19c7

Browse files
authored
[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.
1 parent df16842 commit 09f19c7

File tree

2 files changed

+36
-30
lines changed

2 files changed

+36
-30
lines changed

llvm/lib/Support/VirtualFileSystem.cpp

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -867,21 +867,16 @@ bool InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime,
867867
// Any intermediate directories we create should be accessible by
868868
// the owner, even if Perms says otherwise for the final path.
869869
const auto NewDirectoryPerms = ResolvedPerms | sys::fs::owner_all;
870+
871+
StringRef Name = *I;
870872
while (true) {
871-
StringRef Name = *I;
872-
detail::InMemoryNode *Node = Dir->getChild(Name);
873+
Name = *I;
873874
++I;
875+
if (I == E)
876+
break;
877+
detail::InMemoryNode *Node = Dir->getChild(Name);
874878
if (!Node) {
875-
if (I == E) {
876-
// End of the path.
877-
Dir->addChild(
878-
Name, MakeNode({Dir->getUniqueID(), Path, Name, ModificationTime,
879-
std::move(Buffer), ResolvedUser, ResolvedGroup,
880-
ResolvedType, ResolvedPerms}));
881-
return true;
882-
}
883-
884-
// Create a new directory. Use the path up to here.
879+
// This isn't the last element, so we create a new directory.
885880
Status Stat(
886881
StringRef(Path.str().begin(), Name.end() - Path.str().begin()),
887882
getDirectoryID(Dir->getUniqueID(), Name),
@@ -891,27 +886,33 @@ bool InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime,
891886
Name, std::make_unique<detail::InMemoryDirectory>(std::move(Stat))));
892887
continue;
893888
}
889+
// Creating file under another file.
890+
if (!isa<detail::InMemoryDirectory>(Node))
891+
return false;
892+
Dir = cast<detail::InMemoryDirectory>(Node);
893+
}
894+
detail::InMemoryNode *Node = Dir->getChild(Name);
895+
if (!Node) {
896+
Dir->addChild(Name,
897+
MakeNode({Dir->getUniqueID(), Path, Name, ModificationTime,
898+
std::move(Buffer), ResolvedUser, ResolvedGroup,
899+
ResolvedType, ResolvedPerms}));
900+
return true;
901+
}
902+
if (isa<detail::InMemoryDirectory>(Node))
903+
return ResolvedType == sys::fs::file_type::directory_file;
894904

895-
if (auto *NewDir = dyn_cast<detail::InMemoryDirectory>(Node)) {
896-
Dir = NewDir;
897-
} else {
898-
assert((isa<detail::InMemoryFile>(Node) ||
899-
isa<detail::InMemoryHardLink>(Node)) &&
900-
"Must be either file, hardlink or directory!");
901-
902-
// Trying to insert a directory in place of a file.
903-
if (I != E)
904-
return false;
905+
assert((isa<detail::InMemoryFile>(Node) ||
906+
isa<detail::InMemoryHardLink>(Node)) &&
907+
"Must be either file, hardlink or directory!");
905908

906-
// Return false only if the new file is different from the existing one.
907-
if (auto Link = dyn_cast<detail::InMemoryHardLink>(Node)) {
908-
return Link->getResolvedFile().getBuffer()->getBuffer() ==
909-
Buffer->getBuffer();
910-
}
911-
return cast<detail::InMemoryFile>(Node)->getBuffer()->getBuffer() ==
912-
Buffer->getBuffer();
913-
}
909+
// Return false only if the new file is different from the existing one.
910+
if (auto *Link = dyn_cast<detail::InMemoryHardLink>(Node)) {
911+
return Link->getResolvedFile().getBuffer()->getBuffer() ==
912+
Buffer->getBuffer();
914913
}
914+
return cast<detail::InMemoryFile>(Node)->getBuffer()->getBuffer() ==
915+
Buffer->getBuffer();
915916
}
916917

917918
bool InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime,

llvm/unittests/Support/VirtualFileSystemTest.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,6 +1138,11 @@ TEST_F(InMemoryFileSystemTest, DuplicatedFile) {
11381138
ASSERT_FALSE(FS.addFile("/a/b", 0, MemoryBuffer::getMemBuffer("a")));
11391139
ASSERT_TRUE(FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a")));
11401140
ASSERT_FALSE(FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("b")));
1141+
ASSERT_TRUE(FS.addFile("/b/c/d", 0, MemoryBuffer::getMemBuffer("a")));
1142+
ASSERT_FALSE(FS.addFile("/b/c", 0, MemoryBuffer::getMemBuffer("a")));
1143+
ASSERT_TRUE(FS.addFile(
1144+
"/b/c", 0, MemoryBuffer::getMemBuffer(""), /*User=*/std::nullopt,
1145+
/*Group=*/std::nullopt, sys::fs::file_type::directory_file));
11411146
}
11421147

11431148
TEST_F(InMemoryFileSystemTest, DirectoryIteration) {

0 commit comments

Comments
 (0)