<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, 153 insertions, 29 deletions

diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h
index 2744865819..61a0cbebd7 100644
--- a/include/clang/Serialization/ASTReader.h
+++ b/include/clang/Serialization/ASTReader.h
@@ -48,6 +48,7 @@
 #include <string>
 #include <utility>
 #include <vector>
+#include <sys/stat.h>
 
 namespace llvm {
   class MemoryBuffer;
@@ -236,6 +237,8 @@ public:
     Success,
     /// \brief The AST file itself appears corrupted.
     Failure,
+    /// \brief The AST file was missing.
+    Missing,
     /// \brief The AST file is out-of-date relative to its input files,
     /// and needs to be regenerated.
     OutOfDate,
@@ -949,6 +952,7 @@ private:
   ASTReadResult ReadASTCore(StringRef FileName, ModuleKind Type,
                             SourceLocation ImportLoc, ModuleFile *ImportedBy,
                             SmallVectorImpl<ImportedModule> &Loaded,
+                            off_t ExpectedSize, time_t ExpectedModTime,
                             unsigned ClientLoadCapabilities);
   ASTReadResult ReadControlBlock(ModuleFile &F,
                                  SmallVectorImpl<ImportedModule> &Loaded,
@@ -1146,15 +1150,18 @@ public:
     /// \brief The client can't handle any AST loading failures.
     ARR_None = 0,
     /// \brief The client can handle an AST file that cannot load because it
+    /// is missing.
+    ARR_Missing = 0x1,
+    /// \brief The client can handle an AST file that cannot load because it
     /// is out-of-date relative to its input files.
-    ARR_OutOfDate = 0x1,
+    ARR_OutOfDate = 0x2,
     /// \brief The client can handle an AST file that cannot load because it
     /// was built with a different version of Clang.
-    ARR_VersionMismatch = 0x2,
+    ARR_VersionMismatch = 0x4,
     /// \brief The client can handle an AST file that cannot load because it's
     /// compiled configuration doesn't match that of the context it was
     /// loaded into.
-    ARR_ConfigurationMismatch = 0x4
+    ARR_ConfigurationMismatch = 0x8
   };
 
   /// \brief Load the AST file designated by the given file name.
diff --git a/include/clang/Serialization/GlobalModuleIndex.h b/include/clang/Serialization/GlobalModuleIndex.h
index ec8ad353e0..53f61e2d59 100644
--- a/include/clang/Serialization/GlobalModuleIndex.h
+++ b/include/clang/Serialization/GlobalModuleIndex.h
@@ -34,9 +34,44 @@ class DirectoryEntry;
 class FileEntry;
 class FileManager;
 
+namespace serialization {
+  class ModuleFile;
+};
+
 using llvm::SmallVector;
 using llvm::SmallVectorImpl;
 using llvm::StringRef;
+using serialization::ModuleFile;
+
+/// \brief Abstract class that resolves a module file name to a ModuleFile
+/// pointer, which is used to uniquely describe a module file.
+class ModuleFileNameResolver {
+public:
+  virtual ~ModuleFileNameResolver();
+
+  /// \brief Attempt to resolve the given module file name to a specific,
+  /// already-loaded module.
+  ///
+  /// \param FileName The name of the module file.
+  ///
+  /// \param ExpectedSize The size that the module file is expected to have.
+  /// If the actual size differs, the resolver should return \c true.
+  ///
+  /// \param ExpectedModTime The modification time that the module file is
+  /// expected to have. If the actual modification time differs, the resolver
+  /// should return \c true.
+  ///
+  /// \param File Will be set to the module file if there is one, or null
+  /// otherwise.
+  ///
+  /// \returns True if a module file exists but does not meet the size/
+  /// modification time criteria, false if the module file is available or has
+  /// not yet been loaded.
+  virtual bool resolveModuleFileName(StringRef FileName,
+                                     off_t ExpectedSize,
+                                     time_t ExpectedModTime,
+                                     ModuleFile *&File) = 0;
+};
 
 /// \brief A global index for a set of module files, providing information about
 /// the identifiers within those module files.
@@ -54,6 +89,9 @@ class GlobalModuleIndex {
   /// as the global module index is live.
   llvm::OwningPtr<llvm::MemoryBuffer> Buffer;
 
+  /// \brief The module file name resolver.
+  ModuleFileNameResolver *Resolver;
+
   /// \brief The hash table.
   ///
   /// This pointer actually points to a IdentifierIndexTable object,
@@ -63,25 +101,39 @@ class GlobalModuleIndex {
 
   /// \brief Information about a given module file.
   struct ModuleInfo {
-    ModuleInfo() : File() { }
+    ModuleInfo() : File(), Size(), ModTime() { }
+
+    /// \brief The module file, if it is known.
+    ModuleFile *File;
+
+    /// \brief The module file name.
+    std::string FileName;
+
+    /// \brief Size of the module file at the time the global index was built.
+    off_t Size;
 
-    /// \brief The module file entry.
-    const FileEntry *File;
+    /// \brief Modification time of the module file at the time the global
+    /// index was built.
+    time_t ModTime;
 
-    /// \brief The module files on which this module directly depends.
-    llvm::SmallVector<const FileEntry *, 4> Dependencies;
+    /// \brief The module IDs on which this module directly depends.
+    /// FIXME: We don't really need a vector here.
+    llvm::SmallVector<unsigned, 4> Dependencies;
+
+    /// \brief The module IDs that directly depend on this module.
+    llvm::SmallVector<unsigned, 4> ImportedBy;
   };
 
   /// \brief A mapping from module IDs to information about each module.
   ///
   /// This vector may have gaps, if module files have been removed or have
   /// been updated since the index was built. A gap is indicated by an empty
-  /// \c File pointer.
+  /// file name.
   llvm::SmallVector<ModuleInfo, 16> Modules;
 
-  /// \brief Lazily-populated mapping from module file entries to their
+  /// \brief Lazily-populated mapping from module files to their
   /// corresponding index into the \c Modules vector.
-  llvm::DenseMap<const FileEntry *, unsigned> ModulesByFile;
+  llvm::DenseMap<ModuleFile *, unsigned> ModulesByFile;
 
   /// \brief The number of identifier lookups we performed.
   unsigned NumIdentifierLookups;
@@ -91,7 +143,7 @@ class GlobalModuleIndex {
   unsigned NumIdentifierLookupHits;
 
   /// \brief Internal constructor. Use \c readIndex() to read an index.
-  explicit GlobalModuleIndex(FileManager &FileMgr, llvm::MemoryBuffer *Buffer,
+  explicit GlobalModuleIndex(llvm::MemoryBuffer *Buffer,
                              llvm::BitstreamCursor Cursor);
 
   GlobalModuleIndex(const GlobalModuleIndex &) LLVM_DELETED_FUNCTION;
@@ -115,29 +167,27 @@ public:
 
   /// \brief Read a global index file for the given directory.
   ///
-  /// \param FileMgr The file manager to use for reading files.
-  ///
   /// \param Path The path to the specific module cache where the module files
   /// for the intended configuration reside.
   ///
   /// \returns A pair containing the global module index (if it exists) and
   /// the error code.
   static std::pair<GlobalModuleIndex *, ErrorCode>
-  readIndex(FileManager &FileMgr, StringRef Path);
+  readIndex(StringRef Path);
 
   /// \brief Retrieve the set of modules that have up-to-date indexes.
   ///
   /// \param ModuleFiles Will be populated with the set of module files that
   /// have been indexed.
-  void getKnownModules(SmallVectorImpl<const FileEntry *> &ModuleFiles);
+  void getKnownModules(SmallVectorImpl<ModuleFile *> &ModuleFiles);
 
   /// \brief Retrieve the set of module files on which the given module file
   /// directly depends.
-  void getModuleDependencies(const FileEntry *ModuleFile,
-                             SmallVectorImpl<const FileEntry *> &Dependencies);
+  void getModuleDependencies(ModuleFile *File,
+                             SmallVectorImpl<ModuleFile *> &Dependencies);
 
   /// \brief A set of module files in which we found a result.
-  typedef llvm::SmallPtrSet<const FileEntry *, 4> HitSet;
+  typedef llvm::SmallPtrSet<ModuleFile *, 4> HitSet;
   
   /// \brief Look for all of the module files with information about the given
   /// identifier, e.g., a global function, variable, or type with that name.
@@ -150,6 +200,20 @@ public:
   /// \returns true if the identifier is known to the index, false otherwise.
   bool lookupIdentifier(StringRef Name, HitSet &Hits);
 
+  /// \brief Set the module file name resolver.
+  void setResolver(ModuleFileNameResolver *Resolver) {
+    this->Resolver = Resolver;
+  }
+
+  /// \brief Note that additional modules have been loaded, which invalidates
+  /// the module file -> module cache.
+  void noteAdditionalModulesLoaded() {
+    ModulesByFile.clear();
+  }
+
+  /// \brief Resolve the module file for the module with the given ID.
+  ModuleFile *resolveModuleFile(unsigned ID);
+
   /// \brief Print statistics to standard error.
   void printStats();
 
diff --git a/include/clang/Serialization/ModuleManager.h b/include/clang/Serialization/ModuleManager.h
index 60e3b40a7a..9b58b75ebb 100644
--- a/include/clang/Serialization/ModuleManager.h
+++ b/include/clang/Serialization/ModuleManager.h
@@ -16,17 +16,18 @@
 #define LLVM_CLANG_SERIALIZATION_MODULE_MANAGER_H
 
 #include "clang/Basic/FileManager.h"
+#include "clang/Serialization/GlobalModuleIndex.h"
 #include "clang/Serialization/Module.h"
 #include "llvm/ADT/DenseMap.h"
 
 namespace clang { 
 
-class GlobalModuleIndex;
+class ModuleMap;
 
 namespace serialization {
   
 /// \brief Manages the set of modules loaded by an AST reader.
-class ModuleManager {
+class ModuleManager : public ModuleFileNameResolver {
   /// \brief The chain of AST files. The first entry is the one named by the
   /// user, the last one is the one that doesn't depend on anything further.
   SmallVector<ModuleFile *, 2> Chain;
@@ -143,6 +144,19 @@ public:
   
   /// \brief Number of modules loaded
   unsigned size() const { return Chain.size(); }
+
+  /// \brief The result of attempting to add a new module.
+  enum AddModuleResult {
+    /// \brief The module file had already been loaded.
+    AlreadyLoaded,
+    /// \brief The module file was just loaded in response to this call.
+    NewlyLoaded,
+    /// \brief The module file is missing.
+    Missing,
+    /// \brief The module file is out-of-date.
+    OutOfDate
+  };
+
   /// \brief Attempts to create a new module and add it to the list of known
   /// modules.
   ///
@@ -157,18 +171,30 @@ public:
   ///
   /// \param Generation The generation in which this module was loaded.
   ///
+  /// \param ExpectedSize The expected size of the module file, used for
+  /// validation. This will be zero if unknown.
+  ///
+  /// \param ExpectedModTime The expected modification time of the module
+  /// file, used for validation. This will be zero if unknown.
+  ///
+  /// \param Module A pointer to the module file if the module was successfully
+  /// loaded.
+  ///
   /// \param ErrorStr Will be set to a non-empty string if any errors occurred
   /// while trying to load the module.
   ///
   /// \return A pointer to the module that corresponds to this file name,
-  /// and a boolean indicating whether the module was newly added.
-  std::pair<ModuleFile *, bool> 
-  addModule(StringRef FileName, ModuleKind Type, SourceLocation ImportLoc,
-            ModuleFile *ImportedBy, unsigned Generation,
-            std::string &ErrorStr);
+  /// and a value indicating whether the module was loaded.
+  AddModuleResult addModule(StringRef FileName, ModuleKind Type,
+                            SourceLocation ImportLoc,
+                            ModuleFile *ImportedBy, unsigned Generation,
+                            off_t ExpectedSize, time_t ExpectedModTime,
+                            ModuleFile *&Module,
+                            std::string &ErrorStr);
 
   /// \brief Remove the given set of modules.
-  void removeModules(ModuleIterator first, ModuleIterator last);
+  void removeModules(ModuleIterator first, ModuleIterator last,
+                     ModuleMap *modMap);
 
   /// \brief Add an in-memory buffer the list of known buffers
   void addInMemoryBuffer(StringRef FileName, llvm::MemoryBuffer *Buffer);
@@ -200,7 +226,7 @@ public:
   /// Any module that is known to both the global module index and the module
   /// manager that is *not* in this set can be skipped.
   void visit(bool (*Visitor)(ModuleFile &M, void *UserData), void *UserData,
-             llvm::SmallPtrSet<const FileEntry *, 4> *ModuleFilesHit = 0);
+             llvm::SmallPtrSet<ModuleFile *, 4> *ModuleFilesHit = 0);
   
   /// \brief Visit each of the modules with a depth-first traversal.
   ///
@@ -221,7 +247,34 @@ public:
   void visitDepthFirst(bool (*Visitor)(ModuleFile &M, bool Preorder, 
                                        void *UserData), 
                        void *UserData);
-  
+
+  /// \brief Attempt to resolve the given module file name to a file entry.
+  ///
+  /// \param FileName The name of the module file.
+  ///
+  /// \param ExpectedSize The size that the module file is expected to have.
+  /// If the actual size differs, the resolver should return \c true.
+  ///
+  /// \param ExpectedModTime The modification time that the module file is
+  /// expected to have. If the actual modification time differs, the resolver
+  /// should return \c true.
+  ///
+  /// \param File Will be set to the file if there is one, or null
+  /// otherwise.
+  ///
+  /// \returns True if a file exists but does not meet the size/
+  /// modification time criteria, false if the file is either available and
+  /// suitable, or is missing.
+  bool lookupModuleFile(StringRef FileName,
+                        off_t ExpectedSize,
+                        time_t ExpectedModTime,
+                        const FileEntry *&File);
+
+  virtual bool resolveModuleFileName(StringRef FileName,
+                                     off_t ExpectedSize,
+                                     time_t ExpectedModTime,
+                                     ModuleFile *&File);
+
   /// \brief View the graphviz representation of the module graph.
   void viewGraph();
 };