r209332 - VirtualFileSystem: Fix a few directory traversal bugs in VFSWriter

Justin Bogner mail at justinbogner.com
Wed May 21 15:46:51 PDT 2014


Author: bogner
Date: Wed May 21 17:46:51 2014
New Revision: 209332

URL: http://llvm.org/viewvc/llvm-project?rev=209332&view=rev
Log:
VirtualFileSystem: Fix a few directory traversal bugs in VFSWriter

There are a couple of issues with writing VFS maps that are awkward to
fix within the current mutually recursive approach. Instead, replace
the algorithm with an iterative version that uses an explicit stack of
directories.

Includes tests for cases the old approach was tripping on.

Modified:
    cfe/trunk/include/clang/Basic/VirtualFileSystem.h
    cfe/trunk/lib/Basic/VirtualFileSystem.cpp
    cfe/trunk/unittests/libclang/LibclangTest.cpp

Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=209332&r1=209331&r2=209332&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Wed May 21 17:46:51 2014
@@ -168,25 +168,17 @@ getVFSFromYAML(llvm::MemoryBuffer *Buffe
                void *DiagContext = nullptr,
                IntrusiveRefCntPtr<FileSystem> ExternalFS = getRealFileSystem());
 
+struct YAMLVFSEntry {
+  template <typename T1, typename T2> YAMLVFSEntry(T1 &&VPath, T2 &&RPath)
+      : VPath(std::forward<T1>(VPath)), RPath(std::forward<T2>(RPath)) {}
+  std::string VPath;
+  std::string RPath;
+};
+
 class YAMLVFSWriter {
-  struct MapEntry {
-    template <typename T1, typename T2> MapEntry(T1 &&VPath, T2 &&RPath)
-        : VPath(std::forward<T1>(VPath)), RPath(std::forward<T2>(RPath)) {}
-    std::string VPath;
-    std::string RPath;
-  };
-  std::vector<MapEntry> Mappings;
+  std::vector<YAMLVFSEntry> Mappings;
   Optional<bool> IsCaseSensitive;
 
-  llvm::ArrayRef<MapEntry> printDirNodes(llvm::raw_ostream &OS,
-                                         llvm::ArrayRef<MapEntry> Entries,
-                                         StringRef ParentPath, unsigned Indent);
-  llvm::ArrayRef<MapEntry> printContents(llvm::raw_ostream &OS,
-                                         llvm::ArrayRef<MapEntry> Entries,
-                                         unsigned Indent);
-  bool containedIn(StringRef Parent, StringRef Path);
-  StringRef containedPart(StringRef Parent, StringRef Path);
-
 public:
   YAMLVFSWriter() {}
   void addFileMapping(StringRef VirtualPath, StringRef RealPath);

Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=209332&r1=209331&r2=209332&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Wed May 21 17:46:51 2014
@@ -862,72 +862,25 @@ void YAMLVFSWriter::addFileMapping(Strin
   Mappings.emplace_back(VirtualPath, RealPath);
 }
 
-ArrayRef<YAMLVFSWriter::MapEntry>
-YAMLVFSWriter::printDirNodes(llvm::raw_ostream &OS, ArrayRef<MapEntry> Entries,
-                             StringRef ParentPath, unsigned Indent) {
-  while (!Entries.empty()) {
-    const MapEntry &Entry = Entries.front();
-    OS.indent(Indent) << "{\n";
-    Indent += 2;
-    OS.indent(Indent) << "'type': 'directory',\n";
-    StringRef DirName =
-        containedPart(ParentPath, sys::path::parent_path(Entry.VPath));
-    OS.indent(Indent)
-        << "'name': \"" << llvm::yaml::escape(DirName) << "\",\n";
-    OS.indent(Indent) << "'contents': [\n";
-    Entries = printContents(OS, Entries, Indent + 2);
-    OS.indent(Indent) << "]\n";
-    Indent -= 2;
-    OS.indent(Indent) << '}';
-    if (Entries.empty()) {
-      OS << '\n';
-      break;
-    }
-    StringRef NextVPath = Entries.front().VPath;
-    if (!containedIn(ParentPath, NextVPath)) {
-      OS << '\n';
-      break;
-    }
-    OS << ",\n";
-  }
-  return Entries;
-}
-
-ArrayRef<YAMLVFSWriter::MapEntry>
-YAMLVFSWriter::printContents(llvm::raw_ostream &OS, ArrayRef<MapEntry> Entries,
-                             unsigned Indent) {
-  using namespace llvm::sys;
-  while (!Entries.empty()) {
-    const MapEntry &Entry = Entries.front();
-    Entries = Entries.slice(1);
-    StringRef ParentPath = path::parent_path(Entry.VPath);
-    StringRef VName = path::filename(Entry.VPath);
-    OS.indent(Indent) << "{\n";
-    Indent += 2;
-    OS.indent(Indent) << "'type': 'file',\n";
-    OS.indent(Indent) << "'name': \"" << llvm::yaml::escape(VName) << "\",\n";
-    OS.indent(Indent) << "'external-contents': \""
-                      << llvm::yaml::escape(Entry.RPath) << "\"\n";
-    Indent -= 2;
-    OS.indent(Indent) << '}';
-    if (Entries.empty()) {
-      OS << '\n';
-      break;
-    }
-    StringRef NextVPath = Entries.front().VPath;
-    if (!containedIn(ParentPath, NextVPath)) {
-      OS << '\n';
-      break;
-    }
-    OS << ",\n";
-    if (path::parent_path(NextVPath) != ParentPath) {
-      Entries = printDirNodes(OS, Entries, ParentPath, Indent);
-    }
-  }
-  return Entries;
+namespace {
+class JSONWriter {
+  llvm::raw_ostream &OS;
+  SmallVector<StringRef, 16> DirStack;
+  inline unsigned getDirIndent() { return 4 * DirStack.size(); }
+  inline unsigned getFileIndent() { return 4 * (DirStack.size() + 1); }
+  bool containedIn(StringRef Parent, StringRef Path);
+  StringRef containedPart(StringRef Parent, StringRef Path);
+  void startDirectory(StringRef Path);
+  void endDirectory();
+  void writeEntry(StringRef VPath, StringRef RPath);
+
+public:
+  JSONWriter(llvm::raw_ostream &OS) : OS(OS) {}
+  void write(ArrayRef<YAMLVFSEntry> Entries, Optional<bool> IsCaseSensitive);
+};
 }
 
-bool YAMLVFSWriter::containedIn(StringRef Parent, StringRef Path) {
+bool JSONWriter::containedIn(StringRef Parent, StringRef Path) {
   using namespace llvm::sys;
   // Compare each path component.
   auto IParent = path::begin(Parent), EParent = path::end(Parent);
@@ -940,31 +893,89 @@ bool YAMLVFSWriter::containedIn(StringRe
   return IParent == EParent;
 }
 
-StringRef YAMLVFSWriter::containedPart(StringRef Parent, StringRef Path) {
+StringRef JSONWriter::containedPart(StringRef Parent, StringRef Path) {
+  assert(!Parent.empty());
   assert(containedIn(Parent, Path));
-  if (Parent.empty())
-    return Path;
   return Path.slice(Parent.size() + 1, StringRef::npos);
 }
 
-void YAMLVFSWriter::write(llvm::raw_ostream &OS) {
-  std::sort(Mappings.begin(), Mappings.end(),
-            [](const MapEntry &LHS, const MapEntry &RHS) {
-    return LHS.VPath < RHS.VPath;
-  });
+void JSONWriter::startDirectory(StringRef Path) {
+  StringRef Name =
+      DirStack.empty() ? Path : containedPart(DirStack.back(), Path);
+  DirStack.push_back(Path);
+  unsigned Indent = getDirIndent();
+  OS.indent(Indent) << "{\n";
+  OS.indent(Indent + 2) << "'type': 'directory',\n";
+  OS.indent(Indent + 2) << "'name': \"" << llvm::yaml::escape(Name) << "\",\n";
+  OS.indent(Indent + 2) << "'contents': [\n";
+}
+
+void JSONWriter::endDirectory() {
+  unsigned Indent = getDirIndent();
+  OS.indent(Indent + 2) << "]\n";
+  OS.indent(Indent) << "}";
+
+  DirStack.pop_back();
+}
+
+void JSONWriter::writeEntry(StringRef VPath, StringRef RPath) {
+  unsigned Indent = getFileIndent();
+  OS.indent(Indent) << "{\n";
+  OS.indent(Indent + 2) << "'type': 'file',\n";
+  OS.indent(Indent + 2) << "'name': \"" << llvm::yaml::escape(VPath) << "\",\n";
+  OS.indent(Indent + 2) << "'external-contents': \""
+                        << llvm::yaml::escape(RPath) << "\"\n";
+  OS.indent(Indent) << "}";
+}
+
+void JSONWriter::write(ArrayRef<YAMLVFSEntry> Entries,
+                       Optional<bool> IsCaseSensitive) {
+  using namespace llvm::sys;
 
   OS << "{\n"
         "  'version': 0,\n";
-  if (IsCaseSensitive.hasValue()) {
-    OS << "  'case-sensitive': '";
-    if (IsCaseSensitive.getValue())
-      OS << "true";
-    else
-      OS << "false";
-    OS << "',\n";
-  }
+  if (IsCaseSensitive.hasValue())
+    OS << "  'case-sensitive': '"
+       << (IsCaseSensitive.getValue() ? "true" : "false") << "',\n";
   OS << "  'roots': [\n";
-  printDirNodes(OS, Mappings, "", 4);
-  OS << "  ]\n"
+
+  if (Entries.empty())
+    return;
+
+  const YAMLVFSEntry &Entry = Entries.front();
+  startDirectory(path::parent_path(Entry.VPath));
+  writeEntry(path::filename(Entry.VPath), Entry.RPath);
+
+  for (const auto &Entry : Entries.slice(1)) {
+    StringRef Dir = path::parent_path(Entry.VPath);
+    if (Dir == DirStack.back())
+      OS << ",\n";
+    else {
+      while (!DirStack.empty() && !containedIn(DirStack.back(), Dir)) {
+        OS << "\n";
+        endDirectory();
+      }
+      OS << ",\n";
+      startDirectory(Dir);
+    }
+    writeEntry(path::filename(Entry.VPath), Entry.RPath);
+  }
+
+  while (!DirStack.empty()) {
+    OS << "\n";
+    endDirectory();
+  }
+
+  OS << "\n"
+     << "  ]\n"
      << "}\n";
 }
+
+void YAMLVFSWriter::write(llvm::raw_ostream &OS) {
+  std::sort(Mappings.begin(), Mappings.end(),
+            [](const YAMLVFSEntry &LHS, const YAMLVFSEntry &RHS) {
+    return LHS.VPath < RHS.VPath;
+  });
+
+  JSONWriter(OS).write(Mappings, IsCaseSensitive);
+}

Modified: cfe/trunk/unittests/libclang/LibclangTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/libclang/LibclangTest.cpp?rev=209332&r1=209331&r2=209332&view=diff
==============================================================================
--- cfe/trunk/unittests/libclang/LibclangTest.cpp (original)
+++ cfe/trunk/unittests/libclang/LibclangTest.cpp Wed May 21 17:46:51 2014
@@ -240,6 +240,74 @@ TEST(libclang, VirtualFileOverlay_Shared
   T.map("/path/foobarbaz.h", "/real/foobarbaz.h");
 }
 
+TEST(libclang, VirtualFileOverlay_AdjacentDirectory) {
+  const char *contents =
+      "{\n"
+      "  'version': 0,\n"
+      "  'roots': [\n"
+      "    {\n"
+      "      'type': 'directory',\n"
+      "      'name': \"/path/dir1\",\n"
+      "      'contents': [\n"
+      "        {\n"
+      "          'type': 'file',\n"
+      "          'name': \"foo.h\",\n"
+      "          'external-contents': \"/real/foo.h\"\n"
+      "        },\n"
+      "        {\n"
+      "          'type': 'directory',\n"
+      "          'name': \"subdir\",\n"
+      "          'contents': [\n"
+      "            {\n"
+      "              'type': 'file',\n"
+      "              'name': \"bar.h\",\n"
+      "              'external-contents': \"/real/bar.h\"\n"
+      "            }\n"
+      "          ]\n"
+      "        }\n"
+      "      ]\n"
+      "    },\n"
+      "    {\n"
+      "      'type': 'directory',\n"
+      "      'name': \"/path/dir2\",\n"
+      "      'contents': [\n"
+      "        {\n"
+      "          'type': 'file',\n"
+      "          'name': \"baz.h\",\n"
+      "          'external-contents': \"/real/baz.h\"\n"
+      "        }\n"
+      "      ]\n"
+      "    }\n"
+      "  ]\n"
+      "}\n";
+  TestVFO T(contents);
+  T.map("/path/dir1/foo.h", "/real/foo.h");
+  T.map("/path/dir1/subdir/bar.h", "/real/bar.h");
+  T.map("/path/dir2/baz.h", "/real/baz.h");
+}
+
+TEST(libclang, VirtualFileOverlay_TopLevel) {
+  const char *contents =
+      "{\n"
+      "  'version': 0,\n"
+      "  'roots': [\n"
+      "    {\n"
+      "      'type': 'directory',\n"
+      "      'name': \"/\",\n"
+      "      'contents': [\n"
+      "        {\n"
+      "          'type': 'file',\n"
+      "          'name': \"foo.h\",\n"
+      "          'external-contents': \"/real/foo.h\"\n"
+      "        }\n"
+      "      ]\n"
+      "    }\n"
+      "  ]\n"
+      "}\n";
+  TestVFO T(contents);
+  T.map("/foo.h", "/real/foo.h");
+}
+
 TEST(libclang, ModuleMapDescriptor) {
   const char *Contents =
     "framework module TestFrame {\n"





More information about the cfe-commits mailing list