diff options
author | jensmassberg <87519353+jensmassberg@users.noreply.github.com> | 2024-06-06 17:32:50 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-06-06 17:32:50 +0200 |
commit | 09f19c7396ecf26623d08c4288b35a60e950fcd8 (patch) | |
tree | 848ccc08cee0542a561656465972ff2c9a092558 | |
parent | df168427b314f057c739eaccb21f361d3628f03b (diff) | |
download | llvm-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.cpp | 61 | ||||
-rw-r--r-- | llvm/unittests/Support/VirtualFileSystemTest.cpp | 5 |
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) { |