<rdar://problem/13363214> Eliminate race condition between module rebuild and the global module index.

The global module index was querying the file manager for each of the
module files it knows about at load time, to prune out any out-of-date
information. The file manager would then cache the results of the
stat() falls used to find that module file.

Later, the same translation unit could end up trying to import one of the
module files that had previously been ignored by the module cache, but
after some other Clang instance rebuilt the module file to bring it
up-to-date. The stale stat() results in the file manager would
trigger a second rebuild of the already-up-to-date module, causing
failures down the line.

The global module index now lazily resolves its module file references
to actual AST reader module files only after the module file has been
loaded, eliminating the stat-caching race. Moreover, the AST reader
can communicate to its caller that a module file is missing (rather
than simply being out-of-date), allowing us to simplify the
module-loading logic and allowing the compiler to recover if a
dependent module file ends up getting deleted.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177367 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 58 insertions, 101 deletions

diff --git a/lib/Frontend/ASTUnit.cpp b/lib/Frontend/ASTUnit.cpp
index 4a956ff3ab..c1115aedbf 100644
--- a/lib/Frontend/ASTUnit.cpp
+++ b/lib/Frontend/ASTUnit.cpp
@@ -801,6 +801,7 @@ ASTUnit *ASTUnit::LoadFromASTFile(const std::string &Filename,
     break;
 
   case ASTReader::Failure:
+  case ASTReader::Missing:
   case ASTReader::OutOfDate:
   case ASTReader::VersionMismatch:
   case ASTReader::ConfigurationMismatch:
diff --git a/lib/Frontend/ChainedIncludesSource.cpp b/lib/Frontend/ChainedIncludesSource.cpp
index f414f93812..3b31bf6362 100644
--- a/lib/Frontend/ChainedIncludesSource.cpp
+++ b/lib/Frontend/ChainedIncludesSource.cpp
@@ -47,6 +47,7 @@ static ASTReader *createASTReader(CompilerInstance &CI,
     return Reader.take();
 
   case ASTReader::Failure:
+  case ASTReader::Missing:
   case ASTReader::OutOfDate:
   case ASTReader::VersionMismatch:
   case ASTReader::ConfigurationMismatch:
diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp
index ce6572aa1b..d5c1a27ec8 100644
--- a/lib/Frontend/CompilerInstance.cpp
+++ b/lib/Frontend/CompilerInstance.cpp
@@ -338,6 +338,7 @@ CompilerInstance::createPCHExternalASTSource(StringRef Path,
     // Unrecoverable failure: don't even try to process the input file.
     break;
 
+  case ASTReader::Missing:
   case ASTReader::OutOfDate:
   case ASTReader::VersionMismatch:
   case ASTReader::ConfigurationMismatch:
@@ -945,88 +946,6 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
     } else
       ModuleFileName = PP->getHeaderSearchInfo().getModuleFileName(ModuleName);
 
-    if (ModuleFileName.empty()) {
-      getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
-        << ModuleName
-        << SourceRange(ImportLoc, ModuleNameLoc);
-      LastModuleImportLoc = ImportLoc;
-      LastModuleImportResult = ModuleLoadResult();
-      return LastModuleImportResult;
-    }
-    
-    const FileEntry *ModuleFile
-      = getFileManager().getFile(ModuleFileName, /*OpenFile=*/false,
-                                 /*CacheFailure=*/false);
-    bool BuildingModule = false;
-    if (!ModuleFile && Module) {
-      // The module is not cached, but we have a module map from which we can
-      // build the module.
-
-      // Check whether there is a cycle in the module graph.
-      ModuleBuildStack Path = getSourceManager().getModuleBuildStack();
-      ModuleBuildStack::iterator Pos = Path.begin(), PosEnd = Path.end();
-      for (; Pos != PosEnd; ++Pos) {
-        if (Pos->first == ModuleName)
-          break;
-      }
-
-      if (Pos != PosEnd) {
-        SmallString<256> CyclePath;
-        for (; Pos != PosEnd; ++Pos) {
-          CyclePath += Pos->first;
-          CyclePath += " -> ";
-        }
-        CyclePath += ModuleName;
-
-        getDiagnostics().Report(ModuleNameLoc, diag::err_module_cycle)
-          << ModuleName << CyclePath;
-        return ModuleLoadResult();
-      }
-
-      // Check whether we have already attempted to build this module (but
-      // failed).
-      if (getPreprocessorOpts().FailedModules &&
-          getPreprocessorOpts().FailedModules->hasAlreadyFailed(ModuleName)) {
-        getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_built)
-          << ModuleName
-          << SourceRange(ImportLoc, ModuleNameLoc);
-        ModuleBuildFailed = true;
-        return ModuleLoadResult();
-      }
-
-      BuildingModule = true;
-      compileModule(*this, ModuleNameLoc, Module, ModuleFileName);
-      ModuleFile = FileMgr->getFile(ModuleFileName, /*OpenFile=*/false,
-                                    /*CacheFailure=*/false);
-
-      if (!ModuleFile && getPreprocessorOpts().FailedModules)
-        getPreprocessorOpts().FailedModules->addFailed(ModuleName);
-    }
-
-    if (!ModuleFile) {
-      getDiagnostics().Report(ModuleNameLoc,
-                              BuildingModule? diag::err_module_not_built
-                                            : diag::err_module_not_found)
-        << ModuleName
-        << SourceRange(ImportLoc, ModuleNameLoc);
-      ModuleBuildFailed = true;
-      return ModuleLoadResult();
-    }
-
-    // If there is already a module file associated with this module, make sure
-    // it is the same as the module file we're looking for. Otherwise, we
-    // have two module files for the same module.
-    if (const FileEntry *CurModuleFile = Module? Module->getASTFile() : 0) {
-      if (CurModuleFile != ModuleFile) {
-        getDiagnostics().Report(ModuleNameLoc, diag::err_module_file_conflict)
-          << ModuleName
-          << CurModuleFile->getName()
-          << ModuleFile->getName();
-        ModuleBuildFailed = true;
-        return ModuleLoadResult();
-      }
-    }
-
     // If we don't already have an ASTReader, create one now.
     if (!ModuleManager) {
       if (!hasASTContext())
@@ -1056,21 +975,53 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
         ModuleManager->StartTranslationUnit(&getASTConsumer());
     }
 
-    // Try to load the module we found.
-    unsigned ARRFlags = ASTReader::ARR_None;
-    if (Module)
-      ARRFlags |= ASTReader::ARR_OutOfDate;
-    switch (ModuleManager->ReadAST(ModuleFile->getName(),
-                                   serialization::MK_Module, ImportLoc,
-                                   ARRFlags)) {
+    // Try to load the module file.
+    unsigned ARRFlags = ASTReader::ARR_OutOfDate | ASTReader::ARR_Missing;
+    switch (ModuleManager->ReadAST(ModuleFileName, serialization::MK_Module,
+                                   ImportLoc, ARRFlags)) {
     case ASTReader::Success:
       break;
 
     case ASTReader::OutOfDate: {
-      // The module file is out-of-date. Rebuild it.
-      getFileManager().invalidateCache(ModuleFile);
+      // The module file is out-of-date. Remove it, then rebuild it.
       bool Existed;
       llvm::sys::fs::remove(ModuleFileName, Existed);
+    }
+    // Fall through to build the module again.
+
+    case ASTReader::Missing: {
+      // The module file is (now) missing. Build it.
+
+      // If we don't have a module, we don't know how to build the module file.
+      // Complain and return.
+      if (!Module) {
+        getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
+          << ModuleName
+          << SourceRange(ImportLoc, ModuleNameLoc);
+        ModuleBuildFailed = true;
+        return ModuleLoadResult();
+      }
+
+      // Check whether there is a cycle in the module graph.
+      ModuleBuildStack ModPath = getSourceManager().getModuleBuildStack();
+      ModuleBuildStack::iterator Pos = ModPath.begin(), PosEnd = ModPath.end();
+      for (; Pos != PosEnd; ++Pos) {
+        if (Pos->first == ModuleName)
+          break;
+      }
+
+      if (Pos != PosEnd) {
+        SmallString<256> CyclePath;
+        for (; Pos != PosEnd; ++Pos) {
+          CyclePath += Pos->first;
+          CyclePath += " -> ";
+        }
+        CyclePath += ModuleName;
+
+        getDiagnostics().Report(ModuleNameLoc, diag::err_module_cycle)
+          << ModuleName << CyclePath;
+        return ModuleLoadResult();
+      }
 
       // Check whether we have already attempted to build this module (but
       // failed).
@@ -1083,15 +1034,23 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
         return ModuleLoadResult();
       }
 
+      // Try to compile the module.
       compileModule(*this, ModuleNameLoc, Module, ModuleFileName);
 
-      // Try loading the module again.
-      ModuleFile = FileMgr->getFile(ModuleFileName, /*OpenFile=*/false,
-                                    /*CacheFailure=*/false);
-      if (!ModuleFile ||
-          ModuleManager->ReadAST(ModuleFileName,
+      // Try to read the module file, now that we've compiled it.
+      ASTReader::ASTReadResult ReadResult
+        = ModuleManager->ReadAST(ModuleFileName,
                                  serialization::MK_Module, ImportLoc,
-                                 ASTReader::ARR_None) != ASTReader::Success) {
+                                 ASTReader::ARR_Missing);
+      if (ReadResult != ASTReader::Success) {
+        if (ReadResult == ASTReader::Missing) {
+          getDiagnostics().Report(ModuleNameLoc,
+                                  Module? diag::err_module_not_built
+                                        : diag::err_module_not_found)
+            << ModuleName
+            << SourceRange(ImportLoc, ModuleNameLoc);
+        }
+
         if (getPreprocessorOpts().FailedModules)
           getPreprocessorOpts().FailedModules->addFailed(ModuleName);
         KnownModules[Path[0].first] = 0;
@@ -1125,10 +1084,6 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
                  .findModule((Path[0].first->getName()));
     }
 
-    if (Module) {
-      Module->setASTFile(ModuleFile);
-    }
-    
     // Cache the result of this top-level module lookup for later.
     Known = KnownModules.insert(std::make_pair(Path[0].first, Module)).first;
   }