[Modules] Use vfs for (recursive) directory iteration

[bruno]

Summary

Clang performs directory walk while searching headers inside modules by
using the ::sys::fs instead of ::vfs. This prevents any code that uses
the VFS (e.g, reproducer scripts) to actually find such headers, since
the VFS will never be searched for those.

Change these places to use vfs::recursive_directory_iterator and
vfs::directory_iterator instead.

[benlangmuir]

LGTM

[bruno]

Committed in r269661

lib/Frontend/FrontendActions.cpp

if (Module->Parent)
       // Include the umbrella header for submodules.
       addHeaderInclude(UmbrellaHeader.NameAsWritten, Includes, LangOpts,
                        Module->IsExternC);
   } else if (Module::DirectoryName UmbrellaDir = Module->getUmbrellaDir()) {
     // Add all of the headers we find in this subdirectory.
     std::error_code EC;
     SmallString<128> DirNative;
     llvm::sys::path::native(UmbrellaDir.Entry->getName(), DirNative);
-    for (llvm::sys::fs::recursive_directory_iterator Dir(DirNative, EC), 
-                                                     DirEnd;
-         Dir != DirEnd && !EC; Dir.increment(EC)) {
+
+    vfs::FileSystem &FS = *FileMgr.getVirtualFileSystem();
+    for (vfs::recursive_directory_iterator Dir(FS, DirNative, EC), End;
+         Dir != End && !EC; Dir.increment(EC)) {
       // Check whether this entry has an extension typically associated with 
       // headers.
-      if (!llvm::StringSwitch<bool>(llvm::sys::path::extension(Dir->path()))
+      if (!llvm::StringSwitch<bool>(llvm::sys::path::extension(Dir->getName()))
           .Cases(".h", ".H", ".hh", ".hpp", true)
           .Default(false))
         continue;
 
-      const FileEntry *Header = FileMgr.getFile(Dir->path());
+      const FileEntry *Header = FileMgr.getFile(Dir->getName());
       // FIXME: This shouldn't happen unless there is a file system race. Is
       // that worth diagnosing?
       if (!Header)
         continue;
 
       // If this header is marked 'unavailable' in this module, don't include 
       // it.
       if (ModMap.isHeaderUnavailableInModule(Header, Module))
         continue;
 
       // Compute the relative path from the directory to this file.
       SmallVector<StringRef, 16> Components;
-      auto PathIt = llvm::sys::path::rbegin(Dir->path());
+      auto PathIt = llvm::sys::path::rbegin(Dir->getName());
       for (int I = 0; I != Dir.level() + 1; ++I, ++PathIt)
         Components.push_back(*PathIt);
       SmallString<128> RelativeHeader(UmbrellaDir.NameAsWritten);
       for (auto It = Components.rbegin(), End = Components.rend(); It != End;
            ++It)
         llvm::sys::path::append(RelativeHeader, *It);
 
       // Include this header as part of the umbrella directory.

lib/Lex/HeaderSearch.cpp

for (unsigned Idx = 0, N = SearchDirs.size(); Idx != N; ++Idx) {
       bool IsSystem = SearchDirs[Idx].isSystemHeaderDirectory();
       if (SearchDirs[Idx].isFramework()) {
         std::error_code EC;
         SmallString<128> DirNative;
         llvm::sys::path::native(SearchDirs[Idx].getFrameworkDir()->getName(),
                                 DirNative);
 
         // Search each of the ".framework" directories to load them as modules.
-        for (llvm::sys::fs::directory_iterator Dir(DirNative, EC), DirEnd;
+        vfs::FileSystem &FS = *FileMgr.getVirtualFileSystem();
+        for (vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd;
              Dir != DirEnd && !EC; Dir.increment(EC)) {
-          if (llvm::sys::path::extension(Dir->path()) != ".framework")
+          if (llvm::sys::path::extension(Dir->getName()) != ".framework")
             continue;
 
           const DirectoryEntry *FrameworkDir =
-              FileMgr.getDirectory(Dir->path());
+              FileMgr.getDirectory(Dir->getName());
           if (!FrameworkDir)
             continue;
 
           // Load this framework module.
-          loadFrameworkModule(llvm::sys::path::stem(Dir->path()), FrameworkDir,
-                              IsSystem);
+          loadFrameworkModule(llvm::sys::path::stem(Dir->getName()),
+                              FrameworkDir, IsSystem);
         }
         continue;
       }
 
       // FIXME: Deal with header maps.
       if (SearchDirs[Idx].isHeaderMap())
         continue;
 
assert(HSOpts->ImplicitModuleMaps &&
          "Should not be loading subdirectory module maps");
 
   if (SearchDir.haveSearchedAllModuleMaps())
     return;
 
   std::error_code EC;
   SmallString<128> DirNative;
   llvm::sys::path::native(SearchDir.getDir()->getName(), DirNative);
-  for (llvm::sys::fs::directory_iterator Dir(DirNative, EC), DirEnd;
+  vfs::FileSystem &FS = *FileMgr.getVirtualFileSystem();
+  for (vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd;
        Dir != DirEnd && !EC; Dir.increment(EC)) {
-    bool IsFramework = llvm::sys::path::extension(Dir->path()) == ".framework";
+    bool IsFramework =
+        llvm::sys::path::extension(Dir->getName()) == ".framework";
     if (IsFramework == SearchDir.isFramework())
-      loadModuleMapFile(Dir->path(), SearchDir.isSystemHeaderDirectory(),
+      loadModuleMapFile(Dir->getName(), SearchDir.isSystemHeaderDirectory(),
                         SearchDir.isFramework());
   }
 
   SearchDir.setSearchedAllModuleMaps(true);
 }
 
 std::string HeaderSearch::suggestPathToFileForDiagnostics(const FileEntry *File,
                                                           bool *IsSystem) {

lib/Lex/ModuleMap.cpp

Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir,
   Result->InferExportWildcard = true;
   
   // Look for subframeworks.
   std::error_code EC;
   SmallString<128> SubframeworksDirName
     = StringRef(FrameworkDir->getName());
   llvm::sys::path::append(SubframeworksDirName, "Frameworks");
   llvm::sys::path::native(SubframeworksDirName);
-  for (llvm::sys::fs::directory_iterator Dir(SubframeworksDirName, EC), DirEnd;
+  vfs::FileSystem &FS = *FileMgr.getVirtualFileSystem();
+  for (vfs::directory_iterator Dir = FS.dir_begin(SubframeworksDirName, EC),
+                               DirEnd;
        Dir != DirEnd && !EC; Dir.increment(EC)) {
-    if (!StringRef(Dir->path()).endswith(".framework"))
+    if (!StringRef(Dir->getName()).endswith(".framework"))
       continue;
 
-    if (const DirectoryEntry *SubframeworkDir
-          = FileMgr.getDirectory(Dir->path())) {
+    if (const DirectoryEntry *SubframeworkDir =
+            FileMgr.getDirectory(Dir->getName())) {
       // Note: as an egregious but useful hack, we use the real path here and
       // check whether it is actually a subdirectory of the parent directory.
       // This will not be the case if the 'subframework' is actually a symlink
       // out to a top-level framework.
       StringRef SubframeworkDirName = FileMgr.getCanonicalName(SubframeworkDir);
       bool FoundParent = false;
       do {
         // Get the parent directory name.
void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
 
   if (UsesRequiresExcludedHack.count(ActiveModule)) {
     // Mark this header 'textual' (see doc comment for
     // ModuleMapParser::UsesRequiresExcludedHack). Although iterating over the
     // directory is relatively expensive, in practice this only applies to the
     // uncommonly used Tcl module on Darwin platforms.
     std::error_code EC;
     SmallVector<Module::Header, 6> Headers;
-    for (llvm::sys::fs::recursive_directory_iterator I(Dir->getName(), EC), E;
+    vfs::FileSystem &FS = *SourceMgr.getFileManager().getVirtualFileSystem();
+    for (vfs::recursive_directory_iterator I(FS, Dir->getName(), EC), E;
          I != E && !EC; I.increment(EC)) {
-      if (const FileEntry *FE = SourceMgr.getFileManager().getFile(I->path())) {
+      if (const FileEntry *FE =
+              SourceMgr.getFileManager().getFile(I->getName())) {
 
-        Module::Header Header = {I->path(), FE};
+        Module::Header Header = {I->getName(), FE};
         Headers.push_back(std::move(Header));
       }
     }
 
     // Sort header paths so that the pcm doesn't depend on iteration order.
     llvm::array_pod_sort(Headers.begin(), Headers.end(), compareModuleHeaders);
 
     for (auto &Header : Headers)

test/Modules/crash-vfs-umbrella-frameworks.m

 // CHECKYAML:      'name': "/[[PATH]]/i/Frameworks/B.framework/Headers",
 // CHECKYAML-NEXT:      'contents': [
 // CHECKYAML-NEXT:        {
 // CHECKYAML-NEXT:          'type': 'file',
 // CHECKYAML-NEXT:          'name': "B.h",
 // CHECKYAML-NEXT:          'external-contents': "/[[PATH]]/i/Frameworks/B.framework/Headers/B.h"
 
 @import I;
+
+// Run the reproducer script - regular exit code is enough to test it works. The
+// intent here is to guarantee that the collect umbrella headers into the VFS
+// can be used, testing that vfs::recursive_directory_iterator is used correctly
+// Make sure to erase the include paths used to build the modules to guarantee
+// that the VFS overlay won't fallback to use it. Also wipe out the module cache
+// to force header search.
+//
+// RUN: cd %t
+// RUN: rm -rf i
+// RUN: rm -rf crash-vfs-umbrella-*.cache/modules/*
+// RUN: chmod 755 crash-vfs-*.sh
+// RUN: ./crash-vfs-*.sh